[macOS] Multiview compositor (flutter#52253)

This PR makes the macOS `FlutterCompositor` able to handle multiple views by adding methods `AddView` and `RemoveView`.

These two methods must take place on the main queue (platform thread), which is indeed the case, but extra effort is made to assert so. This is important because, due to the constraint of the macOS system that requires rendering to take place on the platform thread (hence `FlutterThreadSynchronizer`), no locks are needed to avoid race condition between `Add/RemoveView` and presenting.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

shell/platform/darwin/macos/framework/Source/FlutterCompositor.h

@@ -46,6 +46,18 @@ class FlutterCompositor {
 
   ~FlutterCompositor() = default;
 
+  // Allocate the resources for displaying a view.
+  //
+  // This method must be called when a view is added to FlutterEngine, and must be
+  // called on the main dispatch queue, or an assertion will be thrown.
+  void AddView(FlutterViewId view_id);
+
+  // Deallocate the resources for displaying a view.
+  //
+  // This method must be called when a view is removed from FlutterEngine, and
+  // must be called on the main dispatch queue, or an assertion will be thrown.
+  void RemoveView(FlutterViewId view_id);
+
   // Creates a backing store and saves updates the backing_store_out data with
   // the new FlutterBackingStore data.
   //
@@ -65,6 +77,11 @@ class FlutterCompositor {
   // `view_id` using the layer content.
   bool Present(FlutterViewIdentifier view_id, const FlutterLayer** layers, size_t layers_count);
 
+  // The number of views that the FlutterCompositor is keeping track of.
+  //
+  // This method must only be used in unit tests.
+  size_t DebugNumViews();
+
  private:
   // A class that contains the information for a view to be presented.
   class ViewPresenter {
@@ -103,7 +120,8 @@ class FlutterCompositor {
   // The controller used to manage creation and deletion of platform views.
   const FlutterPlatformViewController* platform_view_controller_;
 
-  std::unordered_map<int64_t, ViewPresenter> presenters_;
+  // The view presenters for views. Each key is a view ID.
+  std::unordered_map<FlutterViewId, ViewPresenter> presenters_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(FlutterCompositor);
 };

shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm

@@ -41,12 +41,19 @@
   FML_CHECK(view_provider != nullptr) << "view_provider cannot be nullptr";
 }
 
+void FlutterCompositor::AddView(FlutterViewId view_id) {
+  dispatch_assert_queue(dispatch_get_main_queue());
+  presenters_.try_emplace(view_id);
+}
+
+void FlutterCompositor::RemoveView(FlutterViewId view_id) {
+  dispatch_assert_queue(dispatch_get_main_queue());
+  presenters_.erase(view_id);
+}
+
 bool FlutterCompositor::CreateBackingStore(const FlutterBackingStoreConfig* config,
                                            FlutterBackingStore* backing_store_out) {
-  // TODO(dkwingsmt): This class only supports single-view for now. As more
-  // classes are gradually converted to multi-view, it should get the view ID
-  // from somewhere.
-  FlutterView* view = [view_provider_ viewForIdentifier:kFlutterImplicitViewId];
+  FlutterView* view = [view_provider_ viewForIdentifier:config->view_id];
   if (!view) {
     return false;
   }
@@ -103,18 +110,31 @@
   // the layer information instead of passing the original pointers from embedder.
   auto layers_copy = std::make_shared<std::vector<LayerVariant>>(CopyLayers(layers, layers_count));
 
-  [view.surfaceManager
-      presentSurfaces:surfaces
-               atTime:presentation_time
-               notify:^{
-                 // Gets a presenter or create a new one for the view.
-                 ViewPresenter& presenter = presenters_[view_id];
-                 presenter.PresentPlatformViews(view, *layers_copy, platform_view_controller_);
-               }];
+  [view.surfaceManager presentSurfaces:surfaces
+                                atTime:presentation_time
+                                notify:^{
+                                  // Accessing presenters_ here does not need a
+                                  // lock to avoid race condition against
+                                  // AddView and RemoveView, since all three
+                                  // take place on the platform thread. (The
+                                  // macOS API requires platform view presenting
+                                  // to take place on the platform thread,
+                                  // enforced by `FlutterThreadSynchronizer`.)
+                                  dispatch_assert_queue(dispatch_get_main_queue());
+                                  auto found_presenter = presenters_.find(view_id);
+                                  if (found_presenter != presenters_.end()) {
+                                    found_presenter->second.PresentPlatformViews(
+                                        view, *layers_copy, platform_view_controller_);
+                                  }
+                                }];
 
   return true;
 }
 
+size_t FlutterCompositor::DebugNumViews() {
+  return presenters_.size();
+}
+
 FlutterCompositor::ViewPresenter::ViewPresenter()
     : mutator_views_([NSMapTable strongToStrongObjectsMapTable]) {}
 

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

@@ -528,6 +528,12 @@ - (instancetype)initWithName:(NSString*)labelPrefix
 
   _platformViewController = [[FlutterPlatformViewController alloc] init];
   _threadSynchronizer = [[FlutterThreadSynchronizer alloc] init];
+  // The macOS compositor must be initialized in the initializer because it is
+  // used when adding views, which might happen before runWithEntrypoint.
+  _macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
+      [[FlutterViewEngineProvider alloc] initWithEngine:self],
+      [[FlutterTimeConverter alloc] initWithEngine:self], _platformViewController);
+
   [self setUpPlatformViewChannel];
   [self setUpAccessibilityChannel];
   [self setUpNotificationCenterListeners];
@@ -731,6 +737,7 @@ - (void)loadAOTData:(NSString*)assetsDir {
 
 - (void)registerViewController:(FlutterViewController*)controller
                  forIdentifier:(FlutterViewIdentifier)viewIdentifier {
+  _macOSCompositor->AddView(viewIdentifier);
   NSAssert(controller != nil, @"The controller must not be nil.");
   NSAssert(controller.engine == nil,
            @"The FlutterViewController is unexpectedly attached to "
@@ -785,6 +792,7 @@ - (void)viewControllerViewDidLoad:(FlutterViewController*)viewController {
 }
 
 - (void)deregisterViewControllerForIdentifier:(FlutterViewIdentifier)viewIdentifier {
+  _macOSCompositor->RemoveView(viewIdentifier);
   FlutterViewController* controller = [self viewControllerForIdentifier:viewIdentifier];
   // The controller can be nil. The engine stores only a weak ref, and this
   // method could have been called from the controller's dealloc.
@@ -852,10 +860,6 @@ - (FlutterViewController*)viewController {
 }
 
 - (FlutterCompositor*)createFlutterCompositor {
-  _macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
-      [[FlutterViewEngineProvider alloc] initWithEngine:self],
-      [[FlutterTimeConverter alloc] initWithEngine:self], _platformViewController);
-
   _compositor = {};
   _compositor.struct_size = sizeof(FlutterCompositor);
   _compositor.user_data = _macOSCompositor.get();

shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm

@@ -940,6 +940,31 @@ @implementation MockableFlutterEngine
   EXPECT_EQ(viewController1.viewIdentifier, 0ll);
 }
 
+TEST_F(FlutterEngineTest, RemovingViewDisposesCompositorResources) {
+  NSString* fixtures = @(flutter::testing::GetFixturesPath());
+  FlutterDartProject* project = [[FlutterDartProject alloc]
+      initWithAssetsPath:fixtures
+             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
+  FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:project];
+
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
+  [viewController loadView];
+  [viewController viewDidLoad];
+  viewController.flutterView.frame = CGRectMake(0, 0, 800, 600);
+
+  EXPECT_TRUE([engine runWithEntrypoint:@"drawIntoAllViews"]);
+  [engine.testThreadSynchronizer blockUntilFrameAvailable];
+  EXPECT_EQ(engine.macOSCompositor->DebugNumViews(), 1u);
+
+  engine.viewController = nil;
+  EXPECT_EQ(engine.macOSCompositor->DebugNumViews(), 0u);
+
+  [engine shutDownEngine];
+  engine = nil;
+}
+
 TEST_F(FlutterEngineTest, HandlesTerminationRequest) {
   id engineMock = CreateMockFlutterEngine(nil);
   __block NSString* nextResponse = @"exit";

shell/platform/darwin/macos/framework/Source/fixtures/flutter_desktop_test.dart

@@ -49,6 +49,18 @@ void canCompositePlatformViews() {
   PlatformDispatcher.instance.scheduleFrame();
 }
 
+@pragma('vm:entry-point')
+void drawIntoAllViews() {
+  PlatformDispatcher.instance.onBeginFrame = (Duration duration) {
+    SceneBuilder builder = SceneBuilder();
+    builder.addPicture(Offset(1.0, 1.0), _createSimplePicture());
+    for (final FlutterView view in PlatformDispatcher.instance.views) {
+      view.render(builder.build());
+    }
+  };
+  PlatformDispatcher.instance.scheduleFrame();
+}
+
 /// Returns a [Picture] of a simple black square.
 Picture _createSimplePicture() {
   Paint blackPaint = Paint();