macOS: Fix platform view threading issues

Author: knopp

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

@@ -60,7 +60,9 @@ class FlutterCompositor {
 
   // Callback triggered at the end of the Present function. has_flutter_content
   // is true when Flutter content was rendered, otherwise false.
-  using PresentCallback = std::function<bool(bool has_flutter_content)>;
+  using PresentCallback =
+      std::function<bool(bool has_flutter_content,
+                         dispatch_block_t platfrom_thread_notify)>;
 
   // Registers a callback to be triggered at the end of the Present function.
   // If a callback was previously registered, it will be replaced.
@@ -85,18 +87,22 @@ class FlutterCompositor {
   void SetFrameStatus(FrameStatus frame_status);
   FrameStatus GetFrameStatus();
 
-  // Clears the previous CALayers and updates the frame status to frame started.
+  // Sets frame status to frame started.
   void StartFrame();
 
+  // Clears the previous CALayers.
+  void RemoveOldLayers();
+
   // Calls the present callback and ensures the frame status is updated
   // to frame ended, returning whether the present was successful or not.
-  bool EndFrame(bool has_flutter_content);
+  bool EndFrame(bool has_flutter_content, dispatch_block_t on_notify);
 
   // Creates a CALayer object which is backed by the supplied IOSurface, and
   // adds it to the root CALayer for the given view.
   void InsertCALayerForIOSurface(
       FlutterView* view,
       const IOSurfaceRef& io_surface,
+      size_t layer_position,
       CATransform3D transform = CATransform3DIdentity);
 
  private:

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

@@ -90,6 +90,12 @@
   SetFrameStatus(FrameStatus::kPresenting);
 
   bool has_flutter_content = false;
+
+  std::vector<dispatch_block_t> actions;
+  actions.push_back(^{
+    RemoveOldLayers();
+  });
+
   for (size_t i = 0; i < layers_count; ++i) {
     const auto* layer = layers[i];
     FlutterBackingStore* backing_store = const_cast<FlutterBackingStore*>(layer->backing_store);
@@ -100,19 +106,27 @@
           FlutterIOSurfaceHolder* io_surface_holder =
               (__bridge FlutterIOSurfaceHolder*)backing_store->metal.texture.user_data;
           IOSurfaceRef io_surface = [io_surface_holder ioSurface];
-          InsertCALayerForIOSurface(view, io_surface);
+          actions.push_back(^{
+            InsertCALayerForIOSurface(view, io_surface, i);
+          });
         }
         has_flutter_content = true;
         break;
       }
       case kFlutterLayerContentTypePlatformView: {
-        PresentPlatformView(view, layer, i);
+        actions.push_back(^{
+          PresentPlatformView(view, layer, i);
+        });
         break;
       }
     };
   }
 
-  return EndFrame(has_flutter_content);
+  return EndFrame(has_flutter_content, ^{
+    for (auto& action : actions) {
+      action();
+    }
+  });
 }
 
 void FlutterCompositor::PresentPlatformView(FlutterView* default_base_view,
@@ -143,18 +157,21 @@
 }
 
 void FlutterCompositor::StartFrame() {
+  SetFrameStatus(FrameStatus::kStarted);
+}
+
+void FlutterCompositor::RemoveOldLayers() {
   // First remove all CALayers from the superlayer.
   for (auto layer : active_ca_layers_) {
     [layer removeFromSuperlayer];
   }
 
   // Reset active layers.
   active_ca_layers_.clear();
-  SetFrameStatus(FrameStatus::kStarted);
 }
 
