[Windows] Allow adding/removing views

Author: loic-sharma

shell/platform/embedder/embedder.cc

@@ -3375,6 +3375,8 @@ FlutterEngineResult FlutterEngineGetProcAddresses(
   SET_PROC(NotifyDisplayUpdate, FlutterEngineNotifyDisplayUpdate);
   SET_PROC(ScheduleFrame, FlutterEngineScheduleFrame);
   SET_PROC(SetNextFrameCallback, FlutterEngineSetNextFrameCallback);
+  SET_PROC(AddView, FlutterEngineAddView);
+  SET_PROC(RemoveView, FlutterEngineRemoveView);
 #undef SET_PROC
 
   return kSuccess;

shell/platform/embedder/embedder.h

@@ -3287,6 +3287,12 @@ typedef FlutterEngineResult (*FlutterEngineSetNextFrameCallbackFnPtr)(
     FLUTTER_API_SYMBOL(FlutterEngine) engine,
     VoidCallback callback,
     void* user_data);
+typedef FlutterEngineResult (*FlutterEngineAddViewFnPtr)(
+    FLUTTER_API_SYMBOL(FlutterEngine) engine,
+    const FlutterAddViewInfo* info);
+typedef FlutterEngineResult (*FlutterEngineRemoveViewFnPtr)(
+    FLUTTER_API_SYMBOL(FlutterEngine) engine,
+    const FlutterRemoveViewInfo* info);
 
 /// Function-pointer-based versions of the APIs above.
 typedef struct {
@@ -3333,6 +3339,8 @@ typedef struct {
   FlutterEngineNotifyDisplayUpdateFnPtr NotifyDisplayUpdate;
   FlutterEngineScheduleFrameFnPtr ScheduleFrame;
   FlutterEngineSetNextFrameCallbackFnPtr SetNextFrameCallback;
+  FlutterEngineAddViewFnPtr AddView;
+  FlutterEngineRemoveViewFnPtr RemoveView;
 } FlutterEngineProcTable;
 
 //------------------------------------------------------------------------------

shell/platform/windows/fixtures/main.dart

@@ -361,3 +361,12 @@ void signalViewIds() {
 
   signalStringValue('View IDs: [${viewIds.join(', ')}]');
 }
+
+@pragma('vm:entry-point')
+void onMetricsChangedSignalViewIds() {
+  ui.PlatformDispatcher.instance.onMetricsChanged = () {
+    signalViewIds();
+  };
+
+  signal();
+}

shell/platform/windows/flutter_windows.cc

@@ -93,6 +93,10 @@ static FlutterDesktopViewControllerRef CreateViewController(
 
   std::unique_ptr<flutter::FlutterWindowsView> view =
       engine_ptr->CreateView(std::move(window_wrapper));
+  if (!view) {
+    return nullptr;
+  }
+
   auto controller = std::make_unique<flutter::FlutterWindowsViewController>(
       std::move(engine), std::move(view));
 

shell/platform/windows/flutter_windows_engine.cc

@@ -12,6 +12,7 @@
 #include "flutter/fml/logging.h"
 #include "flutter/fml/paths.h"
 #include "flutter/fml/platform/win/wstring_conversion.h"
+#include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/shell/platform/common/client_wrapper/binary_messenger_impl.h"
 #include "flutter/shell/platform/common/client_wrapper/include/flutter/standard_message_codec.h"
 #include "flutter/shell/platform/common/path_utils.h"
@@ -149,6 +150,7 @@ FlutterWindowsEngine::FlutterWindowsEngine(
     : project_(std::make_unique<FlutterProjectBundle>(project)),
       windows_proc_table_(std::move(windows_proc_table)),
       aot_data_(nullptr, nullptr),
+      views_mutex_(fml::SharedMutex::Create()),
       lifecycle_manager_(std::make_unique<WindowsLifecycleManager>(this)) {
   if (windows_proc_table_ == nullptr) {
     windows_proc_table_ = std::make_shared<WindowsProcTable>();
@@ -492,34 +494,118 @@ bool FlutterWindowsEngine::Stop() {
 
 std::unique_ptr<FlutterWindowsView> FlutterWindowsEngine::CreateView(
     std::unique_ptr<WindowBindingHandler> window) {
-  // TODO(loicsharma): Remove implicit view assumption.
-  // https://github.com/flutter/flutter/issues/142845
+  auto view_id = next_view_id_;
   auto view = std::make_unique<FlutterWindowsView>(
-      kImplicitViewId, this, std::move(window), windows_proc_table_);
+      view_id, this, std::move(window), windows_proc_table_);
 
   view->CreateRenderSurface();
 
-  views_[kImplicitViewId] = view.get();
+  next_view_id_++;
+
+  {
+    // Add the view to the embedder. This must happen before the engine
+    // is notified the view exists and starts presenting to it.
+    fml::UniqueLock write_lock{*views_mutex_};
+    FML_DCHECK(views_.find(view_id) == views_.end());
+    views_[view_id] = view.get();
+  }
+
+  if (!view->IsImplicitView()) {
+    FML_DCHECK(running());
+
+    struct Captures {
+      fml::AutoResetWaitableEvent latch;
+      bool added;
+    };
+    Captures captures;
+
+    FlutterWindowMetricsEvent metrics = view->CreateWindowMetricsEvent();
+
+    FlutterAddViewInfo info = {};
+    info.struct_size = sizeof(FlutterAddViewInfo);
+    info.view_id = view_id;
+    info.view_metrics = &metrics;
+    info.user_data = &captures;
+    info.add_view_callback = [](const FlutterAddViewResult* result) {
+      Captures* captures = reinterpret_cast<Captures*>(result->user_data);
+      captures->added = result->added;
+      captures->latch.Signal();
+    };
+
+    embedder_api_.AddView(engine_, &info);
+
+    // Block the platform thread until the engine has added the view.
+    // TODO(loicsharma): This blocks the platform thread eagerly and can
+    // cause unnecessary delay in input processing. Instead, this should block
+    // lazily only when the app does an operation which needs the view.
+    // https://github.com/flutter/flutter/issues/146248
+    captures.latch.Wait();
+
+    if (!captures.added) {
+      // Adding the view failed. Update the embedder's state to match the
+      // engine's state. This is unexpected and indicates a bug in the Windows
+      // embedder.
+      FML_LOG(ERROR) << "FlutterEngineAddView failed to add view";
+      fml::UniqueLock write_lock{*views_mutex_};
+      views_.erase(view_id);
+      return nullptr;
+    }
+  }
 
   return std::move(view);
 }
 
 void FlutterWindowsEngine::RemoveView(FlutterViewId view_id) {
   FML_DCHECK(running());
-  FML_DCHECK(views_.find(view_id) != views_.end());
 
-  if (view_id == kImplicitViewId) {
-    // The engine and framework assume the implicit view always exists.
-    // Attempts to render to the implicit view will be ignored.
-    views_.erase(view_id);
-    return;
+  // Notify the engine to stop rendering to the view if it isn't the implicit
+  // view. The engine and framework assume the implicit view always exists and
+  // can continue presenting.
+  if (view_id != kImplicitViewId) {
+    struct Captures {
+      fml::AutoResetWaitableEvent latch;
+      bool removed;
+    };
+    Captures captures;
+
+    FlutterRemoveViewInfo info = {};
+    info.struct_size = sizeof(FlutterRemoveViewInfo);
+    info.view_id = view_id;
+    info.user_data = &captures;
+    info.remove_view_callback = [](const FlutterRemoveViewResult* result) {
+      // This is invoked on the raster thread, the same thread that the present
+      // callback is invoked. If |FlutterRemoveViewResult.removed| is `true`,
+      // the engine guarantees the view won't be presented.
+      Captures* captures = reinterpret_cast<Captures*>(result->user_data);
+      captures->removed = result->removed;
+      captures->latch.Signal();
+    };
+
+    embedder_api_.RemoveView(engine_, &info);
+
+    // Block the platform thread until the engine has removed the view.
+    // TODO(loicsharma): This blocks the platform thread eagerly and can
+    // cause unnecessary delay in input processing. Instead, this should block
+    // lazily only when an operation needs the view.
+    // https://github.com/flutter/flutter/issues/146248
+    captures.latch.Wait();
+
+    if (!captures.removed) {
+      // Removing the view failed. This is unexpected and indicates a bug in the
+      // Windows embedder.
+      FML_LOG(ERROR) << "FlutterEngineRemoveView failed to remove view";
+      return;
+    }
   }
 
-  // TODO(loicsharma): Remove the view from the engine using the
-  // `FlutterEngineRemoveView` embedder API. Windows does not
-  // support views other than the implicit view yet.
-  // https://github.com/flutter/flutter/issues/144810
-  FML_UNREACHABLE();
+  {
+    // The engine no longer presents to the view. Remove the view from the
+    // embedder.
+    fml::UniqueLock write_lock{*views_mutex_};
+
+    FML_DCHECK(views_.find(view_id) != views_.end());
+    views_.erase(view_id);
+  }
 }
 
 void FlutterWindowsEngine::OnVsync(intptr_t baton) {
@@ -551,6 +637,8 @@ std::chrono::nanoseconds FlutterWindowsEngine::FrameInterval() {
 }
 
 FlutterWindowsView* FlutterWindowsEngine::view(FlutterViewId view_id) const {
+  fml::SharedLock read_lock{*views_mutex_};
+
   auto iterator = views_.find(view_id);
   if (iterator == views_.end()) {
     return nullptr;
@@ -779,6 +867,8 @@ bool FlutterWindowsEngine::DispatchSemanticsAction(
 
 void FlutterWindowsEngine::UpdateSemanticsEnabled(bool enabled) {
   if (engine_ && semantics_enabled_ != enabled) {
+    fml::SharedLock read_lock{*views_mutex_};
+
     semantics_enabled_ = enabled;
     embedder_api_.UpdateSemanticsEnabled(engine_, enabled);
     for (auto iterator = views_.begin(); iterator != views_.end(); iterator++) {
@@ -844,6 +934,8 @@ void FlutterWindowsEngine::OnQuit(std::optional<HWND> hwnd,
 }
 
 void FlutterWindowsEngine::OnDwmCompositionChanged() {
+  fml::SharedLock read_lock{*views_mutex_};
+
   for (auto iterator = views_.begin(); iterator != views_.end(); iterator++) {
     iterator->second->OnDwmCompositionChanged();
   }
@@ -875,6 +967,11 @@ void FlutterWindowsEngine::OnChannelUpdate(std::string name, bool listening) {
 }
 
 bool FlutterWindowsEngine::Present(const FlutterPresentViewInfo* info) {
+  // This runs on the raster thread. Lock the views map for the entirety of the
+  // present operation to block the platform thread from destroying the
+  // view during the present.
+  fml::SharedLock read_lock{*views_mutex_};
+
   auto iterator = views_.find(info->view_id);
   if (iterator == views_.end()) {
     return false;

shell/platform/windows/flutter_windows_engine.h

@@ -16,6 +16,7 @@
 
 #include "flutter/fml/closure.h"
 #include "flutter/fml/macros.h"
+#include "flutter/fml/synchronization/shared_mutex.h"
 #include "flutter/shell/platform/common/accessibility_bridge.h"
 #include "flutter/shell/platform/common/app_lifecycle_state.h"
 #include "flutter/shell/platform/common/client_wrapper/binary_messenger_impl.h"
@@ -121,6 +122,8 @@ class FlutterWindowsEngine {
   virtual bool Stop();
 
   // Create a view that can display this engine's content.
+  //
+  // Returns null on failure.
   std::unique_ptr<FlutterWindowsView> CreateView(
       std::unique_ptr<WindowBindingHandler> window);
 
@@ -359,9 +362,30 @@ class FlutterWindowsEngine {
   // AOT data, if any.
   UniqueAotDataPtr aot_data_;
 
+  // The ID that the next view will have.
+  FlutterViewId next_view_id_ = kImplicitViewId;
+
   // The views displaying the content running in this engine, if any.
+  //
+  // This is read and mutated by the platform thread. This is read by the raster
+  // thread to present content to a view.
+  //
+  // Reads to this object on non-platform threads must be protected
+  // by acquiring a shared lock on |views_mutex_|.
+  //
+  // Writes to this object must only happen on the platform thread
+  // and must be protected by acquiring an exclusive lock on |views_mutex_|.
   std::unordered_map<FlutterViewId, FlutterWindowsView*> views_;
 
+  // The mutex that protects the |views_| map.
+  //
+  // The raster thread acquires a shared lock to present to a view.
+  //
+  // The platform thread acquires a shared lock to access the view.
+  // The platform thread acquires an exclusive lock before adding
+  // a view to the engine or after removing a view from the engine.
+  std::unique_ptr<fml::SharedMutex> views_mutex_;
+
   // Task runner for tasks posted from the engine.
   std::unique_ptr<TaskRunner> task_runner_;
 

shell/platform/windows/flutter_windows_unittests.cc

@@ -570,5 +570,65 @@ TEST_F(WindowsTest, GetKeyboardStateHeadless) {
   }
 }
 
+// Verify the embedder can add and remove views.
+TEST_F(WindowsTest, AddRemoveView) {
+  std::mutex mutex;
+  std::string view_ids;
+
+  auto& context = GetContext();
+  WindowsConfigBuilder builder(context);
+  builder.SetDartEntrypoint("onMetricsChangedSignalViewIds");
+
+  fml::AutoResetWaitableEvent ready_latch;
+  context.AddNativeFunction(
+      "Signal", CREATE_NATIVE_ENTRY(
+                    [&](Dart_NativeArguments args) { ready_latch.Signal(); }));
+
+  context.AddNativeFunction(
+      "SignalStringValue", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) {
+        auto handle = Dart_GetNativeArgument(args, 0);
+        ASSERT_FALSE(Dart_IsError(handle));
+
+        std::scoped_lock lock{mutex};
+        view_ids = tonic::DartConverter<std::string>::FromDart(handle);
+      }));
+
+  // Create the implicit view.
+  ViewControllerPtr first_controller{builder.Run()};
+  ASSERT_NE(first_controller, nullptr);
+
+  ready_latch.Wait();
+
+  // Create a second view.
+  FlutterDesktopEngineRef engine =
+      FlutterDesktopViewControllerGetEngine(first_controller.get());
+  FlutterDesktopViewControllerProperties properties = {};
+  properties.width = 100;
+  properties.height = 100;
+  ViewControllerPtr second_controller{
+      FlutterDesktopEngineCreateViewController(engine, &properties)};
+  ASSERT_NE(second_controller, nullptr);
+
+  // Pump messages for the Windows platform task runner until the view is added.
+  while (true) {
+    PumpMessage();
+    std::scoped_lock lock{mutex};
+    if (view_ids == "View IDs: [0, 1]") {
+      break;
+    }
+  }
+
+  // Delete the second view and pump messages for the Windows platform task
+  // runner until the view is removed.
+  second_controller.reset();
+  while (true) {
+    PumpMessage();
+    std::scoped_lock lock{mutex};
+    if (view_ids == "View IDs: [0]") {
+      break;
+    }
+  }
+}
+
 }  // namespace testing
 }  // namespace flutter