From f783266f9bf9b60cf52a260af88d78782a339f32 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 00:37:41 -0700 Subject: [PATCH 1/7] Impl --- .../macos/framework/Source/FlutterEngine.mm | 61 ++++++++++--------- .../framework/Source/FlutterEngine_Internal.h | 3 +- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 9070fbf613057..257606de01561 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -456,9 +456,6 @@ @implementation FlutterEngine { FlutterThreadSynchronizer* _threadSynchronizer; - // The next available view ID. - int _nextviewIdentifier; - // Whether the application is currently the active application. BOOL _active; @@ -515,8 +512,6 @@ - (instancetype)initWithName:(NSString*)labelPrefix _binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self]; _isResponseValid = [[NSMutableArray alloc] initWithCapacity:1]; [_isResponseValid addObject:@YES]; - // kFlutterImplicitViewId is reserved for the implicit view. - _nextviewIdentifier = kFlutterImplicitViewId + 1; _embedderAPI.struct_size = sizeof(FlutterEngineProcTable); FlutterEngineGetProcAddresses(&_embedderAPI); @@ -567,6 +562,14 @@ - (void)dealloc { } } } + // Detach all view controllers. + NSEnumerator* viewControllerEnumerator = [_viewControllers objectEnumerator]; + FlutterViewController* nextViewController; + while ((nextViewController = [viewControllerEnumerator nextObject])) { + [nextViewController detachFromEngine]; + } + [_viewControllers removeAllObjects]; + // Clear any published values, just in case a plugin has created a retain cycle with the // registrar. for (NSString* pluginName in _pluginRegistrars) { @@ -736,15 +739,21 @@ - (void)loadAOTData:(NSString*)assetsDir { - (void)registerViewController:(FlutterViewController*)controller forIdentifier:(FlutterViewIdentifier)viewIdentifier { NSAssert(controller != nil, @"The controller must not be nil."); - NSAssert(![controller attached], - @"The incoming view controller is already attached to an engine."); + NSAssert(controller.engine == nil, + @"The FlutterViewController is unexpectedly attached to " + @"engine %@ before initialization.", + controller.engine); NSAssert([_viewControllers objectForKey:@(viewIdentifier)] == nil, @"The requested view ID is occupied."); + [_viewControllers setObject:controller forKey:@(viewIdentifier)]; [controller setUpWithEngine:self viewIdentifier:viewIdentifier threadSynchronizer:_threadSynchronizer]; NSAssert(controller.viewIdentifier == viewIdentifier, @"Failed to assign view ID."); - [_viewControllers setObject:controller forKey:@(viewIdentifier)]; + NSAssert(controller.attached && controller.engine == self, + @"The FlutterViewController unexpectedly stays unattached after being added. " + @"In unit tests, this is likely because either the FlutterViewController or " + @"the FlutterEngine is mocked. Please subclass these classes instead."); if (controller.viewLoaded) { [self viewControllerViewDidLoad:controller]; @@ -779,11 +788,17 @@ - (void)viewControllerViewDidLoad:(FlutterViewController*)viewController { } - (void)deregisterViewControllerForIdentifier:(FlutterViewIdentifier)viewIdentifier { - FlutterViewController* oldController = [self viewControllerForIdentifier:viewIdentifier]; - if (oldController != nil) { - [oldController detachFromEngine]; - [_viewControllers removeObjectForKey:@(viewIdentifier)]; + FlutterViewController* controller = [self viewControllerForIdentifier:viewIdentifier]; + // The controller might be nil, because the engine only stores a weak ref, and + // this method might have been called from the controller's dealloc. + if (controller != nil) { + [controller detachFromEngine]; + NSAssert(!controller.attached, + @"The FlutterViewController unexpectedly stays attached after being removed. " + @"In unit tests, this is likely because either the FlutterViewController or " + @"the FlutterEngine is mocked. Please subclass these classes instead."); } + [_viewControllers removeObjectForKey:@(viewIdentifier)]; @synchronized(_vsyncWaiters) { [_vsyncWaiters removeObjectForKey:@(viewIdentifier)]; } @@ -877,26 +892,14 @@ - (FlutterCompositor*)createFlutterCompositor { #pragma mark - Framework-internal methods - (void)addViewController:(FlutterViewController*)controller { - NSAssert(controller.engine == nil, - @"The FlutterViewController is unexpectedly attached to " - @"engine %@ before initialization.", - controller.engine); - [self registerViewController:controller forIdentifier:kFlutterImplicitViewId]; - NSAssert(controller.attached, - @"The FlutterViewController unexpectedly stays unattached after being added. " - @"In unit tests, this is likely because either the FlutterViewController or " - @"the FlutterEngine is mocked. Please subclass these classes instead."); - NSAssert(controller.engine == self, - @"The FlutterViewController #%lld has unexpected engine %@ after being added, " - @"instead of %@. " - @"In unit tests, this is likely because either the FlutterViewController or " - @"the FlutterEngine is mocked. Please subclass these classes instead.", - controller.viewIdentifier, controller.engine, self); + // Adding a view controller when there is no implicit view should always + // assign it to the implicit view controller. + NSAssert(self.viewController == nil, + @"The engine already has a view controller for the implicit view."); + self.viewController = controller; } - (void)removeViewController:(nonnull FlutterViewController*)viewController { - NSAssert([viewController attached] && viewController.engine == self, - @"The given view controller is not associated with this engine."); [self deregisterViewControllerForIdentifier:viewController.viewIdentifier]; [self shutDownIfNeeded]; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index 86b79f3b79b4e..23f2cd0421e5c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -127,7 +127,8 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { * * Practically, since FlutterEngine can only be attached with one controller, * the given controller, if successfully attached, will always have the default - * view ID kFlutterImplicitViewId. + * view ID kFlutterImplicitViewId. If the engine already has an implicit view, + * this call throws an assertion. * * The engine holds a weak reference to the attached view controller. * From 26f6b0f11942f87cc205e2af2b4b3e7d81a1e868 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 00:42:26 -0700 Subject: [PATCH 2/7] Remove useless code --- .../darwin/macos/framework/Source/FlutterEngine.mm | 7 ------- 1 file changed, 7 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 257606de01561..aa572511e9d8f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -562,13 +562,6 @@ - (void)dealloc { } } } - // Detach all view controllers. - NSEnumerator* viewControllerEnumerator = [_viewControllers objectEnumerator]; - FlutterViewController* nextViewController; - while ((nextViewController = [viewControllerEnumerator nextObject])) { - [nextViewController detachFromEngine]; - } - [_viewControllers removeAllObjects]; // Clear any published values, just in case a plugin has created a retain cycle with the // registrar. From a7782125481795d320625609ee53331d58c11698 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 11:21:20 -0700 Subject: [PATCH 3/7] Better doc --- shell/platform/darwin/macos/framework/Source/FlutterEngine.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index aa572511e9d8f..22fbf2f3348dc 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -885,8 +885,8 @@ - (FlutterCompositor*)createFlutterCompositor { #pragma mark - Framework-internal methods - (void)addViewController:(FlutterViewController*)controller { - // Adding a view controller when there is no implicit view should always - // assign it to the implicit view controller. + // FlutterEngine can only handle the implicit view for now. Adding more views + // throws an assertion. NSAssert(self.viewController == nil, @"The engine already has a view controller for the implicit view."); self.viewController = controller; From 850a4683c0049c06c63af670b1ae51c92446c8f2 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 11:21:48 -0700 Subject: [PATCH 4/7] Remove extra changes --- shell/platform/darwin/macos/framework/Source/FlutterEngine.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 22fbf2f3348dc..308a96af77000 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -562,7 +562,6 @@ - (void)dealloc { } } } - // Clear any published values, just in case a plugin has created a retain cycle with the // registrar. for (NSString* pluginName in _pluginRegistrars) { From 84ba335b76bafb21017ce892adb85c01286d79c9 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 11:23:36 -0700 Subject: [PATCH 5/7] Better doc --- .../macos/framework/Source/FlutterEngine_Internal.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index 23f2cd0421e5c..e62c64d6aeb50 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -125,10 +125,10 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { /** * Attach a view controller to the engine as its default controller. * - * Practically, since FlutterEngine can only be attached with one controller, - * the given controller, if successfully attached, will always have the default - * view ID kFlutterImplicitViewId. If the engine already has an implicit view, - * this call throws an assertion. + * Since FlutterEngine can only handle the implicit view for now, the given + * controller will always be assigned to the implicit view, if there isn't an + * implicit view yet. If the engine already has an implicit view, this call + * throws an assertion. * * The engine holds a weak reference to the attached view controller. * @@ -147,9 +147,6 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { * * If the view controller is not associated with this engine, this call throws an * assertion. - * - * Practically, since FlutterEngine can only be attached with one controller for - * now, the given controller must be the current view controller. */ - (void)removeViewController:(FlutterViewController*)viewController; From 6f459e7a649a85ccc12943152615afbc81d196af Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 11:24:01 -0700 Subject: [PATCH 6/7] Remove extra space --- .../darwin/macos/framework/Source/FlutterEngine_Internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index e62c64d6aeb50..2d63ee2c47c93 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -127,7 +127,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { * * Since FlutterEngine can only handle the implicit view for now, the given * controller will always be assigned to the implicit view, if there isn't an - * implicit view yet. If the engine already has an implicit view, this call + * implicit view yet. If the engine already has an implicit view, this call * throws an assertion. * * The engine holds a weak reference to the attached view controller. From 6e810f125f486df459330f8324944e7a08359e18 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 19 Apr 2024 11:35:30 -0700 Subject: [PATCH 7/7] Fix some docs --- .../macos/framework/Source/FlutterEngine.mm | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 308a96af77000..b7dd26aedf420 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -742,10 +742,14 @@ - (void)registerViewController:(FlutterViewController*)controller viewIdentifier:viewIdentifier threadSynchronizer:_threadSynchronizer]; NSAssert(controller.viewIdentifier == viewIdentifier, @"Failed to assign view ID."); - NSAssert(controller.attached && controller.engine == self, - @"The FlutterViewController unexpectedly stays unattached after being added. " - @"In unit tests, this is likely because either the FlutterViewController or " - @"the FlutterEngine is mocked. Please subclass these classes instead."); + // Verify that the controller's property are updated accordingly. Failing the + // assertions is likely because either the FlutterViewController or the + // FlutterEngine is mocked. Please subclass these classes instead. + NSAssert(controller.attached, @"The FlutterViewController should switch to the attached mode " + @"after it is added to a FlutterEngine."); + NSAssert(controller.engine == self, + @"The FlutterViewController was added to %@, but its engine unexpectedly became %@.", + self, controller.engine); if (controller.viewLoaded) { [self viewControllerViewDidLoad:controller]; @@ -781,8 +785,8 @@ - (void)viewControllerViewDidLoad:(FlutterViewController*)viewController { - (void)deregisterViewControllerForIdentifier:(FlutterViewIdentifier)viewIdentifier { FlutterViewController* controller = [self viewControllerForIdentifier:viewIdentifier]; - // The controller might be nil, because the engine only stores a weak ref, and - // this method might have been called from the controller's dealloc. + // The controller can be nil. The engine stores only a weak ref, and this + // method could have been called from the controller's dealloc. if (controller != nil) { [controller detachFromEngine]; NSAssert(!controller.attached,