Add completion callback to Shell::AddView

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<tonic::DartState> 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) {

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.

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,

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<void(bool added)>;
+
   //----------------------------------------------------------------------------
   /// @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<PlatformIsolateManager>(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<int64_t, AddViewCallback> 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

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<void(bool added)> callback) {
+  runtime_controller_->AddView(view_id, view_metrics, std::move(callback));
 }
 
 bool Engine::RemoveView(int64_t view_id) {

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<void(bool added)> callback);
 
   //----------------------------------------------------------------------------
   /// @brief      Notify the Flutter application that a view is no

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();

shell/common/shell.cc

@@ -754,7 +754,11 @@ bool Shell::Setup(std::unique_ptr<PlatformView> 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);
     }
   });
 }

shell/common/shell.h

@@ -133,6 +133,7 @@ class Shell final : public PlatformView::Delegate,
       const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
       impeller::RuntimeStageBackend runtime_stage_type)>
       EngineCreateCallback;
+  using AddViewCallback = std::function<void(bool added)>;
   using RemoveViewCallback = std::function<void(bool removed)>;
 
   //----------------------------------------------------------------------------
@@ -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.
   ///