-bool FlutterCompositor::EndFrame(bool has_flutter_content) {
-  bool status = present_callback_(has_flutter_content);
+bool FlutterCompositor::EndFrame(bool has_flutter_content, dispatch_block_t on_notify) {
+  bool status = present_callback_(has_flutter_content, on_notify);
   SetFrameStatus(FrameStatus::kEnded);
   return status;
 }
@@ -173,14 +190,15 @@
 
 void FlutterCompositor::InsertCALayerForIOSurface(FlutterView* view,
                                                   const IOSurfaceRef& io_surface,
+                                                  size_t layer_position,
                                                   CATransform3D transform) {
   // FlutterCompositor manages the lifecycle of CALayers.
   CALayer* content_layer = [[CALayer alloc] init];
   content_layer.transform = transform;
   content_layer.frame = view.layer.bounds;
   [content_layer setContents:(__bridge id)io_surface];
   [view.layer addSublayer:content_layer];
-
+  content_layer.zPosition = layer_position;
   active_ca_layers_.push_back(content_layer);
 }
 

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

@@ -67,7 +67,7 @@ - (nullable FlutterView*)getView:(uint64_t)viewId {
                                           /*mtl_device*/ nullptr);
 
   bool flag = false;
-  macos_compositor->SetPresentCallback([f = &flag](bool has_flutter_content) {
+  macos_compositor->SetPresentCallback([f = &flag](bool has_flutter_content, dispatch_block_t) {
     *f = true;
     return true;
   });

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

@@ -423,18 +423,19 @@ - (FlutterCompositor*)createFlutterCompositor {
 
   _macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
       _viewProvider, _platformViewController, _renderer.device);
-  _macOSCompositor->SetPresentCallback([weakSelf](bool has_flutter_content) {
-    // TODO(dkwingsmt): The compositor only supports single-view for now. As
-    // more classes are gradually converted to multi-view, it should get the
-    // view ID from somewhere.
-    uint64_t viewId = kFlutterDefaultViewId;
-    if (has_flutter_content) {
-      return [weakSelf.renderer present:viewId] == YES;
-    } else {
-      [weakSelf.renderer presentWithoutContent:viewId];
-      return true;
-    }
-  });
+  _macOSCompositor->SetPresentCallback(
+      [weakSelf](bool has_flutter_content, dispatch_block_t on_notify) {
+        // TODO(dkwingsmt): The compositor only supports single-view for now. As
+        // more classes are gradually converted to multi-view, it should get the
+        // view ID from somewhere.
+        uint64_t viewId = kFlutterDefaultViewId;
+        if (has_flutter_content) {
+          return [weakSelf.renderer present:viewId withBlock:on_notify] == YES;
+        } else {
+          [weakSelf.renderer presentWithoutContent:viewId];
+          return true;
+        }
+      });
 
   _compositor = {};
   _compositor.struct_size = sizeof(FlutterCompositor);

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

@@ -465,7 +465,7 @@ @interface FlutterEngine (Test)
   // Latch to ensure the entire layer tree has been generated and presented.
   fml::AutoResetWaitableEvent latch;
   auto compositor = engine.macOSCompositor;
-  compositor->SetPresentCallback([&](bool has_flutter_content) {
+  compositor->SetPresentCallback([&](bool has_flutter_content, dispatch_block_t completion) {
     latch.Signal();
     return true;
   });

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

@@ -38,7 +38,7 @@
 /**
  * Called by the engine when the given view's buffers should be swapped.
  */
-- (BOOL)present:(uint64_t)viewId;
+- (BOOL)present:(uint64_t)viewId withBlock:(nullable dispatch_block_t)block;
 
 /**
  * Tells the renderer that there is no Flutter content available for this frame.

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

@@ -28,7 +28,7 @@ static bool OnPresentDrawableOfDefaultView(FlutterEngine* engine,
   // operates on the default view. To support multi-view, we need a new callback
   // that also receives a view ID.
   uint64_t viewId = kFlutterDefaultViewId;
-  return [engine.renderer present:viewId];
+  return [engine.renderer present:viewId withBlock:nil];
 }
 
 static bool OnAcquireExternalTexture(FlutterEngine* engine,
@@ -104,12 +104,12 @@ - (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size {
   return embedderTexture;
 }
 
-- (BOOL)present:(uint64_t)viewId {
+- (BOOL)present:(uint64_t)viewId withBlock:(dispatch_block_t)block {
   FlutterView* view = [_viewProvider getView:viewId];
   if (view == nil) {
     return NO;
   }
-  [view present];
+  [view presentWithBlock:block];
   return YES;
 }
 

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

@@ -39,8 +39,8 @@ void SetEngineDefaultView(FlutterEngine* engine, id flutterView) {
   FlutterRenderer* renderer = [[FlutterRenderer alloc] initWithFlutterEngine:engine];
   id mockFlutterView = OCMClassMock([FlutterView class]);
   SetEngineDefaultView(engine, mockFlutterView);
-  [(FlutterView*)[mockFlutterView expect] present];
-  [renderer present:kFlutterDefaultViewId];
+  [(FlutterView*)[mockFlutterView expect] presentWithBlock:nil];
+  [renderer present:kFlutterDefaultViewId withBlock:nil];
 }
 
 TEST(FlutterRenderer, TextureReturnedByFlutterView) {

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

@@ -75,6 +75,15 @@
  */
 - (void)requestCommit;
 
+/**
+ * Called from rasterizer thread, will block until delegate resizeSynchronizerCommit:
+ * method is called (on platform thread).
+ *
+ * The onCommit block will be invoked on platform thread during the core
+ * animation transaction.
+ */
+- (void)requestCommitWithBlock:(nullable dispatch_block_t)onCommit;
+
 /**
  * Called from view to notify the synchronizer that there are no Flutter frames
  * coming. Synchronizer must unblock main thread and not block until another

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

@@ -4,6 +4,7 @@
 
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.h"
 
+#import <QuartzCore/QuartzCore.h>
 #include <mutex>
 
 @interface FlutterResizeSynchronizer () {
@@ -38,6 +39,8 @@ @interface FlutterResizeSynchronizer () {
   BOOL _shuttingDown;
 
   __weak id<FlutterResizeSynchronizerDelegate> _delegate;
+
+  dispatch_block_t _onCommit;
 }
 @end
 
@@ -83,7 +86,16 @@ - (void)beginResize:(CGSize)size notify:(dispatch_block_t)notify {
   _condBlockRequestCommit.wait(lock, [&] { return _pendingCommit || _shuttingDown; });
 
   [_delegate resizeSynchronizerFlush:self];
+
+  [CATransaction begin];
+  [CATransaction setDisableActions:YES];
   [_delegate resizeSynchronizerCommit:self];
+  if (_onCommit) {
+    _onCommit();
+  }
+  _onCommit = nil;
+  [CATransaction commit];
+
   _pendingCommit = NO;
   _condBlockBeginResize.notify_all();
 
@@ -106,6 +118,10 @@ - (BOOL)shouldEnsureSurfaceForSize:(CGSize)size {
 }
 
 - (void)requestCommit {
+  [self requestCommitWithBlock:nil];
+}
+
+- (void)requestCommitWithBlock:(dispatch_block_t)onCommit {
   std::unique_lock<std::mutex> lock(_mutex);
 
   if (!_acceptingCommit || _shuttingDown) {
@@ -116,18 +132,25 @@ - (void)requestCommit {
 
   _pendingCommit = YES;
   if (_waiting) {  // BeginResize is in progress, interrupt it and schedule commit call
+    _onCommit = onCommit;
     _condBlockRequestCommit.notify_all();
     _condBlockBeginResize.wait(lock, [&]() { return !_pendingCommit || _shuttingDown; });
   } else {
     // No resize, schedule commit on platform thread and wait until either done
     // or interrupted by incoming BeginResize
     [_delegate resizeSynchronizerFlush:self];
-    dispatch_async(dispatch_get_main_queue(), [self, cookie = _cookie] {
+    dispatch_async(dispatch_get_main_queue(), [self, cookie = _cookie, onCommit] {
       std::unique_lock<std::mutex> lock(_mutex);
       if (cookie == _cookie) {
+        [CATransaction begin];
+        [CATransaction setDisableActions:YES];
         if (_delegate) {
           [_delegate resizeSynchronizerCommit:self];
         }
+        if (onCommit) {
+          onCommit();
+        }
+        [CATransaction commit];
         _pendingCommit = NO;
         _condBlockBeginResize.notify_all();
       }