macOS: Fix platform view threading issues

Author: knopp

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

@@ -55,7 +55,9 @@ class FlutterCompositor {
   // Present sets frame_started_ to false.
   bool Present(const FlutterLayer** layers, size_t layers_count);
 
-  using PresentCallback = std::function<bool(bool has_flutter_content)>;
+  using PresentCallback =
+      std::function<bool(bool has_flutter_content,
+                         dispatch_block_t platfrom_thread_notify)>;
 
   // PresentCallback is called at the end of the Present function.
   void SetPresentCallback(const PresentCallback& present_callback);
@@ -80,18 +82,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

@@ -81,6 +81,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);
@@ -91,19 +97,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,
@@ -144,18 +158,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;
 }
@@ -174,14 +191,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/FlutterCompositorUnittests.mm

@@ -68,7 +68,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

@@ -9,9 +9,9 @@
 #include <iostream>
 #include <vector>
 
+#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMenuPlugin.h"
-#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMetalRenderer.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMouseCursorPlugin.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
@@ -430,14 +430,15 @@ - (FlutterCompositor*)createFlutterCompositor {
   FlutterMetalRenderer* metalRenderer = reinterpret_cast<FlutterMetalRenderer*>(_renderer);
   _macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
       _viewProvider, _platformViewController, metalRenderer.device);
-  _macOSCompositor->SetPresentCallback([weakSelf](bool has_flutter_content) {
-    if (has_flutter_content) {
-      return [weakSelf.renderer present] == YES;
-    } else {
-      [weakSelf.renderer presentWithoutContent];
-      return true;
-    }
-  });
+  _macOSCompositor->SetPresentCallback(
+      [weakSelf](bool has_flutter_content, dispatch_block_t on_notify) {
+        if (has_flutter_content) {
+          return [weakSelf.renderer presentWithBlock:on_notify] == YES;
+        } else {
+          [weakSelf.renderer presentWithoutContent];
+          return true;
+        }
+      });
 
   _compositor = {};
   _compositor.struct_size = sizeof(FlutterCompositor);

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

@@ -473,7 +473,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/FlutterMetalRenderer.mm

@@ -20,7 +20,7 @@ static FlutterMetalTexture OnGetNextDrawable(FlutterEngine* engine,
 }
 
 static bool OnPresentDrawable(FlutterEngine* engine, const FlutterMetalTexture* texture) {
-  return [engine.renderer present];
+  return [engine.renderer presentWithBlock:nil];
 }
 
 static bool OnAcquireExternalTexture(FlutterEngine* engine,
@@ -94,11 +94,11 @@ - (FlutterMetalTexture)createTextureForSize:(CGSize)size {
   return embedderTexture;
 }
 
-- (BOOL)present {
+- (BOOL)presentWithBlock:(dispatch_block_t)block {
   if (!_flutterView) {
     return NO;
   }
-  [_flutterView present];
+  [_flutterView presentWithBlock:block];
   return YES;
 }
 

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

@@ -30,9 +30,9 @@
   FlutterEngine* engine = CreateTestEngine();
   FlutterMetalRenderer* renderer = [[FlutterMetalRenderer alloc] initWithFlutterEngine:engine];
   id mockFlutterView = OCMClassMock([FlutterView class]);
-  [(FlutterView*)[mockFlutterView expect] present];
+  [(FlutterView*)[mockFlutterView expect] presentWithBlock:nil];
   [renderer setFlutterView:mockFlutterView];
-  [renderer present];
+  [renderer presentWithBlock:nil];
 }
 
 TEST(FlutterMetalRenderer, TextureReturnedByFlutterView) {

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

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

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