diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 574b9e27f33ca..e6a934a63c69b 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -83,15 +83,18 @@ void PlatformConfiguration::DidCreateIsolate() { Dart_GetField(library, tonic::ToDart("_reportTimings"))); } -void PlatformConfiguration::AddView(int64_t view_id, +bool PlatformConfiguration::AddView(int64_t view_id, const ViewportMetrics& view_metrics) { auto [view_iterator, insertion_happened] = metrics_.emplace(view_id, view_metrics); - FML_DCHECK(insertion_happened); + if (!insertion_happened) { + FML_LOG(ERROR) << "View #" << view_id << " already exists."; + return false; + } std::shared_ptr dart_state = add_view_.dart_state().lock(); if (!dart_state) { - return; + return false; } tonic::DartState::Scope scope(dart_state); tonic::CheckAndHandleError(tonic::DartInvoke( @@ -119,6 +122,7 @@ void PlatformConfiguration::AddView(int64_t view_id, tonic::ToDart(view_metrics.physical_display_features_state), tonic::ToDart(view_metrics.display_id), })); + return true; } bool PlatformConfiguration::RemoveView(int64_t view_id) { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 6a583765d962f..b668b85cfd603 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -262,8 +262,7 @@ class PlatformConfigurationClient { /// @brief A class for holding and distributing platform-level information /// to and from the Dart code in Flutter's framework. /// -/// It handles communication between the engine and the framework, -/// and owns the main window. +/// It handles communication between the engine and the framework. /// /// It communicates with the RuntimeController through the use of a /// PlatformConfigurationClient interface, which the @@ -315,7 +314,9 @@ class PlatformConfiguration final { /// @param[in] view_id The ID of the new view. /// @param[in] viewport_metrics The initial viewport metrics for the view. /// - void AddView(int64_t view_id, const ViewportMetrics& view_metrics); + /// @return Whether the view was added. + /// + bool AddView(int64_t view_id, const ViewportMetrics& view_metrics); //---------------------------------------------------------------------------- /// @brief Notify the framework that a view is no longer available. diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index b967b9cde9912..b336b3dde6c83 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -121,12 +121,30 @@ bool RuntimeController::FlushRuntimeStateToIsolate() { FML_DCHECK(!has_flushed_runtime_state_) << "FlushRuntimeStateToIsolate is called more than once somehow."; has_flushed_runtime_state_ = true; + + auto platform_configuration = GetPlatformConfigurationIfAvailable(); + if (!platform_configuration) { + return false; + } + for (auto const& [view_id, viewport_metrics] : platform_data_.viewport_metrics_for_views) { - if (!AddView(view_id, viewport_metrics)) { - return false; + bool added = platform_configuration->AddView(view_id, viewport_metrics); + + // Callbacks will have been already invoked if the engine was restarted. + if (pending_add_view_callbacks_.find(view_id) != + pending_add_view_callbacks_.end()) { + pending_add_view_callbacks_[view_id](added); + pending_add_view_callbacks_.erase(view_id); + } + + if (!added) { + FML_LOG(ERROR) << "Failed to flush view #" << view_id + << ". The Dart isolate may be in an inconsistent state."; } } + + FML_DCHECK(pending_add_view_callbacks_.empty()); return SetLocales(platform_data_.locale_data) && SetSemanticsEnabled(platform_data_.semantics_enabled) && SetAccessibilityFeatures( @@ -136,25 +154,53 @@ bool RuntimeController::FlushRuntimeStateToIsolate() { SetDisplays(platform_data_.displays); } -bool RuntimeController::AddView(int64_t view_id, - const ViewportMetrics& view_metrics) { - platform_data_.viewport_metrics_for_views[view_id] = view_metrics; - if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { - platform_configuration->AddView(view_id, view_metrics); +void RuntimeController::AddView(int64_t view_id, + const ViewportMetrics& view_metrics, + AddViewCallback callback) { + // If the Dart isolate is not running, |FlushRuntimeStateToIsolate| will + // add the view and invoke the callback when the isolate is started. + auto* platform_configuration = GetPlatformConfigurationIfAvailable(); + if (!platform_configuration) { + FML_DCHECK(has_flushed_runtime_state_ == false); + + if (pending_add_view_callbacks_.find(view_id) != + pending_add_view_callbacks_.end()) { + FML_LOG(ERROR) << "View #" << view_id << " is already pending creation."; + callback(false); + return; + } - return true; + platform_data_.viewport_metrics_for_views[view_id] = view_metrics; + pending_add_view_callbacks_[view_id] = std::move(callback); + return; } - return false; + FML_DCHECK(has_flushed_runtime_state_ || pending_add_view_callbacks_.empty()); + + platform_data_.viewport_metrics_for_views[view_id] = view_metrics; + bool added = platform_configuration->AddView(view_id, view_metrics); + callback(added); } bool RuntimeController::RemoveView(int64_t view_id) { platform_data_.viewport_metrics_for_views.erase(view_id); - if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { - return platform_configuration->RemoveView(view_id); + + // If the Dart isolate has not been launched yet, the pending + // add view operation's callback is stored by the runtime controller. + // Notify this callback of the cancellation. + auto* platform_configuration = GetPlatformConfigurationIfAvailable(); + if (!platform_configuration) { + FML_DCHECK(has_flushed_runtime_state_ == false); + if (pending_add_view_callbacks_.find(view_id) != + pending_add_view_callbacks_.end()) { + pending_add_view_callbacks_[view_id](false); + pending_add_view_callbacks_.erase(view_id); + } + + return false; } - return false; + return platform_configuration->RemoveView(view_id); } bool RuntimeController::SetViewportMetrics(int64_t view_id, diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index cb1618f9d3326..8bb01efecabc5 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -50,6 +50,16 @@ class Window; /// class RuntimeController : public PlatformConfigurationClient { public: + /// A callback that's invoked after this `RuntimeController` attempts to + /// add a view to the Dart isolate. + /// + /// If the Dart isolate is not launched yet, this callback will be stored + /// and invoked after the isolate is launched. + /// + /// The `added` parameter is false if the add operation fails or was + /// cancelled while pending using `RemoveView`. + using AddViewCallback = std::function; + //---------------------------------------------------------------------------- /// @brief Creates a new instance of a runtime controller. This is /// usually only done by the engine instance associated with the @@ -174,23 +184,43 @@ class RuntimeController : public PlatformConfigurationClient { /// /// A view must be added before other methods can refer to it, /// including the implicit view. Adding a view that already exists - /// triggers an assertion. + /// is an error. + /// + /// The `callback` is invoked when the add operation is attempted, + /// failed, or is cancelled. + /// + /// If the isolate is not running, the view add will be queued and + /// flushed to the isolate when it starts. Calling `RemoveView` + /// before the isolate is launched cancels the add operation. + /// /// /// @param[in] view_id The ID of the new view. /// @param[in] viewport_metrics The initial viewport metrics for the view. + /// @param[in] callback Callback that will be invoked after the add + /// operation is attempted or cancelled. /// - bool AddView(int64_t view_id, const ViewportMetrics& view_metrics); + void AddView(int64_t view_id, + const ViewportMetrics& view_metrics, + AddViewCallback callback); //---------------------------------------------------------------------------- /// @brief Notify the isolate that a view is no longer available. /// - /// Removing a view that does not exist triggers an assertion. + /// Views that are added before the isolate is started are + /// queued until the isolate is launched. If one of these + /// "pending" views are removed, the view add is cancelled: + /// the `AddViewCallback` will be invoked with an `added` of + /// false and `RemoveView` will return false. /// /// The implicit view (kFlutterImplicitViewId) should never be /// removed. Doing so triggers an assertion. /// /// @param[in] view_id The ID of the view. /// + /// @return If the remove view operation was forwarded to the running + /// isolate. False if the view does not exist. If the Dart isolate + /// is not running, then the pending view creation (if any) is + /// cancelled and the return value is always false. bool RemoveView(int64_t view_id); //---------------------------------------------------------------------------- @@ -660,6 +690,12 @@ class RuntimeController : public PlatformConfigurationClient { std::shared_ptr(new PlatformIsolateManager()); bool has_flushed_runtime_state_ = false; + // Callbacks when `AddView` was called before the Dart isolate is launched. + // + // These views will be added when `FlushRuntimeStateToIsolate` is called. + // This is no longer used once the Dart isolate starts. + std::unordered_map pending_add_view_callbacks_; + // Tracks the views that have been called `Render` during a frame. // // If all views that have been registered by `AddView` have been called diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 7dd03ed19fb78..ac628eb929465 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -297,8 +297,10 @@ tonic::DartErrorHandleType Engine::GetUIIsolateLastError() { return runtime_controller_->GetLastError(); } -void Engine::AddView(int64_t view_id, const ViewportMetrics& view_metrics) { - runtime_controller_->AddView(view_id, view_metrics); +void Engine::AddView(int64_t view_id, + const ViewportMetrics& view_metrics, + std::function callback) { + runtime_controller_->AddView(view_id, view_metrics, std::move(callback)); } bool Engine::RemoveView(int64_t view_id) { diff --git a/shell/common/engine.h b/shell/common/engine.h index 65660f6124c3f..7819ed675cd9f 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -721,8 +721,12 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// /// @param[in] view_id The ID of the new view. /// @param[in] viewport_metrics The initial viewport metrics for the view. + /// @param[in] callback Callback that will be invoked once + /// the engine attempts to add the view. /// - void AddView(int64_t view_id, const ViewportMetrics& view_metrics); + void AddView(int64_t view_id, + const ViewportMetrics& view_metrics, + std::function callback); //---------------------------------------------------------------------------- /// @brief Notify the Flutter application that a view is no diff --git a/shell/common/engine_animator_unittests.cc b/shell/common/engine_animator_unittests.cc index 331bab77e9a16..12bea41da96d3 100644 --- a/shell/common/engine_animator_unittests.cc +++ b/shell/common/engine_animator_unittests.cc @@ -313,8 +313,10 @@ TEST_F(EngineAnimatorTest, AnimatorAcceptsMultipleRenders) { engine_context->Run(std::move(configuration)); engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}); - engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); + engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); }); native_latch.Wait(); @@ -368,8 +370,10 @@ TEST_F(EngineAnimatorTest, IgnoresOutOfFrameRenders) { std::move(animator)); engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}); - engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); + engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); }); auto configuration = RunConfiguration::InferFromSettings(settings_); @@ -444,7 +448,8 @@ TEST_F(EngineAnimatorTest, IgnoresDuplicateRenders) { std::move(animator)); engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); }); auto configuration = RunConfiguration::InferFromSettings(settings_); @@ -504,7 +509,8 @@ TEST_F(EngineAnimatorTest, AnimatorSubmitsImplicitViewBeforeDrawFrameEnds) { std::move(animator)); engine_context->EngineTaskSync([](Engine& engine) { - engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}); + engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}, + [](bool added) { ASSERT_TRUE(added); }); }); auto configuration = RunConfiguration::InferFromSettings(settings_); @@ -568,7 +574,8 @@ TEST_F(EngineAnimatorTest, AnimatorSubmitWarmUpImplicitView) { engine.ScheduleFrame(true); // Add the implicit view so that the engine recognizes it and that its // metrics is not empty. - engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}); + engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0}, + [](bool added) { ASSERT_TRUE(added); }); }); continuation_ready_latch.Wait(); @@ -634,9 +641,12 @@ TEST_F(EngineAnimatorTest, AnimatorSubmitPartialViewsForWarmUp) { // Schedule a frame to make the animator create a continuation. engine.ScheduleFrame(true); // Add multiple views. - engine.AddView(0, ViewportMetrics{1, 10, 10, 22, 0}); - engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}); - engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}); + engine.AddView(0, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); + engine.AddView(1, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); + engine.AddView(2, ViewportMetrics{1, 10, 10, 22, 0}, + [](bool added) { ASSERT_TRUE(added); }); }); continuation_ready_latch.Wait(); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 0228f2744f72d..8673f4e5c5515 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -754,7 +754,11 @@ bool Shell::Setup(std::unique_ptr platform_view, weak_rasterizer_ = rasterizer_->GetWeakPtr(); weak_platform_view_ = platform_view_->GetWeakPtr(); - engine_->AddView(kFlutterImplicitViewId, ViewportMetrics{}); + // Add the implicit view with empty metrics. + engine_->AddView(kFlutterImplicitViewId, ViewportMetrics{}, [](bool added) { + FML_DCHECK(added) << "Failed to add the implicit view"; + }); + // Setup the time-consuming default font manager right after engine created. if (!settings_.prefetched_default_font_manager) { fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), @@ -2102,7 +2106,9 @@ bool Shell::OnServiceProtocolReloadAssetFonts( return true; } -void Shell::AddView(int64_t view_id, const ViewportMetrics& viewport_metrics) { +void Shell::AddView(int64_t view_id, + const ViewportMetrics& viewport_metrics, + AddViewCallback callback) { TRACE_EVENT0("flutter", "Shell::AddView"); FML_DCHECK(is_set_up_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); @@ -2112,10 +2118,11 @@ void Shell::AddView(int64_t view_id, const ViewportMetrics& viewport_metrics) { task_runners_.GetUITaskRunner()->PostTask([engine = engine_->GetWeakPtr(), // viewport_metrics, // - view_id // + view_id, // + callback = std::move(callback) // ] { if (engine) { - engine->AddView(view_id, viewport_metrics); + engine->AddView(view_id, viewport_metrics, callback); } }); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 3e2da290ed130..a510e141ed6d6 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -133,6 +133,7 @@ class Shell final : public PlatformView::Delegate, const std::shared_ptr& gpu_disabled_switch, impeller::RuntimeStageBackend runtime_stage_type)> EngineCreateCallback; + using AddViewCallback = std::function; using RemoveViewCallback = std::function; //---------------------------------------------------------------------------- @@ -317,8 +318,12 @@ class Shell final : public PlatformView::Delegate, /// /// @param[in] view_id The view ID of the new view. /// @param[in] viewport_metrics The initial viewport metrics for the view. + /// @param[in] callback The callback that's invoked once the engine + /// has attempted to add the view. /// - void AddView(int64_t view_id, const ViewportMetrics& viewport_metrics); + void AddView(int64_t view_id, + const ViewportMetrics& viewport_metrics, + AddViewCallback callback); /// @brief Deallocates resources for a non-implicit view. /// diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index c145f2b8b6da7..0046376bd0ca4 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4504,8 +4504,10 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), - [&shell] { shell->AddView(2, ViewportMetrics{}); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { + shell->AddView(2, ViewportMetrics{}, + [](bool added) { EXPECT_TRUE(added); }); + }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); @@ -4519,8 +4521,10 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), - [&shell] { shell->AddView(4, ViewportMetrics{}); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { + shell->AddView(4, ViewportMetrics{}, + [](bool added) { EXPECT_TRUE(added); }); + }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); @@ -4530,6 +4534,122 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { DestroyShell(std::move(shell), task_runners); } +// Test that add view fails if the view ID already exists. +TEST_F(ShellTest, ShellCannotAddDuplicateViewId) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + Settings settings = CreateSettingsForFixture(); + ThreadHost thread_host(ThreadHost::ThreadHostConfig( + "io.flutter.test." + GetCurrentTestName() + ".", + ThreadHost::Type::kPlatform | ThreadHost::Type::kRaster | + ThreadHost::Type::kIo | ThreadHost::Type::kUi)); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + std::unique_ptr shell = CreateShell(settings, task_runners); + ASSERT_TRUE(shell); + + bool has_implicit_view; + std::vector view_ids; + fml::AutoResetWaitableEvent report_latch; + AddNativeCallback("NativeReportViewIdsCallback", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + ParseViewIdsCallback(args, &has_implicit_view, &view_ids); + report_latch.Signal(); + })); + + PlatformViewNotifyCreated(shell.get()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("testReportViewIds"); + RunEngine(shell.get(), std::move(configuration)); + + report_latch.Wait(); + ASSERT_TRUE(has_implicit_view); + ASSERT_EQ(view_ids.size(), 1u); + ASSERT_EQ(view_ids[0], kImplicitViewId); + + // Add view 123. + fml::AutoResetWaitableEvent add_latch; + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell, &add_latch] { + shell->AddView(123, ViewportMetrics{}, [&](bool added) { + EXPECT_TRUE(added); + add_latch.Signal(); + }); + }); + + add_latch.Wait(); + + report_latch.Wait(); + ASSERT_EQ(view_ids.size(), 2u); + ASSERT_EQ(view_ids[0], kImplicitViewId); + ASSERT_EQ(view_ids[1], 123); + + // Attempt to add duplicate view ID 123. This should fail. + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell, &add_latch] { + shell->AddView(123, ViewportMetrics{}, [&](bool added) { + EXPECT_FALSE(added); + add_latch.Signal(); + }); + }); + + add_latch.Wait(); + + PlatformViewNotifyDestroyed(shell.get()); + DestroyShell(std::move(shell), task_runners); +} + +// Test that remove view fails if the view ID does not exist. +TEST_F(ShellTest, ShellCannotRemoveNonexistentId) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + Settings settings = CreateSettingsForFixture(); + ThreadHost thread_host(ThreadHost::ThreadHostConfig( + "io.flutter.test." + GetCurrentTestName() + ".", + ThreadHost::Type::kPlatform | ThreadHost::Type::kRaster | + ThreadHost::Type::kIo | ThreadHost::Type::kUi)); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + std::unique_ptr shell = CreateShell(settings, task_runners); + ASSERT_TRUE(shell); + + bool has_implicit_view; + std::vector view_ids; + fml::AutoResetWaitableEvent report_latch; + AddNativeCallback("NativeReportViewIdsCallback", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + ParseViewIdsCallback(args, &has_implicit_view, &view_ids); + report_latch.Signal(); + })); + + PlatformViewNotifyCreated(shell.get()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("testReportViewIds"); + RunEngine(shell.get(), std::move(configuration)); + + report_latch.Wait(); + ASSERT_TRUE(has_implicit_view); + ASSERT_EQ(view_ids.size(), 1u); + ASSERT_EQ(view_ids[0], kImplicitViewId); + + // Remove view 123. This should fail as this view doesn't exist. + fml::AutoResetWaitableEvent remove_latch; + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell, &remove_latch] { + shell->RemoveView(123, [&](bool removed) { + EXPECT_FALSE(removed); + remove_latch.Signal(); + }); + }); + + remove_latch.Wait(); + + PlatformViewNotifyDestroyed(shell.get()); + DestroyShell(std::move(shell), task_runners); +} + // Parse the arguments of NativeReportViewWidthsCallback and // store them in viewWidths. static void ParseViewWidthsCallback(const Dart_NativeArguments& args, @@ -4570,7 +4690,8 @@ TEST_F(ShellTest, ShellFlushesPlatformStatesByMain) { // The construtor for ViewportMetrics{_, width, _, _, _} (only the 2nd // argument matters in this test). platform_view->SetViewportMetrics(0, ViewportMetrics{1, 10, 1, 0, 0}); - shell->AddView(1, ViewportMetrics{1, 30, 1, 0, 0}); + shell->AddView(1, ViewportMetrics{1, 30, 1, 0, 0}, + [](bool added) { ASSERT_TRUE(added); }); platform_view->SetViewportMetrics(0, ViewportMetrics{1, 20, 1, 0, 0}); }); @@ -4601,6 +4722,112 @@ TEST_F(ShellTest, ShellFlushesPlatformStatesByMain) { DestroyShell(std::move(shell), task_runners); } +// A view can be added and removed before the Dart isolate is launched. +TEST_F(ShellTest, CanRemoveViewBeforeLaunchingIsolate) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + Settings settings = CreateSettingsForFixture(); + ThreadHost thread_host(ThreadHost::ThreadHostConfig( + "io.flutter.test." + GetCurrentTestName() + ".", + ThreadHost::Type::kPlatform | ThreadHost::Type::kRaster | + ThreadHost::Type::kIo | ThreadHost::Type::kUi)); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + std::unique_ptr shell = CreateShell(settings, task_runners); + ASSERT_TRUE(shell); + + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { + auto platform_view = shell->GetPlatformView(); + + // A view can be added and removed all before the isolate launches. + // The pending add view operation is cancelled, the view is never + // added to the Dart isolate. + shell->AddView(123, ViewportMetrics{1, 30, 1, 0, 0}, + [](bool added) { ASSERT_FALSE(added); }); + shell->RemoveView(123, [](bool removed) { ASSERT_FALSE(removed); }); + }); + + bool first_report = true; + std::map view_widths; + fml::AutoResetWaitableEvent report_latch; + AddNativeCallback("NativeReportViewWidthsCallback", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + EXPECT_TRUE(first_report); + first_report = false; + ParseViewWidthsCallback(args, &view_widths); + report_latch.Signal(); + })); + + PlatformViewNotifyCreated(shell.get()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("testReportViewWidths"); + RunEngine(shell.get(), std::move(configuration)); + + report_latch.Wait(); + EXPECT_EQ(view_widths.size(), 1u); + + PlatformViewNotifyDestroyed(shell.get()); + DestroyShell(std::move(shell), task_runners); +} + +// Ensure pending "add views" failures are properly flushed when the Dart +// isolate is launched. +TEST_F(ShellTest, IgnoresBadAddViewsBeforeLaunchingIsolate) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + Settings settings = CreateSettingsForFixture(); + ThreadHost thread_host(ThreadHost::ThreadHostConfig( + "io.flutter.test." + GetCurrentTestName() + ".", + ThreadHost::Type::kPlatform | ThreadHost::Type::kRaster | + ThreadHost::Type::kIo | ThreadHost::Type::kUi)); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + std::unique_ptr shell = CreateShell(settings, task_runners); + ASSERT_TRUE(shell); + + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { + auto platform_view = shell->GetPlatformView(); + + // Add the same view twice. The second time should fail. + shell->AddView(123, ViewportMetrics{1, 100, 1, 0, 0}, + [](bool added) { ASSERT_TRUE(added); }); + + shell->AddView(123, ViewportMetrics{1, 200, 1, 0, 0}, + [](bool added) { ASSERT_FALSE(added); }); + + // Add another view. Previous failures should not affect this. + shell->AddView(456, ViewportMetrics{1, 300, 1, 0, 0}, + [](bool added) { ASSERT_TRUE(added); }); + }); + + bool first_report = true; + std::map view_widths; + fml::AutoResetWaitableEvent report_latch; + AddNativeCallback("NativeReportViewWidthsCallback", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + EXPECT_TRUE(first_report); + first_report = false; + ParseViewWidthsCallback(args, &view_widths); + report_latch.Signal(); + })); + + PlatformViewNotifyCreated(shell.get()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("testReportViewWidths"); + RunEngine(shell.get(), std::move(configuration)); + + report_latch.Wait(); + EXPECT_EQ(view_widths.size(), 3u); + EXPECT_EQ(view_widths[0], 0); + EXPECT_EQ(view_widths[123], 100); + EXPECT_EQ(view_widths[456], 300); + + PlatformViewNotifyDestroyed(shell.get()); + DestroyShell(std::move(shell), task_runners); +} + TEST_F(ShellTest, RuntimeStageBackendDefaultsToSkSLWithoutImpeller) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); Settings settings = CreateSettingsForFixture();