From d3d20334899a7fbf61d535e8146e279d5c955064 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Wed, 15 Mar 2023 13:03:35 -0700 Subject: [PATCH] [macOS] Rename FlutterViewController.id to viewId `id` is a type in Objective-C similar to dynamic in Dart, and while it *can* be used as an identifier, it's bad practice to do so and results in unidiomatic code. `viewId` is also more clearly indicates the purpose of this identifier. Also replaces the use of @dynamic (which is meant to indicate properties whose implementation is either added at runtime or stashed away in a superclass) with @synthesize, which is an idiomatic way to declare that an ivar `_foo` should be synthesized for property `foo`. While this is renaming a public property and thus constitutes a breaking change, multi-view support is still under development and thus, practically speaking, this should not be a breaking change. --- .../framework/Headers/FlutterViewController.h | 2 +- .../darwin/macos/framework/Source/FlutterEngine.mm | 14 +++++++------- .../macos/framework/Source/FlutterEngineTest.mm | 10 +++++----- .../framework/Source/FlutterViewController.mm | 12 +++++------- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index 5979131f7451b..62ea527611d0d 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -61,7 +61,7 @@ FLUTTER_DARWIN_EXPORT * If the view controller is unattached (see FlutterViewController#attached), * reading this property throws an assertion. */ -@property(nonatomic, readonly) uint64_t id; +@property(nonatomic, readonly) uint64_t viewId; /** * The style of mouse tracking to use for the view. Defaults to diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 6c33c3dd372eb..c82153b818c45 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -556,7 +556,7 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(uint64_ @"The incoming view controller is already attached to an engine."); NSAssert([_viewControllers objectForKey:@(viewId)] == nil, @"The requested view ID is occupied."); [controller attachToEngine:self withId:viewId]; - NSAssert(controller.id == viewId, @"Failed to assign view ID."); + NSAssert(controller.viewId == viewId, @"Failed to assign view ID."); [_viewControllers setObject:controller forKey:@(viewId)]; } @@ -576,7 +576,7 @@ - (void)shutDownIfNeeded { - (FlutterViewController*)viewControllerForId:(uint64_t)viewId { FlutterViewController* controller = [_viewControllers objectForKey:@(viewId)]; - NSAssert(controller == nil || controller.id == viewId, + NSAssert(controller == nil || controller.viewId == viewId, @"The stored controller has unexpected view ID."); return controller; } @@ -598,8 +598,8 @@ - (void)setViewController:(FlutterViewController*)controller { controller.engine); [self registerViewController:controller forId:kFlutterDefaultViewId]; } else if (currentController != nil && controller == nil) { - NSAssert(currentController.id == kFlutterDefaultViewId, - @"The default controller has an unexpected ID %llu", currentController.id); + NSAssert(currentController.viewId == kFlutterDefaultViewId, + @"The default controller has an unexpected ID %llu", currentController.viewId); // From non-nil to nil. [self deregisterViewControllerForId:kFlutterDefaultViewId]; [self shutDownIfNeeded]; @@ -669,7 +669,7 @@ - (void)addViewController:(FlutterViewController*)controller { - (void)removeViewController:(nonnull FlutterViewController*)viewController { NSAssert([viewController attached] && viewController.engine == self, @"The given view controller is not associated with this engine."); - [self deregisterViewControllerForId:viewController.id]; + [self deregisterViewControllerForId:viewController.viewId]; [self shutDownIfNeeded]; } @@ -733,7 +733,7 @@ - (nonnull NSString*)executableName { } - (void)updateWindowMetricsForViewController:(FlutterViewController*)viewController { - if (viewController.id != kFlutterDefaultViewId) { + if (viewController.viewId != kFlutterDefaultViewId) { // TODO(dkwingsmt): The embedder API only supports single-view for now. As // embedder APIs are converted to multi-view, this method should support any // views. @@ -742,7 +742,7 @@ - (void)updateWindowMetricsForViewController:(FlutterViewController*)viewControl if (!_engine || !viewController || !viewController.viewLoaded) { return; } - NSAssert([self viewControllerForId:viewController.id] == viewController, + NSAssert([self viewControllerForId:viewController.viewId] == viewController, @"The provided view controller is not attached to this engine."); NSView* view = viewController.flutterView; CGRect scaledBounds = [view convertRectToBacking:view.bounds]; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index fc105dd710bdd..101174cfd0302 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -646,7 +646,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable @autoreleasepool { // Create FVC1. viewController1 = [[FlutterViewController alloc] initWithProject:project]; - EXPECT_EQ(viewController1.id, 0ull); + EXPECT_EQ(viewController1.viewId, 0ull); engine = viewController1.engine; engine.viewController = nil; @@ -663,7 +663,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable engine.viewController = viewController1; EXPECT_EQ(engine.viewController, viewController1); - EXPECT_EQ(viewController1.id, 0ull); + EXPECT_EQ(viewController1.viewId, 0ull); } TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByEngine) { @@ -677,7 +677,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable @autoreleasepool { viewController1 = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - EXPECT_EQ(viewController1.id, 0ull); + EXPECT_EQ(viewController1.viewId, 0ull); EXPECT_EQ(engine.viewController, viewController1); engine.viewController = nil; @@ -685,7 +685,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable FlutterViewController* viewController2 = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - EXPECT_EQ(viewController2.id, 0ull); + EXPECT_EQ(viewController2.viewId, 0ull); EXPECT_EQ(engine.viewController, viewController2); } // FVC2 is deallocated but FVC1 is retained. @@ -694,7 +694,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable engine.viewController = viewController1; EXPECT_EQ(engine.viewController, viewController1); - EXPECT_EQ(viewController1.id, 0ull); + EXPECT_EQ(viewController1.viewId, 0ull); } TEST_F(FlutterEngineTest, HandlesTerminationRequest) { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index b9999c0cfae01..3356a9852e970 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -315,11 +315,9 @@ @implementation FlutterViewController { FlutterDartProject* _project; std::shared_ptr _bridge; - - uint64_t _id; } -@dynamic id; +@synthesize viewId = _viewId; @dynamic view; @dynamic accessibilityBridge; @@ -341,7 +339,7 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) @"The FlutterViewController unexpectedly stays unattached after initialization. " @"In unit tests, this is likely because either the FlutterViewController or " @"the FlutterEngine is mocked. Please subclass these classes instead.", - controller.engine, controller.id); + controller.engine, controller.viewId); controller->_mouseTrackingMode = FlutterMouseTrackingModeInKeyWindow; controller->_textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:controller]; [controller initializeKeyboard]; @@ -457,9 +455,9 @@ - (void)setBackgroundColor:(NSColor*)color { [_flutterView setBackgroundColor:_backgroundColor]; } -- (uint64_t)id { +- (uint64_t)viewId { NSAssert([self attached], @"This view controller is not attched."); - return _id; + return _viewId; } - (void)onPreEngineRestart { @@ -489,7 +487,7 @@ - (void)notifySemanticsEnabledChanged { - (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId { NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine); _engine = engine; - _id = viewId; + _viewId = viewId; } - (void)detachFromEngine {