flutter · auto-submit · Mar 18, 2024 · Mar 13, 2024 · Mar 14, 2024 · Mar 18, 2024
diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc
@@ -121,26 +121,29 @@ void PlatformConfiguration::AddView(int64_t view_id,
       }));
 }
 
-void PlatformConfiguration::RemoveView(int64_t view_id) {
+bool PlatformConfiguration::RemoveView(int64_t view_id) {
   if (view_id == kFlutterImplicitViewId) {
     FML_LOG(ERROR) << "The implicit view #" << view_id << " cannot be removed.";
-    FML_DCHECK(false);
-    return;
+    FML_UNREACHABLE();
+    return false;
   }
   size_t erased_elements = metrics_.erase(view_id);
-  FML_DCHECK(erased_elements != 0) << "View #" << view_id << " doesn't exist.";
-  (void)erased_elements;  // Suppress unused variable warning
+  if (erased_elements == 0) {
+    FML_LOG(ERROR) << "View #" << view_id << " doesn't exist.";
+    return false;
+  }
 
   std::shared_ptr<tonic::DartState> dart_state =
       remove_view_.dart_state().lock();
   if (!dart_state) {
-    return;
+    return false;
   }
   tonic::DartState::Scope scope(dart_state);
   tonic::CheckAndHandleError(
       tonic::DartInvoke(remove_view_.Get(), {
                                                 tonic::ToDart(view_id),
                                             }));
+  return true;
 }
 
 bool PlatformConfiguration::UpdateViewMetrics(

diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h
@@ -327,7 +327,9 @@ class PlatformConfiguration final {
   ///
   /// @param[in]  view_id  The ID of the view.
   ///
-  void RemoveView(int64_t view_id);
+  /// @return     Whether the view was removed.
+  ///
+  bool RemoveView(int64_t view_id);
 
   //----------------------------------------------------------------------------
   /// @brief      Update the view metrics for the specified view.

diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc
@@ -151,8 +151,7 @@ bool RuntimeController::AddView(int64_t view_id,
 bool RuntimeController::RemoveView(int64_t view_id) {
   platform_data_.viewport_metrics_for_views.erase(view_id);
   if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
-    platform_configuration->RemoveView(view_id);
-    return true;
+    return platform_configuration->RemoveView(view_id);
   }
 
   return false;

diff --git a/shell/common/engine.cc b/shell/common/engine.cc
@@ -301,8 +301,8 @@ void Engine::AddView(int64_t view_id, const ViewportMetrics& view_metrics) {
   runtime_controller_->AddView(view_id, view_metrics);
 }
 
-void Engine::RemoveView(int64_t view_id) {
-  runtime_controller_->RemoveView(view_id);
+bool Engine::RemoveView(int64_t view_id) {
+  return runtime_controller_->RemoveView(view_id);
 }
 
 void Engine::SetViewportMetrics(int64_t view_id,

diff --git a/shell/common/engine.h b/shell/common/engine.h
@@ -735,7 +735,9 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
   ///
   /// @param[in]  view_id  The ID of the view.
   ///
-  void RemoveView(int64_t view_id);
+  /// @return     Whether the view was removed.
+  ///
+  bool RemoveView(int64_t view_id);
 
   //----------------------------------------------------------------------------
   /// @brief      Updates the viewport metrics for a view. The viewport metrics

diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -2120,7 +2120,7 @@ void Shell::AddView(int64_t view_id, const ViewportMetrics& viewport_metrics) {
   });
 }
 
-void Shell::RemoveView(int64_t view_id) {
+void Shell::RemoveView(int64_t view_id, RemoveViewCallback callback) {
   TRACE_EVENT0("flutter", "Shell::RemoveView");
   FML_DCHECK(is_set_up_);
   FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread());
@@ -2133,10 +2133,12 @@ void Shell::RemoveView(int64_t view_id) {
       [&task_runners = task_runners_,           //
        engine = engine_->GetWeakPtr(),          //
        rasterizer = rasterizer_->GetWeakPtr(),  //
-       view_id                                  //
+       view_id,                                 //
+       callback = std::move(callback)           //
   ] {
         if (engine) {
-          engine->RemoveView(view_id);
+          bool removed = engine->RemoveView(view_id);
+          callback(removed);
         }
         // Don't wait for the raster task here, which only cleans up memory and
         // does not affect functionality. Make sure it is done after Dart

diff --git a/shell/common/shell.h b/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 RemoveViewCallback = std::function<void(bool removed)>;
 
   //----------------------------------------------------------------------------
   /// @brief      Creates a shell instance using the provided settings. The
@@ -330,8 +331,10 @@ class Shell final : public PlatformView::Delegate,
   ///         `kFlutterImplicitViewId` triggers an assertion.
   ///
   /// @param[in]  view_id     The view ID of the view to be removed.
+  /// @param[in]  callback    The callback that's invoked once the engine has
+  ///                         attempted to remove the view.
   ///
-  void RemoveView(int64_t view_id);
+  void RemoveView(int64_t view_id, RemoveViewCallback callback);
 
   //----------------------------------------------------------------------------
   /// @brief      Captures a screenshot and optionally Base64 encodes the data

diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc
@@ -4511,8 +4511,9 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) {
   ASSERT_EQ(viewIds.size(), 2u);
   ASSERT_EQ(viewIds[1], 2ll);
 
-  PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(),
-           [&shell] { shell->RemoveView(2); });
+  PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] {
+    shell->RemoveView(2, [](bool removed) { ASSERT_TRUE(removed); });
+  });
   reportLatch.Wait();
   ASSERT_TRUE(hasImplicitView);
   ASSERT_EQ(viewIds.size(), 1u);

diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc
@@ -2111,6 +2111,44 @@ FlutterEngineResult FlutterEngineRunInitialized(
   return kSuccess;
 }
 
+FLUTTER_EXPORT
+FlutterEngineResult FlutterEngineRemoveView(FLUTTER_API_SYMBOL(FlutterEngine)
+                                                engine,
+                                            const FlutterRemoveViewInfo* info) {
+  if (!engine) {
+    return LOG_EMBEDDER_ERROR(kInvalidArguments, "Engine handle was invalid.");
+  }
+  if (!info || !info->remove_view_callback) {
+    return LOG_EMBEDDER_ERROR(kInvalidArguments,
+                              "Remove view info handle was invalid.");
+  }
+
+  if (info->view_id == kFlutterImplicitViewId) {
+    return LOG_EMBEDDER_ERROR(
+        kInvalidArguments,
+        "Remove view info was invalid. The implicit view cannot be removed.");
+  }
+
+  // The engine must be running to remove a view.
+  auto embedder_engine = reinterpret_cast<flutter::EmbedderEngine*>(engine);
+  if (!embedder_engine->IsValid()) {
+    return LOG_EMBEDDER_ERROR(kInvalidArguments, "Engine handle was invalid.");
+  }
+
+  flutter::Shell::RemoveViewCallback callback =
+      [c_callback = info->remove_view_callback,
+       user_data = info->user_data](bool removed) {
+        FlutterRemoveViewResult result = {};
+        result.struct_size = sizeof(FlutterRemoveViewResult);
+        result.removed = removed;
+        result.user_data = user_data;
+        c_callback(&result);
+      };
+
+  embedder_engine->GetShell().RemoveView(info->view_id, callback);
+  return kSuccess;
+}
+
 FLUTTER_EXPORT
 FlutterEngineResult FlutterEngineDeinitialize(FLUTTER_API_SYMBOL(FlutterEngine)
                                                   engine) {

@@ -831,6 +831,53 @@ typedef struct {
   };
 } FlutterRendererConfig;
 
+typedef struct {
+  /// The size of this struct.
+  /// Must be sizeof(FlutterRemoveViewResult).
+  size_t struct_size;
+
+  /// True if the remove view operation succeeded.
+  bool removed;
+
+  /// The |FlutterRemoveViewInfo.user_data|.
+  void* user_data;
+} FlutterRemoveViewResult;
+
+/// The callback invoked by the engine when the engine has attempted to remove
+/// a view.
+///
+/// The |FlutterRemoveViewResult| will be deallocated once the callback returns.
+typedef void (*FlutterRemoveViewCallback)(
+    const FlutterRemoveViewResult* /* result */);
+
+typedef struct {
+  /// The size of this struct.
+  /// Must be sizeof(FlutterRemoveViewInfo).
+  size_t struct_size;
+
+  /// The identifier for the view to remove.
+  ///
+  /// The implicit view cannot be removed if it is enabled.
+  FlutterViewId view_id;
+
+  /// A baton that is not interpreted by the engine in any way.
+  /// It will be given back to the embedder in |remove_view_callback|.
+  /// Embedder resources may be associated with this baton.
+  void* user_data;
+
+  /// Called once the engine has attempted to remove the view.
+  /// This callback is required.
+  ///
+  /// The embedder must not destroy the underlying surface until the callback is
+  /// invoked with a `removed` value of `true`.
+  ///
+  /// This callback is invoked on an internal engine managed thread.
+  /// Embedders must re-thread if necessary.
+  ///
+  /// The |result| argument will be deallocated when the callback returns.
+  FlutterRemoveViewCallback remove_view_callback;
+} FlutterRemoveViewInfo;
+
 /// Display refers to a graphics hardware system consisting of a framebuffer,
 /// typically a monitor or a screen. This ID is unique per display and is
 /// stable until the Flutter application restarts.
@@ -2418,6 +2465,25 @@ FLUTTER_EXPORT
 FlutterEngineResult FlutterEngineRunInitialized(
     FLUTTER_API_SYMBOL(FlutterEngine) engine);
 
+//------------------------------------------------------------------------------
+/// @brief      Removes a view.
+///
+///             This is an asynchronous operation. The view's resources must not
+///             be cleaned up until the |remove_view_callback| is invoked with
+///             a |removed| value of `true`.
+///
+/// @param[in]  engine  A running engine instance.
+/// @param[in]  info    The remove view arguments. This can be deallocated
+///                     once |FlutterEngineRemoveView| returns, before
+///                     |remove_view_callback| is invoked.
+///
+/// @return     The result of *starting* the asynchronous operation. If
+///             `kSuccess`, the |remove_view_callback| will be invoked.
+FLUTTER_EXPORT
+FlutterEngineResult FlutterEngineRemoveView(FLUTTER_API_SYMBOL(FlutterEngine)
+                                                engine,
+                                            const FlutterRemoveViewInfo* info);
+
 FLUTTER_EXPORT
 FlutterEngineResult FlutterEngineSendWindowMetricsEvent(
     FLUTTER_API_SYMBOL(FlutterEngine) engine,

diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc
@@ -10,6 +10,7 @@
 
 #include "embedder.h"
 #include "embedder_engine.h"
+#include "flutter/common/constants.h"
 #include "flutter/flow/raster_cache.h"
 #include "flutter/fml/file.h"
 #include "flutter/fml/make_copyable.h"
@@ -1196,6 +1197,50 @@ TEST_F(EmbedderTest, CanDeinitializeAnEngine) {
   engine.reset();
 }
 
+TEST_F(EmbedderTest, CanRemoveView) {
+  // TODO(loicsharma): We can't test this until views can be added!
+  // https://github.com/flutter/flutter/issues/144806
+}
+
+TEST_F(EmbedderTest, CannotRemoveImplicitView) {
+  auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext);
+  EmbedderConfigBuilder builder(context);
+  builder.SetSoftwareRendererConfig();
+
+  auto engine = builder.LaunchEngine();
+  ASSERT_TRUE(engine.is_valid());
+
+  FlutterRemoveViewInfo info = {};
+  info.struct_size = sizeof(FlutterRemoveViewInfo);
+  info.view_id = kFlutterImplicitViewId;
+  info.remove_view_callback = [](const FlutterRemoveViewResult* result) {
+    // Unreachable
+    ASSERT_TRUE(false);
+  };
+  ASSERT_EQ(FlutterEngineRemoveView(engine.get(), &info), kInvalidArguments);
+}
+
+TEST_F(EmbedderTest, CannotRemoveUnknownView) {
+  auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext);
+  EmbedderConfigBuilder builder(context);
+  builder.SetSoftwareRendererConfig();
+
+  auto engine = builder.LaunchEngine();
+  ASSERT_TRUE(engine.is_valid());
+
+  fml::AutoResetWaitableEvent latch;
+  FlutterRemoveViewInfo info = {};
+  info.struct_size = sizeof(FlutterRemoveViewInfo);
+  info.view_id = 123;
+  info.user_data = &latch;
+  info.remove_view_callback = [](const FlutterRemoveViewResult* result) {
+    ASSERT_FALSE(result->removed);
+    reinterpret_cast<fml::AutoResetWaitableEvent*>(result->user_data)->Signal();
+  };
+  ASSERT_EQ(FlutterEngineRemoveView(engine.get(), &info), kSuccess);
+  latch.Wait();
+}
+
 TEST_F(EmbedderTest, CanUpdateLocales) {
   auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext);
   EmbedderConfigBuilder builder(context);