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

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

@@ -67,6 +67,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 to non-nil to nil will terminate the engine if
+ * allowHeadlessExecution is NO.
+ *
+ * Setting this field from non-nil to a different non-nil is prohibited and
+ * will throw an assertion error.
  */
 @property(nonatomic, nullable, weak) FlutterViewController* viewController;
 

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

@@ -31,9 +31,19 @@ FLUTTER_DARWIN_EXPORT
 
 /**
  * The Flutter engine associated with this view controller.
+ *
+ * The engine is strongly referenced by the FlutterViewController, and weakly
+ * vice versa.
  */
 @property(nonatomic, nonnull, readonly) FlutterEngine* engine;
 
+/**
+ * The identifier for this view controller.
+ *
+ * The ID is assigned when the view controller is added to FlutterEngine.
+ */
+@property(nonatomic, readonly) uint64_t id;
+
 /**
  * The style of mouse tracking to use for the view. Defaults to
  * FlutterMouseTrackingModeInKeyWindow.

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/FlutterEngine.mm

@@ -80,6 +80,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 +215,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 +248,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 +410,28 @@ - (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) {
     _viewController = controller;
-
-    if (_semanticsEnabled && _bridge) {
-      _bridge->UpdateDefaultViewController(_viewController);
-    }
-
-    if (!controller && !_allowHeadlessExecution) {
+    [_viewController attachToEngine:self withId:kFlutterDefaultViewId];
+  } else if (_viewController != nil && controller == nil) {
+    [_viewController detachFromEngine];
+    _viewController = nil;
+    if (!_allowHeadlessExecution) {
       [self shutDownEngine];
     }
+  } else {
+    NSLog(@"Failed to set view controller to the engine: "
+          @"Replacing the view controller of the engine is not supported.");
   }
 }
 
 - (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 +537,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 +601,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 +688,7 @@ - (void)shutDownEngine {
     return;
   }
 
-  if (_viewController && _viewController.flutterView) {
-    [_viewController.flutterView shutdown];
-  }
+  [self.viewController.flutterView shutdown];
 
   FlutterEngineResult result = _embedderAPI.Deinitialize(_engine);
   if (result != kSuccess) {

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

@@ -134,14 +134,11 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
   EXPECT_TRUE([engine runWithEntrypoint:@"backgroundTest"]);
   EXPECT_TRUE(engine.running);
 
-  NSString* fixtures = @(flutter::testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
   [viewController loadView];
   viewController.flutterView.frame = CGRectMake(0, 0, 800, 600);
-  [engine setViewController:viewController];
 
   latch.Wait();
 }
@@ -166,14 +163,11 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
   EXPECT_TRUE([engine runWithEntrypoint:@"backgroundTest"]);
   EXPECT_TRUE(engine.running);
 
-  NSString* fixtures = @(flutter::testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
   [viewController loadView];
   viewController.flutterView.frame = CGRectMake(0, 0, 800, 600);
-  [engine setViewController:viewController];
   viewController.flutterView.backgroundColor = [NSColor whiteColor];
 
   latch.Wait();
@@ -193,13 +187,10 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
       }));
   EXPECT_TRUE([engine runWithEntrypoint:@"main"]);
   // Set up view controller.
-  NSString* fixtures = @(testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
   [viewController loadView];
-  [engine setViewController:viewController];
   // Enable the semantics.
   bool enabled_called = false;
   engine.embedderAPI.UpdateSemanticsEnabled =
@@ -337,6 +328,8 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
   FlutterSemanticsNode nodes[] = {root, child1};
   update.nodes = nodes;
   update.custom_actions_count = 0;
+  // This call updates semantics for the default view, which does not exist,
+  // and therefore this call is invalid. But the engine should not crash.
   update_semantics_callback(&update, (__bridge void*)engine);
 
   // No crashes.
@@ -355,93 +348,6 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
   EXPECT_EQ(engine.viewController, nil);
 }
 
-TEST_F(FlutterEngineTest, ResetsAccessibilityBridgeWhenSetsNewViewController) {
-  FlutterEngine* engine = GetFlutterEngine();
-  // Capture the update callbacks before the embedder API initializes.
-  auto original_init = engine.embedderAPI.Initialize;
-  std::function<void(const FlutterSemanticsUpdate*, void*)> update_semantics_callback;
-  engine.embedderAPI.Initialize = MOCK_ENGINE_PROC(
-      Initialize, ([&update_semantics_callback, &original_init](
-                       size_t version, const FlutterRendererConfig* config,
-                       const FlutterProjectArgs* args, void* user_data, auto engine_out) {
-        update_semantics_callback = args->update_semantics_callback;
-        return original_init(version, config, args, user_data, engine_out);
-      }));
-  EXPECT_TRUE([engine runWithEntrypoint:@"main"]);
-  // Set up view controller.
-  NSString* fixtures = @(testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
-  [viewController loadView];
-  [engine setViewController:viewController];
-  // Enable the semantics.
-  bool enabled_called = false;
-  engine.embedderAPI.UpdateSemanticsEnabled =
-      MOCK_ENGINE_PROC(UpdateSemanticsEnabled, ([&enabled_called](auto engine, bool enabled) {
-                         enabled_called = enabled;
-                         return kSuccess;
-                       }));
-  engine.semanticsEnabled = YES;
-  EXPECT_TRUE(enabled_called);
-  // Send flutter semantics updates.
-  FlutterSemanticsNode root;
-  root.id = 0;
-  root.flags = static_cast<FlutterSemanticsFlag>(0);
-  root.actions = static_cast<FlutterSemanticsAction>(0);
-  root.text_selection_base = -1;
-  root.text_selection_extent = -1;
-  root.label = "root";
-  root.hint = "";
-  root.value = "";
-  root.increased_value = "";
-  root.decreased_value = "";
-  root.tooltip = "";
-  root.child_count = 1;
-  int32_t children[] = {1};
-  root.children_in_traversal_order = children;
-  root.custom_accessibility_actions_count = 0;
-
-  FlutterSemanticsNode child1;
-  child1.id = 1;
-  child1.flags = static_cast<FlutterSemanticsFlag>(0);
-  child1.actions = static_cast<FlutterSemanticsAction>(0);
-  child1.text_selection_base = -1;
-  child1.text_selection_extent = -1;
-  child1.label = "child 1";
-  child1.hint = "";
-  child1.value = "";
-  child1.increased_value = "";
-  child1.decreased_value = "";
-  child1.tooltip = "";
-  child1.child_count = 0;
-  child1.custom_accessibility_actions_count = 0;
-
-  FlutterSemanticsUpdate update;
-  update.nodes_count = 2;
-  FlutterSemanticsNode nodes[] = {root, child1};
-  update.nodes = nodes;
-  update.custom_actions_count = 0;
-  update_semantics_callback(&update, (__bridge void*)engine);
-
-  auto native_root = engine.accessibilityBridge.lock()->GetFlutterPlatformNodeDelegateFromID(0);
-  EXPECT_FALSE(native_root.expired());
-
-  // Set up a new view controller.
-  FlutterViewController* newViewController =
-      [[FlutterViewController alloc] initWithProject:project];
-  [newViewController loadView];
-  [engine setViewController:newViewController];
-
-  auto new_native_root = engine.accessibilityBridge.lock()->GetFlutterPlatformNodeDelegateFromID(0);
-  // The tree is recreated and the old tree will be destroyed.
-  EXPECT_FALSE(new_native_root.expired());
-  EXPECT_TRUE(native_root.expired());
-
-  [engine setViewController:nil];
-}
-
 TEST_F(FlutterEngineTest, NativeCallbacks) {
   fml::AutoResetWaitableEvent latch;
   bool latch_called = false;
@@ -465,10 +371,11 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
              ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
   FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:project];
 
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
   [viewController loadView];
   viewController.flutterView.frame = CGRectMake(0, 0, 800, 600);
-  [engine setViewController:viewController];
 
   EXPECT_TRUE([engine runWithEntrypoint:@"canCompositePlatformViews"]);