Reland "[macOS] Formalize FlutterViewController's initialization flow, and prohibit replacing"

shell/platform/darwin/macos/framework/Headers/FlutterEngine.h

@@ -7,6 +7,8 @@
 
 #import <Foundation/Foundation.h>
 
+#include <stdint.h>
+
 #import "FlutterBinaryMessenger.h"
 #import "FlutterDartProject.h"
 #import "FlutterMacros.h"
@@ -15,6 +17,17 @@
 
 // TODO: Merge this file with the iOS FlutterEngine.h.
 
+/**
+ * The view ID for APIs that don't support multi-view.
+ *
+ * Some single-view APIs will eventually be replaced by their multi-view
+ * variant. During the deprecation period, the single-view APIs will coexist with
+ * and work with the multi-view APIs as if the other views don't exist.  For
+ * backward compatibility, single-view APIs will always operate the view with
+ * this ID. Also, the first view assigned to the engine will also have this ID.
+ */
+extern const uint64_t kFlutterDefaultViewId;
+
 @class FlutterViewController;
 
 /**
@@ -67,6 +80,15 @@ FLUTTER_DARWIN_EXPORT
  *
  * The default view always has ID kFlutterDefaultViewId, and is the view
  * operated by the APIs that do not have a view ID specified.
+ *
+ * Setting this field from nil to a non-nil view controller also updates
+ * the view controller's engine and ID.
+ *
+ * Setting this field from non-nil to nil will terminate the engine if
+ * allowHeadlessExecution is NO.
+ *
+ * Setting this field from non-nil to a different non-nil FlutterViewController
+ * is prohibited and will throw an assertion error.
  */
 @property(nonatomic, nullable, weak) FlutterViewController* viewController;
 

shell/platform/darwin/macos/framework/Headers/FlutterViewController.h

