Address pr comments

Author: RichardJCai

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

@@ -71,6 +71,7 @@
 }
 
 bool FlutterGLCompositor::Present(const FlutterLayer** layers, size_t layers_count) {
+  DisposePlatformViews();
   for (size_t i = 0; i < layers_count; ++i) {
     const auto* layer = layers[i];
     FlutterBackingStore* backing_store = const_cast<FlutterBackingStore*>(layer->backing_store);
@@ -114,7 +115,6 @@
   // The frame has been presented, prepare FlutterGLCompositor to
   // render a new frame.
   frame_started_ = false;
-  DisposePlatformViews();
   return present_callback_();
 }
 

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

@@ -18,6 +18,12 @@ NS_ASSUME_NONNULL_BEGIN
 @protocol FlutterPlatformView <NSObject>
 /**
  * Returns a reference to the `NSView` that is wrapped by this `FlutterPlatformView`.
+ *
+ * It is recommended to return a cached view instance in this method.
+ * Constructing a new view instance and return in this method might cause undefined behavior.
+ *
+ * TODO(richardjcai): Prevent [FlutterPlatformView view] to be called multiple times
+ * in a single frame.
  */
 - (NSView*)view;
 @end
@@ -27,16 +33,16 @@ FLUTTER_EXPORT
 /**
  * Create a `FlutterPlatformView`.
  *
- * Implemented by MacOS code that expose a `NSView` for embedding in a Flutter app.
+ * Implemented by MacOS code that expose a `FlutterPlatformView` for embedding in a Flutter app.
  *
- * The implementation of this method should create a new `NSView` and return it.
+ * The implementation of this method should create a new `FlutterPlatformView` and return it.
  *
- * @param frame The rectangle for the newly created `NSView` measured in points.
- * @param viewId A unique identifier for this `NSView`.
- * @param args Parameters for creating the `NSView` sent from the Dart side of the Flutter app.
- *   If `createArgsCodec` is not implemented, or if no creation arguments were sent from the Dart
- *   code, this will be null. Otherwise this will be the value sent from the Dart code as decoded by
- *   `createArgsCodec`.
+ * @param frame The rectangle for the newly created `FlutterPlatformView` measured in points.
+ * @param viewId A unique identifier for this `FlutterPlatformView`.
+ * @param args Parameters for creating the `FlutterPlatformView` sent from the Dart side of the
+ * Flutter app. If `createArgsCodec` is not implemented, or if no creation arguments were sent from
+ * the Dart code, this will be null. Otherwise this will be the value sent from the Dart code as
+ * decoded by `createArgsCodec`.
  */
 - (NSObject<FlutterPlatformView>*)createWithFrame:(CGRect)frame
                                    viewIdentifier:(int64_t)viewId

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

@@ -397,6 +397,8 @@ - (void)addInternalPlugins {
       [weakSelf onCreate:call result:result];
     } else if ([[call method] isEqualToString:@"dispose"]) {
       [weakSelf onDispose:call result:result];
+    } else {
+      result(FlutterMethodNotImplemented);
     }
   }];
 }
@@ -406,7 +408,7 @@ - (void)onCreate:(nonnull FlutterMethodCall*)call result:(nonnull FlutterResult)
   int64_t viewId = [args[@"id"] longValue];
   NSString* viewType = [NSString stringWithUTF8String:([args[@"viewType"] UTF8String])];
 
-  if (self->_platformViews.count(viewId) != 0) {
+  if (_platformViews.count(viewId) != 0) {
     result([FlutterError errorWithCode:@"recreating_view"
                                message:@"trying to create an already created view"
                                details:[NSString stringWithFormat:@"view id: '%lld'", viewId]]);
@@ -425,7 +427,7 @@ - (void)onCreate:(nonnull FlutterMethodCall*)call result:(nonnull FlutterResult)
                                                            viewIdentifier:viewId
                                                                 arguments:nil];
 
-  self->_platformViews[viewId] = [platform_view view];
+  _platformViews[viewId] = [platform_view view];
   result(nil);
 }
 
@@ -434,15 +436,15 @@ - (void)onDispose:(nonnull FlutterMethodCall*)call result:(nonnull FlutterResult
   int64_t viewId = [arg longLongValue];
   NSLog(@"onDispose ViewId: %lld", viewId);
 
-  if (self->_platformViews.count(viewId) == 0) {
+  if (_platformViews.count(viewId) == 0) {
     result([FlutterError errorWithCode:@"unknown_view"
                                message:@"trying to dispose an unknown"
                                details:[NSString stringWithFormat:@"view id: '%lld'", viewId]]);
     return;
   }
 
   // The following FlutterGLCompositor::Present call will dispose the views.
-  self->_platformViewsToDispose.insert(viewId);
+  _platformViewsToDispose.insert(viewId);
   result(nil);
 }
 

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

@@ -15,13 +15,14 @@
 @property(nonatomic, readonly, nullable) FlutterView* flutterView;
 
 // NSDictionary maps strings to FlutterPlatformViewFactorys.
-@property(nonnull) NSMutableDictionary<NSString*, NSObject<FlutterPlatformViewFactory>*>* factories;
+@property(nonnull, nonatomic)
+    NSMutableDictionary<NSString*, NSObject<FlutterPlatformViewFactory>*>* factories;
 
 // A map of platform view ids to views.
-@property() std::map<int, NSView*> platformViews;
+@property(nonatomic) std::map<int, NSView*> platformViews;
 
 // View ids that are going to be disposed on the next present call.
-@property() std::unordered_set<int64_t> platformViewsToDispose;
+@property(nonatomic) std::unordered_set<int64_t> platformViewsToDispose;
 
 /**
  * This just returns the NSPasteboard so that it can be mocked in the tests.