From 48a4c6620cb2fca4efb3c8670121d81cc3d49db6 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Jan 2023 09:35:31 -0800 Subject: [PATCH 01/12] Impl --- .../macos/framework/Headers/FlutterEngine.h | 9 + .../framework/Headers/FlutterViewController.h | 13 ++ .../Source/AccessibilityBridgeMacTest.mm | 33 +--- .../macos/framework/Source/FlutterEngine.mm | 47 +++-- .../framework/Source/FlutterEngineTest.mm | 89 +-------- .../FlutterPlatformNodeDelegateMacTest.mm | 35 ++-- .../Source/FlutterTextInputPluginTest.mm | 176 ++++++++---------- .../macos/framework/Source/FlutterView.h | 2 +- .../framework/Source/FlutterViewController.mm | 62 +++--- .../Source/FlutterViewControllerTest.mm | 17 +- .../Source/FlutterViewController_Internal.h | 9 + .../Source/FlutterViewEngineProviderTest.mm | 4 +- 12 files changed, 218 insertions(+), 278 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h b/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h index 49032f87d3a91..af17c47fe7db3 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h +++ b/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; diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index 4dea666e1262c..80f675eef34c1 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -4,6 +4,8 @@ #import +#include + #import "FlutterEngine.h" #import "FlutterMacros.h" #import "FlutterPlatformViews.h" @@ -31,9 +33,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. @@ -65,6 +77,7 @@ FLUTTER_DARWIN_EXPORT - (nonnull instancetype)initWithEngine:(nonnull FlutterEngine*)engine nibName:(nullable NSString*)nibName bundle:(nullable NSBundle*)nibBundle NS_DESIGNATED_INITIALIZER; + /** * Invoked by the engine right before the engine is restarted. * diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm index 0ad11af0e3e8f..08c47c4a32397 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm +++ b/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" + 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. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 870ca4802f3d9..a2c99bed9043b 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -80,6 +80,8 @@ @interface FlutterEngine () */ @property(nonatomic, strong) NSMutableArray* isResponseValid; +- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId; + /** * Sends the list of user-preferred locales to the Flutter engine. */ @@ -411,27 +413,25 @@ - (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 { + NSAssert(NO, @"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(_viewProvider, _platformViewController); @@ -539,10 +539,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 +603,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 +690,7 @@ - (void)shutDownEngine { return; } - if (_viewController && _viewController.flutterView) { - [_viewController.flutterView shutdown]; - } + [self.viewController.flutterView shutdown]; FlutterEngineResult result = _embedderAPI.Deinitialize(_engine); if (result != kSuccess) { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index a162fbb64c3cf..d9c18b2689e09 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -337,6 +337,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 +357,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 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(0); - root.actions = static_cast(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(0); - child1.actions = static_cast(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; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm index a5809bef78d92..84372964f9501 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm @@ -19,18 +19,19 @@ namespace flutter::testing { namespace { -// Returns an engine configured for the text fixture resource configuration. -FlutterEngine* CreateTestEngine() { +// Returns a view controller configured for the text fixture resource configuration. +FlutterViewController* CreateTestViewController() { NSString* fixtures = @(testing::GetFixturesPath()); FlutterDartProject* project = [[FlutterDartProject alloc] initWithAssetsPath:fixtures ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]]; - return [[FlutterEngine alloc] initWithName:@"test" project:project allowHeadlessExecution:true]; + return [[FlutterViewController alloc] initWithProject:project]; } } // namespace TEST(FlutterPlatformNodeDelegateMac, Basics) { - FlutterEngine* engine = CreateTestEngine(); + FlutterViewController* viewController = CreateTestViewController(); + FlutterEngine* engine = viewController.engine; engine.semanticsEnabled = YES; auto bridge = engine.accessibilityBridge.lock(); // Initialize ax node data. @@ -65,7 +66,8 @@ } TEST(FlutterPlatformNodeDelegateMac, SelectableTextHasCorrectSemantics) { - FlutterEngine* engine = CreateTestEngine(); + FlutterViewController* viewController = CreateTestViewController(); + FlutterEngine* engine = viewController.engine; engine.semanticsEnabled = YES; auto bridge = engine.accessibilityBridge.lock(); // Initialize ax node data. @@ -106,7 +108,8 @@ } TEST(FlutterPlatformNodeDelegateMac, SelectableTextWithoutSelectionReturnZeroRange) { - FlutterEngine* engine = CreateTestEngine(); + FlutterViewController* viewController = CreateTestViewController(); + FlutterEngine* engine = viewController.engine; engine.semanticsEnabled = YES; auto bridge = engine.accessibilityBridge.lock(); // Initialize ax node data. @@ -144,15 +147,8 @@ // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) TEST(FlutterPlatformNodeDelegateMac, CanPerformAction) { - FlutterEngine* engine = CreateTestEngine(); - - // 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]; - [engine setViewController:viewController]; + FlutterViewController* viewController = CreateTestViewController(); + FlutterEngine* engine = viewController.engine; // Attach the view to a NSWindow. NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) @@ -222,14 +218,9 @@ // NOLINTEND(clang-analyzer-core.StackAddressEscape) TEST(FlutterPlatformNodeDelegateMac, TextFieldUsesFlutterTextField) { - 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]; // Unit test localization is unnecessary. // NOLINTNEXTLINE(clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index 9f4a39d4b3818..6a1dd6611f248 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -4,6 +4,7 @@ #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h" @@ -37,6 +38,16 @@ @interface NSTextInputContext (Private) - (BOOL)isActive; @end +@interface TextInputTestViewController : FlutterViewController +@end + +@implementation TextInputTestViewController +- (nonnull FlutterView*)createFlutterViewWithMTLDevice:(id)device + commandQueue:(id)commandQueue { + return OCMClassMock([NSView class]); +} +@end + @interface FlutterInputPluginTestObjc : NSObject - (bool)testEmptyCompositionRange; - (bool)testClearClientDuringComposing; @@ -45,7 +56,7 @@ - (bool)testClearClientDuringComposing; @implementation FlutterInputPluginTestObjc - (bool)testEmptyCompositionRange { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -110,7 +121,7 @@ - (bool)testEmptyCompositionRange { } - (bool)testSetMarkedTextWithSelectionChange { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -178,7 +189,7 @@ - (bool)testSetMarkedTextWithSelectionChange { } - (bool)testSetMarkedTextWithReplacementRange { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -246,7 +257,7 @@ - (bool)testSetMarkedTextWithReplacementRange { } - (bool)testComposingRegionRemovedByFramework { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -324,7 +335,7 @@ - (bool)testComposingRegionRemovedByFramework { - (bool)testClearClientDuringComposing { // Set up FlutterTextInputPlugin. - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -377,26 +388,18 @@ - (bool)testClearClientDuringComposing { } - (bool)testFirstRectForCharacterRange { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) .andReturn(binaryMessengerMock); - FlutterViewController* controllerMock = OCMClassMock([FlutterViewController class]); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [controllerMock engine]) - .andReturn(engineMock); - - id viewMock = OCMClassMock([NSView class]); + FlutterViewController* controllerMock = + [[TextInputTestViewController alloc] initWithEngine:engineMock nibName:nil bundle:nil]; + [controllerMock loadView]; + id viewMock = controllerMock.flutterView; OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [viewMock bounds]) .andReturn(NSMakeRect(0, 0, 200, 200)); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - controllerMock.viewLoaded) - .andReturn(YES); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [controllerMock flutterView]) - .andReturn(viewMock); id windowMock = OCMClassMock([NSWindow class]); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) @@ -407,72 +410,65 @@ - (bool)testFirstRectForCharacterRange { [viewMock convertRect:NSMakeRect(28, 10, 2, 19) toView:nil]) .andReturn(NSMakeRect(28, 10, 2, 19)); - OCMExpect( // NOLINT(google-objc-avoid-throwing-exception) - [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]) - .andReturn(NSMakeRect(38, 20, 2, 19)); - - FlutterTextInputPlugin* plugin = - [[FlutterTextInputPlugin alloc] initWithViewController:controllerMock]; - - FlutterMethodCall* call = [FlutterMethodCall - methodCallWithMethodName:@"TextInput.setEditableSizeAndTransform" - arguments:@{ - @"height" : @(20.0), - @"transform" : @[ - @(1.0), @(0.0), @(0.0), @(0.0), @(0.0), @(1.0), @(0.0), @(0.0), @(0.0), - @(0.0), @(1.0), @(0.0), @(20.0), @(10.0), @(0.0), @(1.0) - ], - @"width" : @(400.0), - }]; - - [plugin handleMethodCall:call - result:^(id){ - }]; - - call = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setCaretRect" - arguments:@{ - @"height" : @(19.0), - @"width" : @(2.0), - @"x" : @(8.0), - @"y" : @(0.0), - }]; - - [plugin handleMethodCall:call - result:^(id){ - }]; - - NSRect rect = [plugin firstRectForCharacterRange:NSMakeRange(0, 0) actualRange:nullptr]; - @try { - OCMVerify( // NOLINT(google-objc-avoid-throwing-exception) - [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]); - } @catch (...) { - return false; - } - - return NSEqualRects(rect, NSMakeRect(38, 20, 2, 19)); + // OCMExpect( // NOLINT(google-objc-avoid-throwing-exception) + // [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]) + // .andReturn(NSMakeRect(38, 20, 2, 19)); + + // FlutterTextInputPlugin* plugin = + // [[FlutterTextInputPlugin alloc] initWithViewController:controllerMock]; + + // FlutterMethodCall* call = [FlutterMethodCall + // methodCallWithMethodName:@"TextInput.setEditableSizeAndTransform" + // arguments:@{ + // @"height" : @(20.0), + // @"transform" : @[ + // @(1.0), @(0.0), @(0.0), @(0.0), @(0.0), @(1.0), @(0.0), @(0.0), @(0.0), + // @(0.0), @(1.0), @(0.0), @(20.0), @(10.0), @(0.0), @(1.0) + // ], + // @"width" : @(400.0), + // }]; + + // [plugin handleMethodCall:call + // result:^(id){ + // }]; + + // call = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setCaretRect" + // arguments:@{ + // @"height" : @(19.0), + // @"width" : @(2.0), + // @"x" : @(8.0), + // @"y" : @(0.0), + // }]; + + // [plugin handleMethodCall:call + // result:^(id){ + // }]; + + // NSRect rect = [plugin firstRectForCharacterRange:NSMakeRange(0, 0) actualRange:nullptr]; + // @try { + // OCMVerify( // NOLINT(google-objc-avoid-throwing-exception) + // [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]); + // } @catch (...) { + // return false; + // } + + // return NSEqualRects(rect, NSMakeRect(38, 20, 2, 19)); + return true; } - (bool)testFirstRectForCharacterRangeAtInfinity { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) .andReturn(binaryMessengerMock); - FlutterViewController* controllerMock = OCMClassMock([FlutterViewController class]); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [controllerMock engine]) - .andReturn(engineMock); - - id viewMock = OCMClassMock([NSView class]); + FlutterViewController* controllerMock = + [[TextInputTestViewController alloc] initWithEngine:engineMock nibName:nil bundle:nil]; + [controllerMock loadView]; + id viewMock = controllerMock.flutterView; OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [viewMock bounds]) .andReturn(NSMakeRect(0, 0, 200, 200)); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - controllerMock.viewLoaded) - .andReturn(YES); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [controllerMock flutterView]) - .andReturn(viewMock); id windowMock = OCMClassMock([NSWindow class]); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) @@ -515,26 +511,18 @@ - (bool)testFirstRectForCharacterRangeAtInfinity { } - (bool)testFirstRectForCharacterRangeWithEsotericAffineTransform { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) .andReturn(binaryMessengerMock); - FlutterViewController* controllerMock = OCMClassMock([FlutterViewController class]); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [controllerMock engine]) - .andReturn(engineMock); - - id viewMock = OCMClassMock([NSView class]); + FlutterViewController* controllerMock = + [[TextInputTestViewController alloc] initWithEngine:engineMock nibName:nil bundle:nil]; + [controllerMock loadView]; + id viewMock = controllerMock.flutterView; OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [viewMock bounds]) .andReturn(NSMakeRect(0, 0, 200, 200)); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - controllerMock.viewLoaded) - .andReturn(YES); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [controllerMock flutterView]) - .andReturn(viewMock); id windowMock = OCMClassMock([NSWindow class]); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) @@ -595,7 +583,7 @@ - (bool)testFirstRectForCharacterRangeWithEsotericAffineTransform { } - (bool)testSetEditingStateWithTextEditingDelta { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -651,7 +639,7 @@ - (bool)testSetEditingStateWithTextEditingDelta { } - (bool)testOperationsThatTriggerDelta { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -768,7 +756,7 @@ - (bool)testOperationsThatTriggerDelta { } - (bool)testComposingWithDelta { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -1005,7 +993,7 @@ - (bool)testComposingWithDelta { } - (bool)testComposingWithDeltasWhenSelectionIsActive { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -1134,7 +1122,7 @@ - (bool)testPerformKeyEquivalent { } - (bool)unhandledKeyEquivalent { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -1210,7 +1198,7 @@ - (bool)unhandledKeyEquivalent { } - (bool)testLocalTextAndSelectionUpdateAfterDelta { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -1271,7 +1259,7 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta { } - (bool)testSelectorsAreForwardedToFramework { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index ba70833b46b76..5feaac36845f5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -18,7 +18,7 @@ * 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. */ -constexpr uint64_t kFlutterDefaultViewId = 0; +constexpr uint64_t kFlutterDefaultViewId = 1; /** * Listener for view resizing. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 9034e8deaa6c1..96d6586166777 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -293,14 +293,20 @@ @implementation FlutterViewController { /** * Performs initialization that's common between the different init paths. */ -static void CommonInit(FlutterViewController* controller) { - if (!controller->_engine) { - controller->_engine = [[FlutterEngine alloc] initWithName:@"io.flutter" - project:controller->_project - allowHeadlessExecution:NO]; +static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) { + if (!engine) { + engine = [[FlutterEngine alloc] initWithName:@"io.flutter" + project:controller->_project + allowHeadlessExecution:NO]; } + NSCAssert(controller.engine == nil && controller.id == 0, + @"Incorrect starting condition: engine %@ ID %llu", controller.engine, controller.id); + engine.viewController = controller; + NSCAssert(controller.engine != nil && controller.id != 0, + @"Incorrect ending condition: engine %@ ID %llu", controller.engine, controller.id); controller->_mouseTrackingMode = FlutterMouseTrackingModeInKeyWindow; controller->_textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:controller]; + [controller initializeKeyboard]; // macOS fires this message when changing IMEs. CFNotificationCenterRef cfCenter = CFNotificationCenterGetDistributedCenter(); __weak FlutterViewController* weakSelf = controller; @@ -313,7 +319,7 @@ - (instancetype)initWithCoder:(NSCoder*)coder { self = [super initWithCoder:coder]; NSAssert(self, @"Super init cannot be nil"); - CommonInit(self); + CommonInit(self, nil); return self; } @@ -321,7 +327,7 @@ - (instancetype)initWithNibName:(NSString*)nibNameOrNil bundle:(NSBundle*)nibBun self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil]; NSAssert(self, @"Super init cannot be nil"); - CommonInit(self); + CommonInit(self, nil); return self; } @@ -330,7 +336,7 @@ - (instancetype)initWithProject:(nullable FlutterDartProject*)project { NSAssert(self, @"Super init cannot be nil"); _project = project; - CommonInit(self); + CommonInit(self, nil); return self; } @@ -346,13 +352,7 @@ - (instancetype)initWithEngine:(nonnull FlutterEngine*)engine self = [super initWithNibName:nibName bundle:nibBundle]; if (self) { - _engine = engine; - CommonInit(self); - if (engine.running) { - [self loadView]; - engine.viewController = self; - [self initializeKeyboard]; - } + CommonInit(self, engine); } return self; @@ -370,9 +370,7 @@ - (void)loadView { NSLog(@"Unable to create FlutterView; no MTLDevice or MTLCommandQueue available."); return; } - flutterView = [[FlutterView alloc] initWithMTLDevice:device - commandQueue:commandQueue - reshapeListener:self]; + flutterView = [self createFlutterViewWithMTLDevice:device commandQueue:commandQueue]; if (_backgroundColor != nil) { [flutterView setBackgroundColor:_backgroundColor]; } @@ -427,12 +425,27 @@ - (void)onPreEngineRestart { [self initializeKeyboard]; } +- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId { + // TODO(dkwingsmt): We are allowing engine not running here because a lot of + // tests creates an FVC using initWithProject:nibName:bundle: and set the FVC + // to the engine. We should migrate these tests to initWithEngine: and stop + // supporting all non-nil engines here. + NSAssert((_engine == nil && _id == 0) || ![_engine running], + @"Already attached to an running engine, engine %@ ID %llu.", _engine, _id); + _engine = engine; + _id = viewId; +} + +- (void)detachFromEngine { + NSAssert(_engine != nil && _id != 0, @"Not attached to an engine, engine %@ ID %llu.", _engine, + _id); + _engine = nil; + _id = 0; +} + #pragma mark - Private methods - (BOOL)launchEngine { - [self initializeKeyboard]; - - _engine.viewController = self; if (![_engine runWithEntrypoint:nil]) { return NO; } @@ -689,6 +702,13 @@ - (void)onAccessibilityStatusChanged:(BOOL)enabled { } } +- (nonnull FlutterView*)createFlutterViewWithMTLDevice:(id)device + commandQueue:(id)commandQueue { + return [[FlutterView alloc] initWithMTLDevice:device + commandQueue:commandQueue + reshapeListener:self]; +} + - (void)onKeyboardLayoutChanged { _keyboardLayoutData = nil; if (_keyboardLayoutNotifier != nil) { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index 903065731a31a..839c16462fc8e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -10,6 +10,7 @@ #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTestUtils.h" @@ -174,7 +175,7 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, @implementation FlutterViewControllerTestObjC - (bool)testKeyEventsAreSentToFramework { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -212,7 +213,7 @@ - (bool)testKeyEventsAreSentToFramework { } - (bool)testKeyEventsArePropagatedIfNotHandled { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -267,7 +268,7 @@ - (bool)testKeyEventsArePropagatedIfNotHandled { } - (bool)testFlutterViewIsConfigured { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); FlutterRenderer* renderer_ = [[FlutterRenderer alloc] initWithFlutterEngine:engineMock]; OCMStub([engineMock renderer]).andReturn(renderer_); @@ -288,7 +289,7 @@ - (bool)testFlutterViewIsConfigured { } - (bool)testFlagsChangedEventsArePropagatedIfNotHandled { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -341,7 +342,7 @@ - (bool)testFlagsChangedEventsArePropagatedIfNotHandled { } - (bool)testKeyEventsAreNotPropagatedIfHandled { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -396,7 +397,7 @@ - (bool)testKeyEventsAreNotPropagatedIfHandled { } - (bool)testKeyboardIsRestartedOnEngineRestart { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); OCMStub( // NOLINT(google-objc-avoid-throwing-exception) [engineMock binaryMessenger]) @@ -458,7 +459,7 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event } - (bool)testTrackpadGesturesAreSentToFramework { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); // Need to return a real renderer to allow view controller to load. FlutterRenderer* renderer_ = [[FlutterRenderer alloc] initWithFlutterEngine:engineMock]; OCMStub([engineMock renderer]).andReturn(renderer_); @@ -754,7 +755,7 @@ - (bool)testTrackpadGesturesAreSentToFramework { } - (bool)testViewWillAppearCalledMultipleTimes { - id engineMock = OCMClassMock([FlutterEngine class]); + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock nibName:@"" bundle:nil]; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index 19870cf5b183b..5571fd2794693 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -4,6 +4,7 @@ #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardViewDelegate.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" @@ -23,9 +24,17 @@ */ - (BOOL)isDispatchingKeyEvent:(nonnull NSEvent*)event; +- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId; + +- (void)detachFromEngine; + @end // Private methods made visible for testing @interface FlutterViewController (TestMethods) - (void)onAccessibilityStatusChanged:(BOOL)enabled; + +- (nonnull FlutterView*)createFlutterViewWithMTLDevice:(nonnull id)device + commandQueue:(nonnull id)commandQueue; + @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm index 6a1aa66b69c59..27d9721afc8e9 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm @@ -29,13 +29,13 @@ viewProvider = [[FlutterViewEngineProvider alloc] initWithEngine:mockEngine]; // When the view controller is not set, the returned view is nil. - EXPECT_EQ([viewProvider getView:0], nil); + EXPECT_EQ([viewProvider getView:1], nil); // When the view controller is set, the returned view is the controller's view. mockFlutterViewController = OCMStrictClassMock([FlutterViewController class]); id mockView = OCMStrictClassMock([FlutterView class]); OCMStub([mockFlutterViewController flutterView]).andReturn(mockView); - EXPECT_EQ([viewProvider getView:0], mockView); + EXPECT_EQ([viewProvider getView:1], mockView); } } // namespace flutter::testing From 7f10f9d317889a06f01576da3844865318ec2969 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Jan 2023 09:35:48 -0800 Subject: [PATCH 02/12] Format --- .../macos/framework/Source/AccessibilityBridgeMacTest.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm index 08c47c4a32397..d493da6461719 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm @@ -57,8 +57,8 @@ @implementation AccessibilityBridgeTestEngine initWithAssetsPath:fixtures ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]]; FlutterEngine* engine = [[AccessibilityBridgeTestEngine alloc] initWithName:@"test" - project:project - allowHeadlessExecution:true]; + project:project + allowHeadlessExecution:true]; return [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; } } // namespace From 1d02bc97a2ec66d8f5eda7a26184df9cd677a5f5 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Jan 2023 10:24:56 -0800 Subject: [PATCH 03/12] Fix provider --- .../darwin/macos/framework/Headers/FlutterViewController.h | 3 --- .../darwin/macos/framework/Source/FlutterEngine.mm | 7 ++----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index 80f675eef34c1..afd812a1815d0 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -4,8 +4,6 @@ #import -#include - #import "FlutterEngine.h" #import "FlutterMacros.h" #import "FlutterPlatformViews.h" @@ -77,7 +75,6 @@ FLUTTER_DARWIN_EXPORT - (nonnull instancetype)initWithEngine:(nonnull FlutterEngine*)engine nibName:(nullable NSString*)nibName bundle:(nullable NSBundle*)nibBundle NS_DESIGNATED_INITIALIZER; - /** * Invoked by the engine right before the engine is restarted. * diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index a2c99bed9043b..1d05394e882f8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -215,8 +215,6 @@ @implementation FlutterEngine { // when the engine is destroyed. std::unique_ptr _macOSCompositor; - FlutterViewEngineProvider* _viewProvider; - // FlutterCompositor is copied and used in embedder.cc. FlutterCompositor _compositor; @@ -250,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]; @@ -432,8 +429,8 @@ - (void)setViewController:(FlutterViewController*)controller { } - (FlutterCompositor*)createFlutterCompositor { - _macOSCompositor = - std::make_unique(_viewProvider, _platformViewController); + _macOSCompositor = std::make_unique( + [[FlutterViewEngineProvider alloc] initWithEngine:self], _platformViewController); _compositor = {}; _compositor.struct_size = sizeof(FlutterCompositor); From dc0158447f24dd5ebe4cb73296d384430ce7d4e7 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Jan 2023 10:27:27 -0800 Subject: [PATCH 04/12] Uncomment test --- .../Source/FlutterTextInputPluginTest.mm | 87 +++++++++---------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index 6a1dd6611f248..b6e7cbb549bf6 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -410,50 +410,49 @@ - (bool)testFirstRectForCharacterRange { [viewMock convertRect:NSMakeRect(28, 10, 2, 19) toView:nil]) .andReturn(NSMakeRect(28, 10, 2, 19)); - // OCMExpect( // NOLINT(google-objc-avoid-throwing-exception) - // [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]) - // .andReturn(NSMakeRect(38, 20, 2, 19)); - - // FlutterTextInputPlugin* plugin = - // [[FlutterTextInputPlugin alloc] initWithViewController:controllerMock]; - - // FlutterMethodCall* call = [FlutterMethodCall - // methodCallWithMethodName:@"TextInput.setEditableSizeAndTransform" - // arguments:@{ - // @"height" : @(20.0), - // @"transform" : @[ - // @(1.0), @(0.0), @(0.0), @(0.0), @(0.0), @(1.0), @(0.0), @(0.0), @(0.0), - // @(0.0), @(1.0), @(0.0), @(20.0), @(10.0), @(0.0), @(1.0) - // ], - // @"width" : @(400.0), - // }]; - - // [plugin handleMethodCall:call - // result:^(id){ - // }]; - - // call = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setCaretRect" - // arguments:@{ - // @"height" : @(19.0), - // @"width" : @(2.0), - // @"x" : @(8.0), - // @"y" : @(0.0), - // }]; - - // [plugin handleMethodCall:call - // result:^(id){ - // }]; - - // NSRect rect = [plugin firstRectForCharacterRange:NSMakeRange(0, 0) actualRange:nullptr]; - // @try { - // OCMVerify( // NOLINT(google-objc-avoid-throwing-exception) - // [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]); - // } @catch (...) { - // return false; - // } - - // return NSEqualRects(rect, NSMakeRect(38, 20, 2, 19)); - return true; + OCMExpect( // NOLINT(google-objc-avoid-throwing-exception) + [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]) + .andReturn(NSMakeRect(38, 20, 2, 19)); + + FlutterTextInputPlugin* plugin = + [[FlutterTextInputPlugin alloc] initWithViewController:controllerMock]; + + FlutterMethodCall* call = [FlutterMethodCall + methodCallWithMethodName:@"TextInput.setEditableSizeAndTransform" + arguments:@{ + @"height" : @(20.0), + @"transform" : @[ + @(1.0), @(0.0), @(0.0), @(0.0), @(0.0), @(1.0), @(0.0), @(0.0), @(0.0), + @(0.0), @(1.0), @(0.0), @(20.0), @(10.0), @(0.0), @(1.0) + ], + @"width" : @(400.0), + }]; + + [plugin handleMethodCall:call + result:^(id){ + }]; + + call = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setCaretRect" + arguments:@{ + @"height" : @(19.0), + @"width" : @(2.0), + @"x" : @(8.0), + @"y" : @(0.0), + }]; + + [plugin handleMethodCall:call + result:^(id){ + }]; + + NSRect rect = [plugin firstRectForCharacterRange:NSMakeRange(0, 0) actualRange:nullptr]; + @try { + OCMVerify( // NOLINT(google-objc-avoid-throwing-exception) + [windowMock convertRectToScreen:NSMakeRect(28, 10, 2, 19)]); + } @catch (...) { + return false; + } + + return NSEqualRects(rect, NSMakeRect(38, 20, 2, 19)); } - (bool)testFirstRectForCharacterRangeAtInfinity { From fa5ccc0da77ba4e4986aec92c6a7cce4950a6b1c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Jan 2023 10:29:12 -0800 Subject: [PATCH 05/12] Format --- shell/platform/darwin/macos/framework/Source/FlutterEngine.mm | 3 ++- .../macos/framework/Source/FlutterTextInputPluginTest.mm | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 1d05394e882f8..e1646a0768ed4 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -424,7 +424,8 @@ - (void)setViewController:(FlutterViewController*)controller { [self shutDownEngine]; } } else { - NSAssert(NO, @"Replacing the view controller of the engine is not supported."); + NSLog(@"Failed to set view controller to the engine: " + @"Replacing the view controller of the engine is not supported."); } } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index b6e7cbb549bf6..7bf37280b7e75 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -415,7 +415,7 @@ - (bool)testFirstRectForCharacterRange { .andReturn(NSMakeRect(38, 20, 2, 19)); FlutterTextInputPlugin* plugin = - [[FlutterTextInputPlugin alloc] initWithViewController:controllerMock]; + [[FlutterTextInputPlugin alloc] initWithViewController:controllerMock]; FlutterMethodCall* call = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setEditableSizeAndTransform" From 764a15736a93f1b08ad942a15d6005e4313c036b Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Jan 2023 10:42:04 -0800 Subject: [PATCH 06/12] Fix all tests, banning running engine --- .../framework/Source/FlutterEngineTest.mm | 32 +++++++------------ .../Source/FlutterTextInputPluginTest.mm | 27 ++++++---------- .../FlutterTextInputSemanticsObjectTest.mm | 9 ++---- .../framework/Source/FlutterViewController.mm | 8 ++--- .../Source/FlutterViewControllerTest.mm | 8 ++--- 5 files changed, 29 insertions(+), 55 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index d9c18b2689e09..bf8da30fe1df8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/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 = @@ -380,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"]); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index 7bf37280b7e75..94c095d354d25 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -1401,13 +1401,10 @@ - (bool)testSelectorsAreForwardedToFramework { TEST(FlutterTextInputPluginTest, CanWorkWithFlutterTextField) { 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 = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; [viewController loadView]; - [engine setViewController:viewController]; // Create a NSWindow so that the native text field can become first responder. NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:NSBorderlessWindowMask @@ -1470,13 +1467,10 @@ - (bool)testSelectorsAreForwardedToFramework { TEST(FlutterTextInputPluginTest, CanNotBecomeResponderIfNoViewController) { 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 = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; [viewController loadView]; - [engine setViewController:viewController]; // Creates a NSWindow so that the native text field can become first responder. NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:NSBorderlessWindowMask @@ -1507,13 +1501,10 @@ - (bool)testSelectorsAreForwardedToFramework { TEST(FlutterTextInputPluginTest, IsAddedAndRemovedFromViewHierarchy) { 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 = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; [viewController loadView]; - [engine setViewController:viewController]; NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:NSBorderlessWindowMask diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm index 287b12713c24a..4e4b821a6115d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm @@ -28,13 +28,10 @@ TEST(FlutterTextInputSemanticsObjectTest, DoesInitialize) { 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 = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; [viewController loadView]; - [engine setViewController:viewController]; // Create a NSWindow so that the native text field can become first responder. NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:NSBorderlessWindowMask diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 96d6586166777..2605afac930c7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -426,12 +426,8 @@ - (void)onPreEngineRestart { } - (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId { - // TODO(dkwingsmt): We are allowing engine not running here because a lot of - // tests creates an FVC using initWithProject:nibName:bundle: and set the FVC - // to the engine. We should migrate these tests to initWithEngine: and stop - // supporting all non-nil engines here. - NSAssert((_engine == nil && _id == 0) || ![_engine running], - @"Already attached to an running engine, engine %@ ID %llu.", _engine, _id); + NSAssert(_engine == nil && _id == 0, @"Already attached to an engine, engine %@ ID %llu.", + _engine, _id); _engine = engine; _id = viewId; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index 839c16462fc8e..84f3401a76684 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -101,11 +101,9 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, TEST(FlutterViewController, ReparentsPluginWhenAccessibilityDisabled) { 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 = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; [viewController loadView]; [engine setViewController:viewController]; // Creates a NSWindow so that sub view can be first responder. From c74f450d9dc2f2acc23a14f57140d556b20b7894 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 19 Jan 2023 12:00:18 -0800 Subject: [PATCH 07/12] Doc --- .../macos/framework/Headers/FlutterViewController.h | 9 ++++++++- .../framework/Source/FlutterViewController_Internal.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index afd812a1815d0..32252f1f7565a 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -53,6 +53,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 + * this 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 for 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. */ @@ -66,7 +72,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. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index 5571fd2794693..c60c5354b8d8e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -24,8 +24,18 @@ */ - (BOOL)isDispatchingKeyEvent:(nonnull NSEvent*)event; +/** + * Set the `engine` and `id` of this controller. + * + * This method is called by FlutterEngine. + */ - (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId; +/** + * Reset the `engine` and `id` of this controller. + * + * This method is called by FlutterEngine. + */ - (void)detachFromEngine; @end From 320312b49d9f599b210a7189925792896089eb0d Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 19 Jan 2023 13:24:05 -0800 Subject: [PATCH 08/12] Better assertion --- .../darwin/macos/framework/Source/FlutterEngine.mm | 14 ++++++++++++-- .../framework/Source/FlutterViewController.mm | 8 ++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index e1646a0768ed4..8d39480c94d61 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -415,6 +415,12 @@ - (void)setViewController:(FlutterViewController*)controller { return; } if (_viewController == nil && controller != 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; [_viewController attachToEngine:self withId:kFlutterDefaultViewId]; } else if (_viewController != nil && controller == nil) { @@ -424,8 +430,12 @@ - (void)setViewController:(FlutterViewController*)controller { [self shutDownEngine]; } } else { - NSLog(@"Failed to set view controller to the engine: " - @"Replacing the view controller of the engine is not supported."); + 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); } } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 2605afac930c7..d790ad227eeb7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -300,10 +300,14 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) allowHeadlessExecution:NO]; } NSCAssert(controller.engine == nil && controller.id == 0, - @"Incorrect starting condition: engine %@ ID %llu", controller.engine, controller.id); + @"The FlutterViewController is unexpectedly attached to " + @"engine %@ with ID %llu before initialization.", + controller.engine, controller.id); engine.viewController = controller; NSCAssert(controller.engine != nil && controller.id != 0, - @"Incorrect ending condition: engine %@ ID %llu", controller.engine, controller.id); + @"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->_mouseTrackingMode = FlutterMouseTrackingModeInKeyWindow; controller->_textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:controller]; [controller initializeKeyboard]; From e47f697ed620dbb495064e9db6a2d0b153b6ec8f Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 24 Jan 2023 11:34:53 -0800 Subject: [PATCH 09/12] Comment --- .../darwin/macos/framework/Headers/FlutterEngine.h | 6 +++--- .../macos/framework/Headers/FlutterViewController.h | 8 ++++---- .../darwin/macos/framework/Source/FlutterEngine.mm | 3 +++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h b/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h index af17c47fe7db3..76d886c651673 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h @@ -71,11 +71,11 @@ FLUTTER_DARWIN_EXPORT * 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 + * 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 is prohibited and - * will throw an assertion error. + * 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; diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index 32252f1f7565a..7269528209de2 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -32,8 +32,8 @@ FLUTTER_DARWIN_EXPORT /** * The Flutter engine associated with this view controller. * - * The engine is strongly referenced by the FlutterViewController, and weakly - * vice versa. + * The engine is strongly referenced by the FlutterViewController, and the + * ViewController weakly from the engine. */ @property(nonatomic, nonnull, readonly) FlutterEngine* engine; @@ -54,9 +54,9 @@ FLUTTER_DARWIN_EXPORT * Initializes a controller that will run the given project. * * In this initializer, this controller creates an engine, and is attached to - * this engine as the default controller. In this way, this controller can not + * 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 for an existing engine, + * 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` diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 8d39480c94d61..b01fcfb9254e1 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -415,6 +415,7 @@ - (void)setViewController:(FlutterViewController*)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 %@. " @@ -424,12 +425,14 @@ - (void)setViewController:(FlutterViewController*)controller { _viewController = controller; [_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 %@. " From 570fca3161061862e422b104e02220aaabc5b7e3 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 24 Jan 2023 15:31:47 -0800 Subject: [PATCH 10/12] Assert id --- .../framework/Headers/FlutterViewController.h | 33 ++++++++++++++++--- .../macos/framework/Source/FlutterView.h | 2 +- .../framework/Source/FlutterViewController.mm | 27 ++++++++++----- .../Source/FlutterViewEngineProviderTest.mm | 4 +-- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index 7269528209de2..e4d8df7d4c48d 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -25,22 +25,41 @@ 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 /** * The Flutter engine associated with this view controller. - * - * The engine is strongly referenced by the FlutterViewController, and the - * ViewController weakly from the engine. */ @property(nonatomic, nonnull, readonly) FlutterEngine* engine; /** * The identifier for this view controller. * - * The ID is assigned when the view controller is added to FlutterEngine. + * 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; @@ -82,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. * diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index 5feaac36845f5..ba70833b46b76 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -18,7 +18,7 @@ * 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. */ -constexpr uint64_t kFlutterDefaultViewId = 1; +constexpr uint64_t kFlutterDefaultViewId = 0; /** * Listener for view resizing. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index d790ad227eeb7..d0a414b9a75ea 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -286,8 +286,11 @@ - (NSArray*)accessibilityChildren { @implementation FlutterViewController { // The project to run in this controller's engine. FlutterDartProject* _project; + + uint64_t _id; } +@dynamic id; @dynamic view; /** @@ -299,12 +302,12 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) project:controller->_project allowHeadlessExecution:NO]; } - NSCAssert(controller.engine == nil && controller.id == 0, + NSCAssert(controller.engine == nil, @"The FlutterViewController is unexpectedly attached to " - @"engine %@ with ID %llu before initialization.", - controller.engine, controller.id); + @"engine %@ before initialization.", + controller.engine); engine.viewController = controller; - NSCAssert(controller.engine != nil && controller.id != 0, + NSCAssert(controller.engine != nil, @"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."); @@ -425,22 +428,28 @@ - (void)setBackgroundColor:(NSColor*)color { [_flutterView setBackgroundColor:_backgroundColor]; } +- (uint64_t)id { + NSAssert([self attached], @"This view controller is not attched."); + return _id; +} + - (void)onPreEngineRestart { [self initializeKeyboard]; } - (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId { - NSAssert(_engine == nil && _id == 0, @"Already attached to an engine, engine %@ ID %llu.", - _engine, _id); + NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine); _engine = engine; _id = viewId; } - (void)detachFromEngine { - NSAssert(_engine != nil && _id != 0, @"Not attached to an engine, engine %@ ID %llu.", _engine, - _id); + NSAssert(_engine != nil, @"Not attached to any engine."); _engine = nil; - _id = 0; +} + +- (BOOL)attached { + return _engine != nil; } #pragma mark - Private methods diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm index 27d9721afc8e9..6a1aa66b69c59 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm @@ -29,13 +29,13 @@ viewProvider = [[FlutterViewEngineProvider alloc] initWithEngine:mockEngine]; // When the view controller is not set, the returned view is nil. - EXPECT_EQ([viewProvider getView:1], nil); + EXPECT_EQ([viewProvider getView:0], nil); // When the view controller is set, the returned view is the controller's view. mockFlutterViewController = OCMStrictClassMock([FlutterViewController class]); id mockView = OCMStrictClassMock([FlutterView class]); OCMStub([mockFlutterViewController flutterView]).andReturn(mockView); - EXPECT_EQ([viewProvider getView:1], mockView); + EXPECT_EQ([viewProvider getView:0], mockView); } } // namespace flutter::testing From 2c47d7158ad0a87465749954d67d495e8543a463 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 25 Jan 2023 12:52:33 -0800 Subject: [PATCH 11/12] better const --- shell/platform/darwin/macos/framework/Source/FlutterView.h | 2 +- shell/platform/darwin/macos/framework/Source/FlutterView.mm | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index ba70833b46b76..d67441aa804d1 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -18,7 +18,7 @@ * 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. */ -constexpr uint64_t kFlutterDefaultViewId = 0; +extern const uint64_t kFlutterDefaultViewId; /** * Listener for view resizing. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/shell/platform/darwin/macos/framework/Source/FlutterView.mm index c3f802d43ed9e..989497a630b91 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -9,6 +9,8 @@ #import +const uint64_t kFlutterDefaultViewId = 0; + @interface FlutterView () { __weak id _reshapeListener; FlutterThreadSynchronizer* _threadSynchronizer; From 4c3ecb282d10c6491cbfbf7b1c0ca8da22f01ffd Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 25 Jan 2023 13:02:52 -0800 Subject: [PATCH 12/12] Move const to engine --- .../darwin/macos/framework/Headers/FlutterEngine.h | 13 +++++++++++++ .../macos/framework/Source/FlutterCompositor.h | 1 - .../darwin/macos/framework/Source/FlutterEngine.mm | 2 ++ .../darwin/macos/framework/Source/FlutterView.h | 11 ----------- .../darwin/macos/framework/Source/FlutterView.mm | 2 -- .../macos/framework/Source/FlutterViewProvider.h | 2 ++ 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h b/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h index 76d886c651673..891ae28928a1e 100644 --- a/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h +++ b/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h @@ -7,6 +7,8 @@ #import +#include + #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; /** diff --git a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h b/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h index 99983e8ddb9bf..ba20f4dadbbfa 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h +++ b/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" diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index b01fcfb9254e1..07a9836f414c7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/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. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index d67441aa804d1..73425737fbd3a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -9,17 +9,6 @@ #include -/** - * 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; - /** * Listener for view resizing. */ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/shell/platform/darwin/macos/framework/Source/FlutterView.mm index 989497a630b91..c3f802d43ed9e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -9,8 +9,6 @@ #import -const uint64_t kFlutterDefaultViewId = 0; - @interface FlutterView () { __weak id _reshapeListener; FlutterThreadSynchronizer* _threadSynchronizer; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h b/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h index 98aca04055d5b..f873c72266a95 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h @@ -4,6 +4,8 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" +extern const uint64_t kFlutterDefaultViewId; + /** * An interface to query FlutterView. *