@@ -25,6 +25,25 @@ typedef NS_ENUM(NSInteger, FlutterMouseTrackingMode) {
 
 /**
  * Controls a view that displays Flutter content and manages input.
+ *
+ * A FlutterViewController works with a FlutterEngine. Upon creation, the view
+ * controller is always added to an engine, either a given engine, or it implicitly
+ * creates an engine and add itself to that engine.
+ *
+ * The FlutterEngine assigns each view controller attached to it a unique ID.
+ * Each view controller corresponds to a view, and the ID is used by the framework
+ * to specify which view to operate.
+ *
+ * A FlutterViewController can also be unattached to an engine after it is manually
+ * unset from the engine, or transiently during the initialization process.
+ * An unattached view controller is invalid. Whether the view controller is attached
+ * can be queried using FlutterViewController#attached.
+ *
+ * The FlutterViewController strongly references the FlutterEngine, while
+ * the engine weakly the view controller. When a FlutterViewController is deallocated,
+ * it automatically removes itself from its attached engine. When a FlutterEngine
+ * has no FlutterViewControllers attached, it might shut down itself or not depending
+ * on its configuration.
  */
 FLUTTER_DARWIN_EXPORT
 @interface FlutterViewController : NSViewController <FlutterPluginRegistry>
@@ -34,6 +53,16 @@ FLUTTER_DARWIN_EXPORT
  */
 @property(nonatomic, nonnull, readonly) FlutterEngine* engine;
 
+/**
+ * The identifier for this view controller.
+ *
+ * The ID is assigned by FlutterEngine when the view controller is attached.
+ *
+ * If the view controller is unattached (see FlutterViewController#attached),
+ * reading this property throws an assertion.
+ */
+@property(nonatomic, readonly) uint64_t id;
+
 /**
  * The style of mouse tracking to use for the view. Defaults to
  * FlutterMouseTrackingModeInKeyWindow.
@@ -43,6 +72,12 @@ FLUTTER_DARWIN_EXPORT
 /**
  * Initializes a controller that will run the given project.
  *
+ * In this initializer, this controller creates an engine, and is attached to
+ * that engine as the default controller. In this way, this controller can not
+ * be set to other engines. This initializer is suitable for the first Flutter
+ * view controller of the app. To use the controller with an existing engine,
+ * use initWithEngine:nibName:bundle: instead.
+ *
  * @param project The project to run in this view controller. If nil, a default `FlutterDartProject`
  *                will be used.
  */
@@ -56,7 +91,8 @@ FLUTTER_DARWIN_EXPORT
 /**
  * Initializes this FlutterViewController with the specified `FlutterEngine`.
  *
- * The initialized viewcontroller will attach itself to the engine as part of this process.
+ * This initializer is suitable for both the first Flutter view controller and
+ * the following ones of the app.
  *
  * @param engine The `FlutterEngine` instance to attach to. Cannot be nil.
  * @param nibName The NIB name to initialize this controller with.
@@ -65,6 +101,12 @@ FLUTTER_DARWIN_EXPORT
 - (nonnull instancetype)initWithEngine:(nonnull FlutterEngine*)engine
                                nibName:(nullable NSString*)nibName
                                 bundle:(nullable NSBundle*)nibBundle NS_DESIGNATED_INITIALIZER;
+
+/**
+ * Return YES if the view controller is attached to an engine.
+ */
+- (BOOL)attached;
+
 /**
  * Invoked by the engine right before the engine is restarted.
  *

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

@@ -51,26 +51,22 @@ @implementation AccessibilityBridgeTestEngine
 namespace {
 
 // Returns an engine configured for the text fixture resource configuration.
-FlutterEngine* CreateTestEngine() {
+FlutterViewController* CreateTestViewController() {
   NSString* fixtures = @(testing::GetFixturesPath());
   FlutterDartProject* project = [[FlutterDartProject alloc]
       initWithAssetsPath:fixtures
              ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  return [[AccessibilityBridgeTestEngine alloc] initWithName:@"test"
-                                                     project:project
-                                      allowHeadlessExecution:true];
+  FlutterEngine* engine = [[AccessibilityBridgeTestEngine alloc] initWithName:@"test"
+                                                                      project:project
+                                                       allowHeadlessExecution:true];
+  return [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil];
 }
 }  // namespace
 
 TEST(AccessibilityBridgeMacTest, sendsAccessibilityCreateNotificationToWindowOfFlutterView) {
-  FlutterEngine* engine = CreateTestEngine();
-  NSString* fixtures = @(testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = CreateTestViewController();
+  FlutterEngine* engine = viewController.engine;
   [viewController loadView];
-  [engine setViewController:viewController];
 
   NSWindow* expectedTarget = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
                                                          styleMask:NSBorderlessWindowMask
@@ -122,14 +118,9 @@ @implementation AccessibilityBridgeTestEngine
 }
 
 TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenHeadless) {
-  FlutterEngine* engine = CreateTestEngine();
-  NSString* fixtures = @(testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = CreateTestViewController();
+  FlutterEngine* engine = viewController.engine;
   [viewController loadView];
-  [engine setViewController:viewController];
   // Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
   // can query semantics information from.
   engine.semanticsEnabled = YES;
@@ -173,15 +164,9 @@ @implementation AccessibilityBridgeTestEngine
 }
 
 TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenNoWindow) {
-  FlutterEngine* engine = CreateTestEngine();
-  // Create a view controller without attaching it to a window.
-  NSString* fixtures = @(testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = CreateTestViewController();
+  FlutterEngine* engine = viewController.engine;
   [viewController loadView];
-  [engine setViewController:viewController];
 
   // Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
   // can query semantics information from.

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

@@ -10,7 +10,6 @@
 
 #include "flutter/fml/macros.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
-#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
 #include "flutter/shell/platform/embedder/embedder.h"
 

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

@@ -19,6 +19,8 @@
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h"
 #include "flutter/shell/platform/embedder/embedder.h"
 
+const uint64_t kFlutterDefaultViewId = 0;
+
 /**
  * Constructs and returns a FlutterLocale struct corresponding to |locale|, which must outlive
  * the returned struct.
@@ -80,6 +82,8 @@ @interface FlutterEngine () <FlutterBinaryMessenger>
  */
 @property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;
 
+- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;
+
 /**
  * Sends the list of user-preferred locales to the Flutter engine.
  */
@@ -213,8 +217,6 @@ @implementation FlutterEngine {
   // when the engine is destroyed.
   std::unique_ptr<flutter::FlutterCompositor> _macOSCompositor;
 
-  FlutterViewEngineProvider* _viewProvider;
-
   // FlutterCompositor is copied and used in embedder.cc.
   FlutterCompositor _compositor;
 
@@ -248,7 +250,6 @@ - (instancetype)initWithName:(NSString*)labelPrefix
   _currentMessengerConnection = 1;
   _allowHeadlessExecution = allowHeadlessExecution;
   _semanticsEnabled = NO;
-  _viewProvider = [[FlutterViewEngineProvider alloc] initWithEngine:self];
   _isResponseValid = [[NSMutableArray alloc] initWithCapacity:1];
   [_isResponseValid addObject:@YES];
 
@@ -411,29 +412,41 @@ - (void)loadAOTData:(NSString*)assetsDir {
 }
 
 - (void)setViewController:(FlutterViewController*)controller {
-  if (_viewController != controller) {
+  if (_viewController == controller) {
+    // From nil to nil, or from non-nil to the same controller.
+    return;
+  }
+  if (_viewController == nil && controller != nil) {
+    // From nil to non-nil.
+    NSAssert(controller.engine == nil,
+             @"Failed to set view controller to the engine: "
+             @"The given FlutterViewController is already attached to an engine %@. "
+             @"If you wanted to create an FlutterViewController and set it to an existing engine, "
+             @"you should create it with init(engine:, nibName, bundle:) instead.",
+             controller.engine);
     _viewController = controller;
-
-    if (_semanticsEnabled && _bridge) {
-      _bridge->UpdateDefaultViewController(_viewController);
-    }
-
-    if (!controller && !_allowHeadlessExecution) {
+    [_viewController attachToEngine:self withId:kFlutterDefaultViewId];
+  } else if (_viewController != nil && controller == nil) {
+    // From non-nil to nil.
+    [_viewController detachFromEngine];
+    _viewController = nil;
+    if (!_allowHeadlessExecution) {
       [self shutDownEngine];
     }
+  } else {
+    // From non-nil to a different non-nil view controller.
+    NSAssert(NO,
+             @"Failed to set view controller to the engine: "
+             @"The engine already has a default view controller %@. "
+             @"If you wanted to make the default view render in a different window, "
+             @"you should attach the current view controller to the window instead.",
+             _viewController);
   }
 }
 
 - (FlutterCompositor*)createFlutterCompositor {
-  // TODO(richardjcai): Add support for creating a FlutterCompositor
-  // with a nil _viewController for headless engines.
-  // https://github.com/flutter/flutter/issues/71606
-  if (!_viewController) {
-    return nil;
-  }
-
-  _macOSCompositor =
-      std::make_unique<flutter::FlutterCompositor>(_viewProvider, _platformViewController);
+  _macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
+      [[FlutterViewEngineProvider alloc] initWithEngine:self], _platformViewController);
 
   _compositor = {};
   _compositor.struct_size = sizeof(FlutterCompositor);
@@ -539,10 +552,10 @@ - (nonnull NSString*)executableName {
 }
 
 - (void)updateWindowMetrics {
-  if (!_engine || !_viewController.viewLoaded) {
+  if (!_engine || !self.viewController.viewLoaded) {
     return;
   }
-  NSView* view = _viewController.flutterView;
+  NSView* view = self.viewController.flutterView;
   CGRect scaledBounds = [view convertRectToBacking:view.bounds];
   CGSize scaledSize = scaledBounds.size;
   double pixelRatio = view.bounds.size.width == 0 ? 1 : scaledSize.width / view.bounds.size.width;
@@ -603,6 +616,17 @@ - (FlutterPlatformViewController*)platformViewController {
 
 #pragma mark - Private methods
 
+- (FlutterViewController*)viewControllerForId:(uint64_t)viewId {
+  // TODO(dkwingsmt): The engine only supports single-view, therefore it
+  // only processes the default ID. After the engine supports multiple views,
+  // this method should be able to return the view for any IDs.
+  NSAssert(viewId == kFlutterDefaultViewId, @"Unexpected view ID %llu", viewId);
+  if (viewId == kFlutterDefaultViewId) {
+    return _viewController;
+  }
+  return nil;
+}
+
 - (void)sendUserLocales {
   if (!self.running) {
     return;
@@ -679,9 +703,7 @@ - (void)shutDownEngine {
     return;
   }
 
-  if (_viewController && _viewController.flutterView) {
-    [_viewController.flutterView shutdown];
-  }
+  [self.viewController.flutterView shutdown];
 
   FlutterEngineResult result = _embedderAPI.Deinitialize(_engine);
   if (result != kSuccess) {