Addressing PR comments

Author: RichardJCai

shell/platform/darwin/macos/BUILD.gn

@@ -77,7 +77,6 @@ source_set("flutter_framework_source") {
   deps = [
     "//flutter/flow:flow",
     "//flutter/fml",
-    "//flutter/fml:fml",
     "//flutter/shell/platform/common/cpp:common_cpp_switches",
     "//flutter/shell/platform/darwin/common:framework_shared",
     "//flutter/shell/platform/embedder:embedder_as_internal_library",
@@ -123,8 +122,8 @@ executable("flutter_desktop_darwin_unittests") {
     "framework/Source/FlutterEngineTest.mm",
     "framework/Source/FlutterMacOSGLCompositorUnittests.mm",
     "framework/Source/FlutterViewControllerTest.mm",
-    "framework/Source/FlutterViewControllerTestsUtils.h",
-    "framework/Source/FlutterViewControllerTestsUtils.mm",
+    "framework/Source/FlutterViewControllerTestUtils.h",
+    "framework/Source/FlutterViewControllerTestUtils.mm",
   ]
 
   cflags_objcc = [ "-fobjc-arc" ]

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

@@ -201,7 +201,11 @@ @implementation FlutterEngine {
   // FlutterMacOSGLCompositor is created by the engine.
   // This is only created when the engine has a FlutterViewController
   // and used to support platform views.
+  // Creation / Destruction.
   std::unique_ptr<flutter::FlutterMacOSGLCompositor> _macOSCompositor;
+
+  // FlutterCompositor is copied and used in embedder.cc.
+  FlutterCompositor _compositor;
 }
 
 - (instancetype)initWithName:(NSString*)labelPrefix project:(FlutterDartProject*)project {
@@ -311,14 +315,8 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint {
     flutterArguments.aot_data = _aotData;
   }
 
-  // Only create a Compositor if we have a ViewController.
-  if (_viewController) {
-    // Engine does not need to manage the life cycle of compositor
-    // since compositor is captured and copied in embedder.cc.
-    FlutterCompositor compositor = {};
-    [self setupCompositor:&compositor];
-    flutterArguments.compositor = &compositor;
-  }
+  [self setupCompositor];
+  flutterArguments.compositor = &_compositor;
 
   FlutterEngineResult result = _embedderAPI.Initialize(
       FLUTTER_ENGINE_VERSION, &rendererConfig, &flutterArguments, (__bridge void*)(self), &_engine);
@@ -374,39 +372,41 @@ - (void)setViewController:(FlutterViewController*)controller {
   }
 }
 
-- (void)setupCompositor:(FlutterCompositor*)compositor {
+- (void)setupCompositor {
   [_mainOpenGLContext makeCurrentContext];
 
-  _macOSCompositor =
-      std::make_unique<flutter::FlutterMacOSGLCompositor>(_viewController, _resourceContext);
+  _macOSCompositor = std::make_unique<flutter::FlutterMacOSGLCompositor>(_viewController);
 
-  compositor->struct_size = sizeof(FlutterCompositor);
-  compositor->user_data = _macOSCompositor.get();
+  _compositor = {};
+  _compositor.struct_size = sizeof(FlutterCompositor);
+  _compositor.user_data = _macOSCompositor.get();
 
-  compositor->create_backing_store_callback = [](const FlutterBackingStoreConfig* config,  //
+  _compositor.create_backing_store_callback = [](const FlutterBackingStoreConfig* config,  //
                                                  FlutterBackingStore* backing_store_out,   //
                                                  void* user_data                           //
                                               ) {
     return reinterpret_cast<flutter::FlutterMacOSGLCompositor*>(user_data)->CreateBackingStore(
         config, backing_store_out);
   };
 
-  compositor->collect_backing_store_callback = [](const FlutterBackingStore* backing_store,  //
+  _compositor.collect_backing_store_callback = [](const FlutterBackingStore* backing_store,  //
                                                   void* user_data                            //
                                                ) {
     return reinterpret_cast<flutter::FlutterMacOSGLCompositor*>(user_data)->CollectBackingStore(
         backing_store);
   };
 
-  compositor->present_layers_callback = [](const FlutterLayer** layers,  //
+  _compositor.present_layers_callback = [](const FlutterLayer** layers,  //
                                            size_t layers_count,          //
                                            void* user_data               //
                                         ) {
     return reinterpret_cast<flutter::FlutterMacOSGLCompositor*>(user_data)->Present(layers,
                                                                                     layers_count);
   };
 
-  _macOSCompositor->SetPresentCallback([self]() { return [self engineCallbackOnPresent]; });
+  __weak FlutterEngine* weak_self = self;
+  _macOSCompositor->SetPresentCallback(
+      [weak_self]() { return [weak_self engineCallbackOnPresent]; });
 }
 
 - (id<FlutterBinaryMessenger>)binaryMessenger {

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

@@ -12,19 +12,20 @@
 
 namespace flutter {
 
-/**
- * FlutterMacOSGLCompositor creates and manages backing stores used for
- * rendering Flutter content and presents Flutter content and Platform views.
- */
+// FlutterMacOSGLCompositor creates and manages the backing stores used for
+// rendering Flutter content and presents Flutter content and Platform views.
+// Platform views are not yet supported.
+// FlutterMacOSGLCompositor is created and destroyed by FlutterEngine. 
 class FlutterMacOSGLCompositor {
  public:
-  FlutterMacOSGLCompositor(FlutterViewController* view_controller,
-                           NSOpenGLContext* open_gl_context);
+  FlutterMacOSGLCompositor(FlutterViewController* view_controller);
 
-  virtual ~FlutterMacOSGLCompositor();
-
-  // Creates a backing store according to FlutterBackingStoreConfig
-  // by modifying backing_store_out.
+  // Creates a FlutterSurfaceManager and uses the FlutterSurfaceManager's
+  // underlying FBO and texture in the backing store.
+  // Any additional state allocated for the backing store and 
+  // saved as user_data in the backing store must be collected
+  // in the backing_store's desctruction_callback field which will
+  // be called when the embedder collects the backing store.
   bool CreateBackingStore(const FlutterBackingStoreConfig* config,
                           FlutterBackingStore* backing_store_out);
 
@@ -40,17 +41,11 @@ class FlutterMacOSGLCompositor {
   // PresentCallback is called at the end of the Present function.
   void SetPresentCallback(const PresentCallback& present_callback);
 
- protected:
+ private:
   FlutterViewController* view_controller_;
   PresentCallback present_callback_;
   NSOpenGLContext* open_gl_context_;
 
-  // Creates a FlutterSurfaceManager and uses the FlutterSurfaceManager's
-  // underlying FBO and texture in the backing store.
-  bool CreateBackingStoreUsingSurfaceManager(
-      const FlutterBackingStoreConfig* config,
-      FlutterBackingStore* backing_store_out);
-
   FML_DISALLOW_COPY_AND_ASSIGN(FlutterMacOSGLCompositor);
 };
 

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

@@ -16,15 +16,39 @@
 
 namespace flutter {
 
-FlutterMacOSGLCompositor::FlutterMacOSGLCompositor(FlutterViewController* view_controller,
-                                                   NSOpenGLContext* open_gl_context)
-    : view_controller_(view_controller), open_gl_context_(open_gl_context) {}
-
-FlutterMacOSGLCompositor::~FlutterMacOSGLCompositor() = default;
+FlutterMacOSGLCompositor::FlutterMacOSGLCompositor(FlutterViewController* view_controller)
+    : view_controller_(view_controller),
+      open_gl_context_(view_controller.flutterView.openGLContext) {}
 
 bool FlutterMacOSGLCompositor::CreateBackingStore(const FlutterBackingStoreConfig* config,
                                                   FlutterBackingStore* backing_store_out) {
-  return CreateBackingStoreUsingSurfaceManager(config, backing_store_out);
+  FlutterSurfaceManager* surfaceManager =
+      [[FlutterSurfaceManager alloc] initWithLayer:view_controller_.flutterView.layer
+                                     openGLContext:open_gl_context_
+                                   numFramebuffers:1];
+
+  GLuint fbo = [surfaceManager getFramebuffer];
+  GLuint texture = [surfaceManager getTexture];
+
+  CGSize size = CGSizeMake(config->size.width, config->size.height);
+  size_t kFlutterSurfaceManagerFrontBuffer = 0;
+  [surfaceManager backTextureWithIOSurface:kFlutterSurfaceManagerFrontBuffer
+                                      size:size
+                            backingTexture:texture
+                                       fbo:fbo];
+
+  backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
+  backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;
+  backing_store_out->open_gl.framebuffer.target = GL_RGBA8;
+  backing_store_out->open_gl.framebuffer.name = fbo;
+  backing_store_out->open_gl.framebuffer.user_data = (__bridge_retained void*)surfaceManager;
+  backing_store_out->open_gl.framebuffer.destruction_callback = [](void* user_data) {
+    if (user_data != nullptr) {
+      CFRelease(user_data);
+    }
+  };
+
+  return true;
 }
 
 bool FlutterMacOSGLCompositor::CollectBackingStore(const FlutterBackingStore* backing_store) {
@@ -40,11 +64,12 @@
     switch (layer->type) {
       case kFlutterLayerContentTypeBackingStore: {
         FlutterSurfaceManager* surfaceManager =
-            (__bridge FlutterSurfaceManager*)backing_store->user_data;
+            (__bridge FlutterSurfaceManager*)backing_store->open_gl.framebuffer.user_data;
 
         CGSize size = CGSizeMake(layer->size.width, layer->size.height);
         [view_controller_.flutterView frameBufferIDForSize:size];
-        [surfaceManager setLayerContentWithIOSurface:[surfaceManager getIOSurface]];
+        size_t kFlutterSurfaceManagerFrontBuffer = 0;
+        [surfaceManager setLayerContentWithIOSurface:kFlutterSurfaceManagerFrontBuffer];
         break;
       }
       case kFlutterLayerContentTypePlatformView:
@@ -56,36 +81,6 @@
   return present_callback_();
 }
 
-bool FlutterMacOSGLCompositor::CreateBackingStoreUsingSurfaceManager(
-    const FlutterBackingStoreConfig* config,
-    FlutterBackingStore* backing_store_out) {
-  FlutterSurfaceManager* surfaceManager =
-      [[FlutterSurfaceManager alloc] initWithLayer:view_controller_.flutterView.layer
-                                     openGLContext:open_gl_context_];
-
-  GLuint fbo = [surfaceManager getFramebuffer];
-  GLuint texture = [surfaceManager getTexture];
-  IOSurfaceRef* io_surface_ref = [surfaceManager getIOSurface];
-
-  CGSize size = CGSizeMake(config->size.width, config->size.height);
-
-  [surfaceManager backTextureWithIOSurface:io_surface_ref size:size backingTexture:texture fbo:fbo];
-
-  backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
-  backing_store_out->user_data = (__bridge_retained void*)surfaceManager;
-  backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;
-  backing_store_out->open_gl.framebuffer.target = GL_RGBA8;
-  backing_store_out->open_gl.framebuffer.name = fbo;
-  backing_store_out->open_gl.framebuffer.user_data = backing_store_out->user_data;
-  backing_store_out->open_gl.framebuffer.destruction_callback = [](void* user_data) {
-    if (user_data != nullptr) {
-      CFRelease(user_data);
-    }
-  };
-
-  return true;
-}
-
 void FlutterMacOSGLCompositor::SetPresentCallback(
     const FlutterMacOSGLCompositor::PresentCallback& present_callback) {
   present_callback_ = present_callback;

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

@@ -5,7 +5,7 @@
 #import <Foundation/Foundation.h>
 
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMacOSGLCompositor.h"
-#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTestsUtils.h"
+#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTestUtils.h"
 #import "flutter/testing/testing.h"
 
 namespace flutter::testing {
@@ -14,7 +14,7 @@
   id mockViewController = CreateMockViewController(nil);
 
   std::unique_ptr<flutter::FlutterMacOSGLCompositor> macos_compositor =
-      std::make_unique<FlutterMacOSGLCompositor>(mockViewController, nil);
+      std::make_unique<FlutterMacOSGLCompositor>(mockViewController);
 
   bool flag = false;
   macos_compositor->SetPresentCallback([f = &flag]() {

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

@@ -3,7 +3,9 @@
 // Manages the IOSurfaces for FlutterView
 @interface FlutterSurfaceManager : NSObject
 
-- (instancetype)initWithLayer:(CALayer*)layer openGLContext:(NSOpenGLContext*)opengLContext;
+- (instancetype)initWithLayer:(CALayer*)containingLayer
+                openGLContext:(NSOpenGLContext*)openGLContext
+              numFramebuffers:(int)numFramebuffers;
 
 - (void)ensureSurfaceSize:(CGSize)size;
 - (void)swapBuffers;
@@ -16,14 +18,15 @@
 - (void)setLayerContent;
 
 /**
- * Sets the CALayer content to the content of the provided ioSurface.
+ * Sets the CALayer content to the content of ioSurface at ioSurfaceNum.
  */
-- (void)setLayerContentWithIOSurface:(IOSurfaceRef*)ioSurface;
+// TODO(richardjcai): Fix this and remove setLayerContent.
+- (void)setLayerContentWithIOSurface:(int)ioSurfaceNum;
 
 /**
  * Binds the IOSurface to the provided texture/framebuffer.
  */
-- (void)backTextureWithIOSurface:(IOSurfaceRef*)ioSurface
+- (void)backTextureWithIOSurface:(int)ioSurfaceNum
                             size:(CGSize)size
                   backingTexture:(GLuint)texture
                              fbo:(GLuint)fbo;
@@ -47,14 +50,4 @@
  */
 - (uint32_t)getTexture;
 
-/**
- * Returns the kFront IOSurfaceRef.
- * The IOSurface is backed by the FBO provided by getFramebuffer
- * and texture provided by getTexture. The IOSurface is used
- * in FlutterMacOSCompositor's Present call.
- * The IOSurface is collected when the backing store that uses the
- * IOSurface is collected.
- */
-- (IOSurfaceRef*)getIOSurface;
-
 @end