From cfae8b68bd54f4c129c7aa6bff3cd0862d40cc4a Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Mon, 8 Apr 2024 22:36:47 -0700 Subject: [PATCH 1/6] Migrate FlutterCallbackCache and FlutterKeyboardManager to ARC --- shell/platform/darwin/ios/BUILD.gn | 13 +++++---- .../framework/Source/FlutterCallbackCache.mm | 12 +++------ .../Source/FlutterKeyboardManager.mm | 27 +++++-------------- 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/shell/platform/darwin/ios/BUILD.gn b/shell/platform/darwin/ios/BUILD.gn index daf4f5db585d5..39e17b1d84648 100644 --- a/shell/platform/darwin/ios/BUILD.gn +++ b/shell/platform/darwin/ios/BUILD.gn @@ -59,10 +59,14 @@ source_set("flutter_framework_source_arc") { public_configs = [ "//flutter:config" ] sources = [ + "framework/Source/FlutterCallbackCache.mm", + "framework/Source/FlutterCallbackCache_Internal.h", "framework/Source/FlutterEmbedderKeyResponder.h", "framework/Source/FlutterEmbedderKeyResponder.mm", "framework/Source/FlutterKeyPrimaryResponder.h", "framework/Source/FlutterKeySecondaryResponder.h", + "framework/Source/FlutterKeyboardManager.h", + "framework/Source/FlutterKeyboardManager.mm", "framework/Source/FlutterMetalLayer.h", "framework/Source/FlutterMetalLayer.mm", "framework/Source/FlutterRestorationPlugin.h", @@ -83,7 +87,10 @@ source_set("flutter_framework_source_arc") { "IOSurface.framework", ] - deps += [ "//flutter/shell/platform/embedder:embedder_as_internal_library" ] + deps += [ + "//flutter/lib/ui", + "//flutter/shell/platform/embedder:embedder_as_internal_library", + ] } source_set("flutter_framework_source") { @@ -98,8 +105,6 @@ source_set("flutter_framework_source") { # New files are highly encouraged to be in ARC. # To add new files in ARC, add them to the `flutter_framework_source_arc` target. "framework/Source/FlutterAppDelegate.mm", - "framework/Source/FlutterCallbackCache.mm", - "framework/Source/FlutterCallbackCache_Internal.h", "framework/Source/FlutterChannelKeyResponder.h", "framework/Source/FlutterChannelKeyResponder.mm", "framework/Source/FlutterDartProject.mm", @@ -110,8 +115,6 @@ source_set("flutter_framework_source") { "framework/Source/FlutterEngineGroup.mm", "framework/Source/FlutterEngine_Internal.h", "framework/Source/FlutterHeadlessDartRunner.mm", - "framework/Source/FlutterKeyboardManager.h", - "framework/Source/FlutterKeyboardManager.mm", "framework/Source/FlutterOverlayView.h", "framework/Source/FlutterOverlayView.mm", "framework/Source/FlutterPlatformPlugin.h", diff --git a/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache.mm b/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache.mm index 624bc7b4ce795..1d752ebcccb4e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache.mm @@ -7,15 +7,9 @@ #include "flutter/fml/logging.h" #include "flutter/lib/ui/plugins/callback_cache.h" -@implementation FlutterCallbackInformation - -- (void)dealloc { - [_callbackName release]; - [_callbackClassName release]; - [_callbackLibraryPath release]; - [super dealloc]; -} +FLUTTER_ASSERT_ARC +@implementation FlutterCallbackInformation @end @implementation FlutterCallbackCache @@ -25,7 +19,7 @@ + (FlutterCallbackInformation*)lookupCallbackInformation:(int64_t)handle { if (info == nullptr) { return nil; } - FlutterCallbackInformation* new_info = [[[FlutterCallbackInformation alloc] init] autorelease]; + FlutterCallbackInformation* new_info = [[FlutterCallbackInformation alloc] init]; new_info.callbackName = [NSString stringWithUTF8String:info->name.c_str()]; new_info.callbackClassName = [NSString stringWithUTF8String:info->class_name.c_str()]; new_info.callbackLibraryPath = [NSString stringWithUTF8String:info->library_path.c_str()]; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm index 524dc6f43f791..423be3c7a7835 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm @@ -3,8 +3,11 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h" + #include "flutter/fml/platform/darwin/message_loop_darwin.h" -#include "flutter/fml/platform/darwin/weak_nsobject.h" +#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" + +FLUTTER_ASSERT_ARC static constexpr CFTimeInterval kDistantFuture = 1.0e10; @@ -28,16 +31,13 @@ - (void)dispatchToSecondaryResponders:(nonnull FlutterUIPressProxy*)press @end -@implementation FlutterKeyboardManager { - std::unique_ptr> _weakFactory; -} +@implementation FlutterKeyboardManager - (nonnull instancetype)init { self = [super init]; if (self != nil) { _primaryResponders = [[NSMutableArray alloc] init]; _secondaryResponders = [[NSMutableArray alloc] init]; - _weakFactory = std::make_unique>(self); } return self; } @@ -51,21 +51,8 @@ - (void)addSecondaryResponder:(nonnull id)responde } - (void)dealloc { - // It will be destroyed and invalidate its weak pointers - // before any other members are destroyed. - _weakFactory.reset(); - [_primaryResponders removeAllObjects]; [_secondaryResponders removeAllObjects]; - [_primaryResponders release]; - [_secondaryResponders release]; - _primaryResponders = nil; - _secondaryResponders = nil; - [super dealloc]; -} - -- (fml::WeakNSObject)getWeakNSObject { - return _weakFactory->GetWeakNSObject(); } - (void)handlePress:(nonnull FlutterUIPressProxy*)press @@ -89,7 +76,7 @@ - (void)handlePress:(nonnull FlutterUIPressProxy*)press // encounter. NSAssert([_primaryResponders count] >= 0, @"At least one primary responder must be added."); - __block auto weakSelf = [self getWeakNSObject]; + __block __weak __typeof(self) weakSelf = self; __block NSUInteger unreplied = [self.primaryResponders count]; __block BOOL anyHandled = false; FlutterAsyncKeyCallback replyCallback = ^(BOOL handled) { @@ -98,7 +85,7 @@ - (void)handlePress:(nonnull FlutterUIPressProxy*)press anyHandled = anyHandled || handled; if (unreplied == 0) { if (!anyHandled && weakSelf) { - [weakSelf.get() dispatchToSecondaryResponders:press complete:completeCallback]; + [weakSelf dispatchToSecondaryResponders:press complete:completeCallback]; } else { completeCallback(true, press); } From bbb9620d7c39b7d07ced026d04ee006be7aabc22 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 10 Apr 2024 17:23:28 -0700 Subject: [PATCH 2/6] Rewrite keyboard manager tests to use expectations --- .../framework/Source/FlutterChannelsTest.m | 2 +- .../Source/FlutterKeyboardManagerTest.mm | 509 +++++++----------- 2 files changed, 199 insertions(+), 312 deletions(-) diff --git a/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m b/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m index 8ed96b7aa17cd..bb8af64ad460e 100644 --- a/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m +++ b/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m @@ -168,7 +168,7 @@ - (void)testResize { [binaryMessenger stopMocking]; } -- (bool)testSetWarnsOnOverflow { +- (void)testSetWarnsOnOverflow { NSString* channelName = @"flutter/test"; id binaryMessenger = OCMStrictProtocolMock(@protocol(FlutterBinaryMessenger)); id codec = OCMProtocolMock(@protocol(FlutterMethodCodec)); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm index 5568c38016db9..3fbe972b00116 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm @@ -6,9 +6,7 @@ #import #import #import -#include <_types/_uint32_t.h> -#include "flutter/fml/platform/darwin/message_loop_darwin.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterFakeKeyEvents.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h" @@ -17,364 +15,253 @@ FLUTTER_ASSERT_ARC; -namespace flutter { -class PointerDataPacket {}; -} // namespace flutter - using namespace flutter::testing; -/// Sometimes we have to use a custom mock to avoid retain cycles in ocmock. -@interface FlutterEnginePartialMock : FlutterEngine -@property(nonatomic, strong) FlutterBasicMessageChannel* lifecycleChannel; -@property(nonatomic, weak) FlutterViewController* viewController; -@property(nonatomic, assign) BOOL didCallNotifyLowMemory; -@end - -@interface FlutterEngine () -- (BOOL)createShell:(NSString*)entrypoint - libraryURI:(NSString*)libraryURI - initialRoute:(NSString*)initialRoute; -- (void)dispatchPointerDataPacket:(std::unique_ptr)packet; -@end - -@interface FlutterEngine (TestLowMemory) -- (void)notifyLowMemory; -@end - -extern NSNotificationName const FlutterViewControllerWillDealloc; - -/// A simple mock class for FlutterEngine. -/// -/// OCMockClass can't be used for FlutterEngine sometimes because OCMock retains arguments to -/// invocations and since the init for FlutterViewController calls a method on the -/// FlutterEngine it creates a retain cycle that stops us from testing behaviors related to -/// deleting FlutterViewControllers. -@interface MockEngine : NSObject -@end - -@interface FlutterKeyboardManagerUnittestsObjC : NSObject -- (bool)nextResponderShouldThrowOnPressesEnded; -- (bool)singlePrimaryResponder; -- (bool)doublePrimaryResponder; -- (bool)singleSecondaryResponder; -- (bool)emptyNextResponder; -@end - -namespace { - -typedef void (^KeyCallbackSetter)(FlutterUIPressProxy* press, FlutterAsyncKeyCallback callback) - API_AVAILABLE(ios(13.4)); -typedef BOOL (^BoolGetter)(); - -} // namespace - +// These tests were designed to run on iOS 13.4 or later. +API_AVAILABLE(ios(13.4)) @interface FlutterKeyboardManagerTest : XCTestCase -@property(nonatomic, strong) id mockEngine; -- (FlutterViewController*)mockOwnerWithPressesBeginOnlyNext API_AVAILABLE(ios(13.4)); @end @implementation FlutterKeyboardManagerTest -- (void)setUp { - // All of these tests were designed to run on iOS 13.4 or later. - if (@available(iOS 13.4, *)) { - } else { - XCTSkip(@"Required API not present for test."); - } - - [super setUp]; - self.mockEngine = OCMClassMock([FlutterEngine class]); -} - -- (void)tearDown { - // We stop mocking here to avoid retain cycles that stop - // FlutterViewControllers from deallocing. - [self.mockEngine stopMocking]; - self.mockEngine = nil; - [super tearDown]; -} - -- (id)checkKeyDownEvent:(UIKeyboardHIDUsage)keyCode API_AVAILABLE(ios(13.4)) { - return [OCMArg checkWithBlock:^BOOL(id value) { - if (![value isKindOfClass:[FlutterUIPressProxy class]]) { - return NO; - } - FlutterUIPressProxy* press = value; - return press.key.keyCode == keyCode; - }]; -} - -- (id)mockPrimaryResponder:(KeyCallbackSetter)callbackSetter - API_AVAILABLE(ios(13.4)) { - id mock = - OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - OCMStub([mock handlePress:[OCMArg any] callback:[OCMArg any]]) - .andDo((^(NSInvocation* invocation) { - FlutterUIPressProxy* press; - FlutterAsyncKeyCallback callback; - [invocation getArgument:&press atIndex:2]; - [invocation getArgument:&callback atIndex:3]; - CFRunLoopPerformBlock(CFRunLoopGetCurrent(), - fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode, ^() { - callbackSetter(press, callback); - }); - })); - return mock; -} - -- (id)mockSecondaryResponder:(BoolGetter)resultGetter - API_AVAILABLE(ios(13.4)) { - id mock = - OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); - OCMStub([mock handlePress:[OCMArg any]]).andDo((^(NSInvocation* invocation) { - BOOL result = resultGetter(); - [invocation setReturnValue:&result]; - })); - return mock; -} - -- (FlutterViewController*)mockOwnerWithPressesBeginOnlyNext API_AVAILABLE(ios(13.4)) { +- (void)testNextResponderShouldThrowOnPressesEnded { // The nextResponder is a strict mock and hasn't stubbed pressesEnded. // An error will be thrown on pressesEnded. UIResponder* nextResponder = OCMStrictClassMock([UIResponder class]); - OCMStub([nextResponder pressesBegan:[OCMArg any] withEvent:[OCMArg any]]).andDo(nil); + OCMStub([nextResponder pressesBegan:OCMOCK_ANY withEvent:OCMOCK_ANY]); - FlutterViewController* viewController = - [[FlutterViewController alloc] initWithEngine:self.mockEngine nibName:nil bundle:nil]; + id mockEngine = OCMClassMock([FlutterEngine class]); + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine + nibName:nil + bundle:nil]; FlutterViewController* owner = OCMPartialMock(viewController); OCMStub([owner nextResponder]).andReturn(nextResponder); - return owner; -} -// Verify that the nextResponder returned from mockOwnerWithPressesBeginOnlyNext() -// throws exception when pressesEnded is called. -- (bool)testNextResponderShouldThrowOnPressesEnded API_AVAILABLE(ios(13.4)) { - FlutterViewController* owner = [self mockOwnerWithPressesBeginOnlyNext]; - @try { - [owner.nextResponder pressesEnded:[NSSet init] withEvent:[[UIPressesEvent alloc] init]]; - return false; - } @catch (...) { - return true; - } + XCTAssertThrowsSpecificNamed([owner.nextResponder pressesEnded:[[NSSet alloc] init] + withEvent:[[UIPressesEvent alloc] init]], + NSException, NSInternalInconsistencyException); + + [mockEngine stopMocking]; } -- (void)testSinglePrimaryResponder API_AVAILABLE(ios(13.4)) { +- (void)testSinglePrimaryResponder { + const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - __block BOOL primaryResponse = FALSE; - __block int callbackCount = 0; - [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, - FlutterAsyncKeyCallback callback) { - callbackCount++; - callback(primaryResponse); - }]]; - constexpr UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - // Case: The responder reports TRUE - __block bool completeHandled = true; - primaryResponse = TRUE; + id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + [manager addPrimaryResponder:mockPrimaryResponder]; + + // Case: The responder reports that is has handled the event (callback returns YES), and the + // manager should NOT call next action. + [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; + + XCTestExpectation* expectationPrimaryHandled = + [self expectationWithDescription:@"primary responder handled"]; + expectationPrimaryHandled.inverted = YES; // Fail if the manager "next action" is called. [manager handlePress:keyDownEvent(keyId) nextAction:^() { - completeHandled = false; + [expectationPrimaryHandled fulfill]; + XCTFail(); }]; - XCTAssertEqual(callbackCount, 1); - XCTAssertTrue(completeHandled); - completeHandled = true; - callbackCount = 0; - - // Case: The responder reports FALSE - primaryResponse = FALSE; + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + + // Case: The responder reports that is has NOT handled the event (callback returns NO), and the + // manager SHOULD not call next action. + [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; + id expectationPrimaryNotHandled = + [self expectationWithDescription:@"primary responder not handled"]; [manager handlePress:keyUpEvent(keyId) nextAction:^() { - completeHandled = false; + [expectationPrimaryNotHandled fulfill]; }]; - XCTAssertEqual(callbackCount, 1); - XCTAssertFalse(completeHandled); + [self waitForExpectationsWithTimeout:1.0 handler:nil]; } -- (void)testDoublePrimaryResponder API_AVAILABLE(ios(13.4)) { - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; +- (void)testDoublePrimaryResponder { + const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - __block BOOL callback1Response = FALSE; - __block int callback1Count = 0; - [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, - FlutterAsyncKeyCallback callback) { - callback1Count++; - callback(callback1Response); - }]]; - - __block BOOL callback2Response = FALSE; - __block int callback2Count = 0; - [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, - FlutterAsyncKeyCallback callback) { - callback2Count++; - callback(callback2Response); - }]]; - - // Case: Both responders report TRUE. - __block bool somethingWasHandled = true; - constexpr UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - callback1Response = TRUE; - callback2Response = TRUE; - [manager handlePress:keyUpEvent(keyId) + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; + id mockPrimaryResponder1 = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + id mockPrimaryResponder2 = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + [manager addPrimaryResponder:mockPrimaryResponder1]; + [manager addPrimaryResponder:mockPrimaryResponder2]; + + // Case: Both responders report they have handled the event (callbacks returns YES), and the + // manager should NOT call next action. + [[mockPrimaryResponder1 expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; + [[mockPrimaryResponder2 expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; + + XCTestExpectation* expectationBothPrimariesHandled = + [self expectationWithDescription:@"both primary responders handled"]; + expectationBothPrimariesHandled.inverted = YES; // Fail if the manager "next action" is called. + [manager handlePress:keyDownEvent(keyId) nextAction:^() { - somethingWasHandled = false; + [expectationBothPrimariesHandled fulfill]; + XCTFail(); }]; - XCTAssertEqual(callback1Count, 1); - XCTAssertEqual(callback2Count, 1); - XCTAssertTrue(somethingWasHandled); - - somethingWasHandled = true; - callback1Count = 0; - callback2Count = 0; - - // Case: One responder reports TRUE. - callback1Response = TRUE; - callback2Response = FALSE; + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + OCMVerifyAll(mockPrimaryResponder1); + OCMVerifyAll(mockPrimaryResponder2); + + // Case: Only one responder reports it has handled the event (callback returns YES), and the + // manager should NOT call next action. + [[mockPrimaryResponder1 expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; + [[mockPrimaryResponder2 expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; + + XCTestExpectation* expectationOnePrimaryHandled = + [self expectationWithDescription:@"one primary responder handled"]; + expectationOnePrimaryHandled.inverted = YES; // Fail if the manager "next action" is called. + [manager handlePress:keyDownEvent(keyId) + nextAction:^() { + [expectationOnePrimaryHandled fulfill]; + XCTFail(); + }]; + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + OCMVerifyAll(mockPrimaryResponder1); + OCMVerifyAll(mockPrimaryResponder2); + + // Case: Both responders report they have NOT handled the event (callbacks returns NO), and the + // manager SHOULD not call next action. + [[mockPrimaryResponder1 expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; + [[mockPrimaryResponder2 expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; + id expectationPrimariesNotHandled = + [self expectationWithDescription:@"primary responders not handled"]; [manager handlePress:keyUpEvent(keyId) nextAction:^() { - somethingWasHandled = false; + [expectationPrimariesNotHandled fulfill]; }]; - XCTAssertEqual(callback1Count, 1); - XCTAssertEqual(callback2Count, 1); - XCTAssertTrue(somethingWasHandled); + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + OCMVerifyAll(mockPrimaryResponder1); + OCMVerifyAll(mockPrimaryResponder2); +} - somethingWasHandled = true; - callback1Count = 0; - callback2Count = 0; +- (void)testPrimaryResponderHandlesNotSecondaryResponder { + const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - // Case: Both responders report FALSE. - callback1Response = FALSE; - callback2Response = FALSE; + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; + id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + id mockSecondaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); + [manager addPrimaryResponder:mockPrimaryResponder]; + [manager addSecondaryResponder:mockSecondaryResponder]; + + // Primary responder responds TRUE. The event shouldn't be handled by + // the secondary responder, and the manager should NOT call next action. + [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; + OCMReject([mockSecondaryResponder handlePress:OCMOCK_ANY]); + + XCTestExpectation* nextActionExpectation = [self expectationWithDescription:@"next action"]; + nextActionExpectation.inverted = YES; // Fail if the manager "next action" is called. [manager handlePress:keyDownEvent(keyId) nextAction:^() { - somethingWasHandled = false; + [nextActionExpectation fulfill]; + XCTFail(); }]; - XCTAssertEqual(callback1Count, 1); - XCTAssertEqual(callback2Count, 1); - XCTAssertFalse(somethingWasHandled); + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + OCMVerifyAll(mockPrimaryResponder); + OCMVerifyAll(mockSecondaryResponder); } -- (void)testSingleSecondaryResponder API_AVAILABLE(ios(13.4)) { - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; +- (void)testSecondaryResponderHandles { + const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - __block BOOL primaryResponse = FALSE; - __block int callbackCount = 0; - [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, - FlutterAsyncKeyCallback callback) { - callbackCount++; - callback(primaryResponse); - }]]; - - __block BOOL secondaryResponse; - [manager addSecondaryResponder:[self mockSecondaryResponder:^() { - return secondaryResponse; - }]]; - - // Case: Primary responder responds TRUE. The event shouldn't be handled by - // the secondary responder. - constexpr UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - secondaryResponse = FALSE; - primaryResponse = TRUE; - __block bool completeHandled = true; - [manager handlePress:keyUpEvent(keyId) - nextAction:^() { - completeHandled = false; - }]; - XCTAssertEqual(callbackCount, 1); - XCTAssertTrue(completeHandled); - completeHandled = true; - callbackCount = 0; - - // Case: Primary responder responds FALSE. The secondary responder returns - // TRUE. - secondaryResponse = TRUE; - primaryResponse = FALSE; - [manager handlePress:keyUpEvent(keyId) + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; + id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + id mockSecondaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); + [manager addPrimaryResponder:mockPrimaryResponder]; + [manager addSecondaryResponder:mockSecondaryResponder]; + + // Primary responder responds TRUE. The event shouldn't be handled by + // the secondary responder, and the manager should NOT call next action. + [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; + OCMExpect([mockSecondaryResponder handlePress:OCMOCK_ANY]).andReturn(YES); + + XCTestExpectation* nextActionExpectation = [self expectationWithDescription:@"next action"]; + nextActionExpectation.inverted = YES; // Fail if the manager "next action" is called. + [manager handlePress:keyDownEvent(keyId) nextAction:^() { - completeHandled = false; + [nextActionExpectation fulfill]; + XCTFail(); }]; - XCTAssertEqual(callbackCount, 1); - XCTAssertTrue(completeHandled); - completeHandled = true; - callbackCount = 0; - - // Case: Primary responder responds FALSE. The secondary responder returns FALSE. - secondaryResponse = FALSE; - primaryResponse = FALSE; + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + OCMVerifyAll(mockPrimaryResponder); + OCMVerifyAll(mockSecondaryResponder); +} + +- (void)testPrimaryAndSecondaryResponderDoNotHandle { + const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; + id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + id mockSecondaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); + [manager addPrimaryResponder:mockPrimaryResponder]; + [manager addSecondaryResponder:mockSecondaryResponder]; + + // Primary responder responds TRUE. The event shouldn't be handled by + // the secondary responder, and the manager should NOT call next action. + [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY + callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; + OCMExpect([mockSecondaryResponder handlePress:OCMOCK_ANY]).andReturn(NO); + + XCTestExpectation* nextActionExpectation = [self expectationWithDescription:@"next action"]; [manager handlePress:keyDownEvent(keyId) nextAction:^() { - completeHandled = false; + [nextActionExpectation fulfill]; }]; - XCTAssertEqual(callbackCount, 1); - XCTAssertFalse(completeHandled); + [self waitForExpectationsWithTimeout:1.0 handler:nil]; + OCMVerifyAll(mockPrimaryResponder); + OCMVerifyAll(mockSecondaryResponder); } -- (void)testEventsProcessedSequentially API_AVAILABLE(ios(13.4)) { - constexpr UIKeyboardHIDUsage keyId1 = (UIKeyboardHIDUsage)0x50; - constexpr UIKeyboardHIDUsage keyId2 = (UIKeyboardHIDUsage)0x51; +- (void)testEventsProcessedSequentially { + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; + id mockPrimaryResponder = + OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + [manager addPrimaryResponder:mockPrimaryResponder]; + + const UIKeyboardHIDUsage keyId1 = (UIKeyboardHIDUsage)0x50; FlutterUIPressProxy* event1 = keyDownEvent(keyId1); + id expectationEvent1Primary = [self expectationWithDescription:@"event1 primary responder"]; + OCMExpect([mockPrimaryResponder handlePress:event1 callback:OCMOCK_ANY]) + .andDo((^(NSInvocation* invocation) { + [expectationEvent1Primary fulfill]; + })); + + const UIKeyboardHIDUsage keyId2 = (UIKeyboardHIDUsage)0x51; FlutterUIPressProxy* event2 = keyDownEvent(keyId2); - __block FlutterAsyncKeyCallback key1Callback; - __block FlutterAsyncKeyCallback key2Callback; - __block bool key1Handled = true; - __block bool key2Handled = true; + id expectationEvent2Primary = [self expectationWithDescription:@"event2 primary responder"]; + OCMExpect([mockPrimaryResponder handlePress:event2 callback:OCMOCK_ANY]) + .andDo((^(NSInvocation* invocation) { + [expectationEvent2Primary fulfill]; + })); - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, - FlutterAsyncKeyCallback callback) { - if (press == event1) { - key1Callback = callback; - } else if (press == event2) { - key2Callback = callback; - } - }]]; - - // Add both presses into the main CFRunLoop queue - CFRunLoopTimerRef timer0 = CFRunLoopTimerCreateWithHandler( - kCFAllocatorDefault, CFAbsoluteTimeGetCurrent(), 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { - [manager handlePress:event1 - nextAction:^() { - key1Handled = false; - }]; - }); - CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer0, kCFRunLoopCommonModes); - CFRunLoopTimerRef timer1 = CFRunLoopTimerCreateWithHandler( - kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + 1, 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { - // key1 should be completely finished by now - XCTAssertFalse(key1Handled); - [manager handlePress:event2 - nextAction:^() { - key2Handled = false; - }]; - // End the nested CFRunLoop - CFRunLoopStop(CFRunLoopGetCurrent()); - }); - CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer1, kCFRunLoopCommonModes); - - // Add the callbacks to the CFRunLoop with mode kMessageLoopCFRunLoopMode - // This allows them to interrupt the loop started within handlePress - CFRunLoopTimerRef timer2 = CFRunLoopTimerCreateWithHandler( - kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + 2, 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { - // No processing should be done on key2 yet - XCTAssertTrue(key1Callback != nil); - XCTAssertTrue(key2Callback == nil); - key1Callback(false); - }); - CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer2, - fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode); - CFRunLoopTimerRef timer3 = CFRunLoopTimerCreateWithHandler( - kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + 3, 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { - // Both keys should be processed by now - XCTAssertTrue(key1Callback != nil); - XCTAssertTrue(key2Callback != nil); - key2Callback(false); - }); - CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer3, - fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode); - - // Start a nested CFRunLoop so we can wait for both presses to complete before exiting the test - CFRunLoopRun(); - XCTAssertFalse(key2Handled); - XCTAssertFalse(key1Handled); + id expectationEvent1Action = [self expectationWithDescription:@"event1 action"]; + [manager handlePress:event1 + nextAction:^() { + [expectationEvent1Action fulfill]; + }]; + + id expectationEvent2Action = [self expectationWithDescription:@"event2 action"]; + [manager handlePress:event2 + nextAction:^() { + [expectationEvent2Action fulfill]; + }]; + + [self waitForExpectations:@[ + expectationEvent1Primary, expectationEvent1Action, expectationEvent2Primary, + expectationEvent2Action + ] + timeout:1.0 + enforceOrder:YES]; } @end From 41c6cfeabb34fef86a905e1944ab4ad29b0c95dd Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 10 Apr 2024 17:47:39 -0700 Subject: [PATCH 3/6] verify --- .../darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm index 3fbe972b00116..a54553144d3a7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm @@ -262,6 +262,7 @@ - (void)testEventsProcessedSequentially { ] timeout:1.0 enforceOrder:YES]; + OCMVerifyAll(mockPrimaryResponder); } @end From e431d1116e16b2d9dad47c9ad2ef689852144c7e Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 10 Apr 2024 20:13:14 -0700 Subject: [PATCH 4/6] Revert the runloop parts of the keyboard manager tests --- .../Source/FlutterKeyboardManagerTest.mm | 405 ++++++++++-------- 1 file changed, 222 insertions(+), 183 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm index a54553144d3a7..0cf1f07026aae 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm @@ -6,7 +6,9 @@ #import #import #import +#include <_types/_uint32_t.h> +#include "flutter/fml/platform/darwin/message_loop_darwin.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterFakeKeyEvents.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.h" @@ -15,8 +17,20 @@ FLUTTER_ASSERT_ARC; +namespace flutter { +class PointerDataPacket {}; +} // namespace flutter + using namespace flutter::testing; +namespace { + +typedef void (^KeyCallbackSetter)(FlutterUIPressProxy* press, FlutterAsyncKeyCallback callback) + API_AVAILABLE(ios(13.4)); +typedef BOOL (^BoolGetter)(); + +} // namespace + // These tests were designed to run on iOS 13.4 or later. API_AVAILABLE(ios(13.4)) @interface FlutterKeyboardManagerTest : XCTestCase @@ -24,6 +38,33 @@ @interface FlutterKeyboardManagerTest : XCTestCase @implementation FlutterKeyboardManagerTest +- (id)mockPrimaryResponder:(KeyCallbackSetter)callbackSetter { + id mock = + OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); + OCMStub([mock handlePress:[OCMArg any] callback:[OCMArg any]]) + .andDo((^(NSInvocation* invocation) { + __unsafe_unretained FlutterUIPressProxy* press; + __unsafe_unretained FlutterAsyncKeyCallback callback; + [invocation getArgument:&press atIndex:2]; + [invocation getArgument:&callback atIndex:3]; + CFRunLoopPerformBlock(CFRunLoopGetCurrent(), + fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode, ^() { + callbackSetter(press, callback); + }); + })); + return mock; +} + +- (id)mockSecondaryResponder:(BoolGetter)resultGetter { + id mock = + OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); + OCMStub([mock handlePress:[OCMArg any]]).andDo((^(NSInvocation* invocation) { + BOOL result = resultGetter(); + [invocation setReturnValue:&result]; + })); + return mock; +} + - (void)testNextResponderShouldThrowOnPressesEnded { // The nextResponder is a strict mock and hasn't stubbed pressesEnded. // An error will be thrown on pressesEnded. @@ -45,224 +86,222 @@ - (void)testNextResponderShouldThrowOnPressesEnded { } - (void)testSinglePrimaryResponder { - const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - [manager addPrimaryResponder:mockPrimaryResponder]; - - // Case: The responder reports that is has handled the event (callback returns YES), and the - // manager should NOT call next action. - [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; - - XCTestExpectation* expectationPrimaryHandled = - [self expectationWithDescription:@"primary responder handled"]; - expectationPrimaryHandled.inverted = YES; // Fail if the manager "next action" is called. + __block BOOL primaryResponse = FALSE; + __block int callbackCount = 0; + [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, + FlutterAsyncKeyCallback callback) { + callbackCount++; + callback(primaryResponse); + }]]; + constexpr UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + // Case: The responder reports TRUE + __block bool completeHandled = true; + primaryResponse = TRUE; [manager handlePress:keyDownEvent(keyId) nextAction:^() { - [expectationPrimaryHandled fulfill]; - XCTFail(); + completeHandled = false; }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - - // Case: The responder reports that is has NOT handled the event (callback returns NO), and the - // manager SHOULD not call next action. - [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; - id expectationPrimaryNotHandled = - [self expectationWithDescription:@"primary responder not handled"]; + XCTAssertEqual(callbackCount, 1); + XCTAssertTrue(completeHandled); + completeHandled = true; + callbackCount = 0; + + // Case: The responder reports FALSE + primaryResponse = FALSE; [manager handlePress:keyUpEvent(keyId) nextAction:^() { - [expectationPrimaryNotHandled fulfill]; + completeHandled = false; }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; + XCTAssertEqual(callbackCount, 1); + XCTAssertFalse(completeHandled); } - (void)testDoublePrimaryResponder { - const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - id mockPrimaryResponder1 = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - id mockPrimaryResponder2 = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - [manager addPrimaryResponder:mockPrimaryResponder1]; - [manager addPrimaryResponder:mockPrimaryResponder2]; - - // Case: Both responders report they have handled the event (callbacks returns YES), and the - // manager should NOT call next action. - [[mockPrimaryResponder1 expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; - [[mockPrimaryResponder2 expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; - - XCTestExpectation* expectationBothPrimariesHandled = - [self expectationWithDescription:@"both primary responders handled"]; - expectationBothPrimariesHandled.inverted = YES; // Fail if the manager "next action" is called. - [manager handlePress:keyDownEvent(keyId) - nextAction:^() { - [expectationBothPrimariesHandled fulfill]; - XCTFail(); - }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - OCMVerifyAll(mockPrimaryResponder1); - OCMVerifyAll(mockPrimaryResponder2); - - // Case: Only one responder reports it has handled the event (callback returns YES), and the - // manager should NOT call next action. - [[mockPrimaryResponder1 expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; - [[mockPrimaryResponder2 expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; - - XCTestExpectation* expectationOnePrimaryHandled = - [self expectationWithDescription:@"one primary responder handled"]; - expectationOnePrimaryHandled.inverted = YES; // Fail if the manager "next action" is called. - [manager handlePress:keyDownEvent(keyId) - nextAction:^() { - [expectationOnePrimaryHandled fulfill]; - XCTFail(); - }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - OCMVerifyAll(mockPrimaryResponder1); - OCMVerifyAll(mockPrimaryResponder2); - - // Case: Both responders report they have NOT handled the event (callbacks returns NO), and the - // manager SHOULD not call next action. - [[mockPrimaryResponder1 expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; - [[mockPrimaryResponder2 expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; - id expectationPrimariesNotHandled = - [self expectationWithDescription:@"primary responders not handled"]; + + __block BOOL callback1Response = FALSE; + __block int callback1Count = 0; + [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, + FlutterAsyncKeyCallback callback) { + callback1Count++; + callback(callback1Response); + }]]; + + __block BOOL callback2Response = FALSE; + __block int callback2Count = 0; + [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, + FlutterAsyncKeyCallback callback) { + callback2Count++; + callback(callback2Response); + }]]; + + // Case: Both responders report TRUE. + __block bool somethingWasHandled = true; + constexpr UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + callback1Response = TRUE; + callback2Response = TRUE; [manager handlePress:keyUpEvent(keyId) nextAction:^() { - [expectationPrimariesNotHandled fulfill]; + somethingWasHandled = false; }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - OCMVerifyAll(mockPrimaryResponder1); - OCMVerifyAll(mockPrimaryResponder2); -} + XCTAssertEqual(callback1Count, 1); + XCTAssertEqual(callback2Count, 1); + XCTAssertTrue(somethingWasHandled); -- (void)testPrimaryResponderHandlesNotSecondaryResponder { - const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + somethingWasHandled = true; + callback1Count = 0; + callback2Count = 0; - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - id mockSecondaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); - [manager addPrimaryResponder:mockPrimaryResponder]; - [manager addSecondaryResponder:mockSecondaryResponder]; - - // Primary responder responds TRUE. The event shouldn't be handled by - // the secondary responder, and the manager should NOT call next action. - [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@YES, nil]]; - OCMReject([mockSecondaryResponder handlePress:OCMOCK_ANY]); - - XCTestExpectation* nextActionExpectation = [self expectationWithDescription:@"next action"]; - nextActionExpectation.inverted = YES; // Fail if the manager "next action" is called. - [manager handlePress:keyDownEvent(keyId) + // Case: One responder reports TRUE. + callback1Response = TRUE; + callback2Response = FALSE; + [manager handlePress:keyUpEvent(keyId) nextAction:^() { - [nextActionExpectation fulfill]; - XCTFail(); + somethingWasHandled = false; }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - OCMVerifyAll(mockPrimaryResponder); - OCMVerifyAll(mockSecondaryResponder); -} + XCTAssertEqual(callback1Count, 1); + XCTAssertEqual(callback2Count, 1); + XCTAssertTrue(somethingWasHandled); -- (void)testSecondaryResponderHandles { - const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + somethingWasHandled = true; + callback1Count = 0; + callback2Count = 0; - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - id mockSecondaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); - [manager addPrimaryResponder:mockPrimaryResponder]; - [manager addSecondaryResponder:mockSecondaryResponder]; - - // Primary responder responds TRUE. The event shouldn't be handled by - // the secondary responder, and the manager should NOT call next action. - [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; - OCMExpect([mockSecondaryResponder handlePress:OCMOCK_ANY]).andReturn(YES); - - XCTestExpectation* nextActionExpectation = [self expectationWithDescription:@"next action"]; - nextActionExpectation.inverted = YES; // Fail if the manager "next action" is called. + // Case: Both responders report FALSE. + callback1Response = FALSE; + callback2Response = FALSE; [manager handlePress:keyDownEvent(keyId) nextAction:^() { - [nextActionExpectation fulfill]; - XCTFail(); + somethingWasHandled = false; }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - OCMVerifyAll(mockPrimaryResponder); - OCMVerifyAll(mockSecondaryResponder); + XCTAssertEqual(callback1Count, 1); + XCTAssertEqual(callback2Count, 1); + XCTAssertFalse(somethingWasHandled); } -- (void)testPrimaryAndSecondaryResponderDoNotHandle { - const UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; - +- (void)testSingleSecondaryResponder { FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - id mockPrimaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - id mockSecondaryResponder = OCMStrictProtocolMock(@protocol(FlutterKeySecondaryResponder)); - [manager addPrimaryResponder:mockPrimaryResponder]; - [manager addSecondaryResponder:mockSecondaryResponder]; - - // Primary responder responds TRUE. The event shouldn't be handled by - // the secondary responder, and the manager should NOT call next action. - [[mockPrimaryResponder expect] handlePress:OCMOCK_ANY - callback:[OCMArg invokeBlockWithArgs:@NO, nil]]; - OCMExpect([mockSecondaryResponder handlePress:OCMOCK_ANY]).andReturn(NO); - - XCTestExpectation* nextActionExpectation = [self expectationWithDescription:@"next action"]; + + __block BOOL primaryResponse = FALSE; + __block int callbackCount = 0; + [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, + FlutterAsyncKeyCallback callback) { + callbackCount++; + callback(primaryResponse); + }]]; + + __block BOOL secondaryResponse; + [manager addSecondaryResponder:[self mockSecondaryResponder:^() { + return secondaryResponse; + }]]; + + // Case: Primary responder responds TRUE. The event shouldn't be handled by + // the secondary responder. + constexpr UIKeyboardHIDUsage keyId = (UIKeyboardHIDUsage)0x50; + secondaryResponse = FALSE; + primaryResponse = TRUE; + __block bool completeHandled = true; + [manager handlePress:keyUpEvent(keyId) + nextAction:^() { + completeHandled = false; + }]; + XCTAssertEqual(callbackCount, 1); + XCTAssertTrue(completeHandled); + completeHandled = true; + callbackCount = 0; + + // Case: Primary responder responds FALSE. The secondary responder returns + // TRUE. + secondaryResponse = TRUE; + primaryResponse = FALSE; + [manager handlePress:keyUpEvent(keyId) + nextAction:^() { + completeHandled = false; + }]; + XCTAssertEqual(callbackCount, 1); + XCTAssertTrue(completeHandled); + completeHandled = true; + callbackCount = 0; + + // Case: Primary responder responds FALSE. The secondary responder returns FALSE. + secondaryResponse = FALSE; + primaryResponse = FALSE; [manager handlePress:keyDownEvent(keyId) nextAction:^() { - [nextActionExpectation fulfill]; + completeHandled = false; }]; - [self waitForExpectationsWithTimeout:1.0 handler:nil]; - OCMVerifyAll(mockPrimaryResponder); - OCMVerifyAll(mockSecondaryResponder); + XCTAssertEqual(callbackCount, 1); + XCTAssertFalse(completeHandled); } - (void)testEventsProcessedSequentially { - FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; - id mockPrimaryResponder = - OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); - [manager addPrimaryResponder:mockPrimaryResponder]; - - const UIKeyboardHIDUsage keyId1 = (UIKeyboardHIDUsage)0x50; + constexpr UIKeyboardHIDUsage keyId1 = (UIKeyboardHIDUsage)0x50; + constexpr UIKeyboardHIDUsage keyId2 = (UIKeyboardHIDUsage)0x51; FlutterUIPressProxy* event1 = keyDownEvent(keyId1); - id expectationEvent1Primary = [self expectationWithDescription:@"event1 primary responder"]; - OCMExpect([mockPrimaryResponder handlePress:event1 callback:OCMOCK_ANY]) - .andDo((^(NSInvocation* invocation) { - [expectationEvent1Primary fulfill]; - })); - - const UIKeyboardHIDUsage keyId2 = (UIKeyboardHIDUsage)0x51; FlutterUIPressProxy* event2 = keyDownEvent(keyId2); - id expectationEvent2Primary = [self expectationWithDescription:@"event2 primary responder"]; - OCMExpect([mockPrimaryResponder handlePress:event2 callback:OCMOCK_ANY]) - .andDo((^(NSInvocation* invocation) { - [expectationEvent2Primary fulfill]; - })); + __block FlutterAsyncKeyCallback key1Callback; + __block FlutterAsyncKeyCallback key2Callback; + __block bool key1Handled = true; + __block bool key2Handled = true; - id expectationEvent1Action = [self expectationWithDescription:@"event1 action"]; - [manager handlePress:event1 - nextAction:^() { - [expectationEvent1Action fulfill]; - }]; - - id expectationEvent2Action = [self expectationWithDescription:@"event2 action"]; - [manager handlePress:event2 - nextAction:^() { - [expectationEvent2Action fulfill]; - }]; - - [self waitForExpectations:@[ - expectationEvent1Primary, expectationEvent1Action, expectationEvent2Primary, - expectationEvent2Action - ] - timeout:1.0 - enforceOrder:YES]; - OCMVerifyAll(mockPrimaryResponder); + FlutterKeyboardManager* manager = [[FlutterKeyboardManager alloc] init]; + [manager addPrimaryResponder:[self mockPrimaryResponder:^(FlutterUIPressProxy* press, + FlutterAsyncKeyCallback callback) { + if (press == event1) { + key1Callback = callback; + } else if (press == event2) { + key2Callback = callback; + } + }]]; + + // Add both presses into the main CFRunLoop queue + CFRunLoopTimerRef timer0 = CFRunLoopTimerCreateWithHandler( + kCFAllocatorDefault, CFAbsoluteTimeGetCurrent(), 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { + [manager handlePress:event1 + nextAction:^() { + key1Handled = false; + }]; + }); + CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer0, kCFRunLoopCommonModes); + CFRunLoopTimerRef timer1 = CFRunLoopTimerCreateWithHandler( + kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + 1, 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { + // key1 should be completely finished by now + XCTAssertFalse(key1Handled); + [manager handlePress:event2 + nextAction:^() { + key2Handled = false; + }]; + // End the nested CFRunLoop + CFRunLoopStop(CFRunLoopGetCurrent()); + }); + CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer1, kCFRunLoopCommonModes); + + // Add the callbacks to the CFRunLoop with mode kMessageLoopCFRunLoopMode + // This allows them to interrupt the loop started within handlePress + CFRunLoopTimerRef timer2 = CFRunLoopTimerCreateWithHandler( + kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + 2, 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { + // No processing should be done on key2 yet + XCTAssertTrue(key1Callback != nil); + XCTAssertTrue(key2Callback == nil); + key1Callback(false); + }); + CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer2, + fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode); + CFRunLoopTimerRef timer3 = CFRunLoopTimerCreateWithHandler( + kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + 3, 0, 0, 0, ^(CFRunLoopTimerRef timerRef) { + // Both keys should be processed by now + XCTAssertTrue(key1Callback != nil); + XCTAssertTrue(key2Callback != nil); + key2Callback(false); + }); + CFRunLoopAddTimer(CFRunLoopGetCurrent(), timer3, + fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode); + + // Start a nested CFRunLoop so we can wait for both presses to complete before exiting the test + CFRunLoopRun(); + XCTAssertFalse(key2Handled); + XCTAssertFalse(key1Handled); } @end From 2f6a5914e004e2a2dd956d56b99b113b2e29fc27 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 10 Apr 2024 21:06:26 -0700 Subject: [PATCH 5/6] strong --- .../darwin/ios/framework/Source/FlutterKeyboardManager.mm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm index 423be3c7a7835..b2400e2f6d837 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm @@ -16,13 +16,12 @@ @interface FlutterKeyboardManager () /** * The primary responders added by addPrimaryResponder. */ -@property(nonatomic, retain, readonly) - NSMutableArray>* primaryResponders; +@property(nonatomic, readonly) NSMutableArray>* primaryResponders; /** * The secondary responders added by addSecondaryResponder. */ -@property(nonatomic, retain, readonly) +@property(nonatomic, readonly) NSMutableArray>* secondaryResponders; - (void)dispatchToSecondaryResponders:(nonnull FlutterUIPressProxy*)press From 25bfa115fd045b92e0c294fe809d58f90dc9c192 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Thu, 11 Apr 2024 21:31:16 -0700 Subject: [PATCH 6/6] __unsafe_unretained --- .../ios/framework/Source/FlutterKeyboardManager.mm | 10 +++------- .../framework/Source/FlutterKeyboardManagerTest.mm | 14 ++++++++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm index b2400e2f6d837..83d1e22a1aabe 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManager.mm @@ -16,12 +16,13 @@ @interface FlutterKeyboardManager () /** * The primary responders added by addPrimaryResponder. */ -@property(nonatomic, readonly) NSMutableArray>* primaryResponders; +@property(nonatomic, copy, readonly) + NSMutableArray>* primaryResponders; /** * The secondary responders added by addSecondaryResponder. */ -@property(nonatomic, readonly) +@property(nonatomic, copy, readonly) NSMutableArray>* secondaryResponders; - (void)dispatchToSecondaryResponders:(nonnull FlutterUIPressProxy*)press @@ -49,11 +50,6 @@ - (void)addSecondaryResponder:(nonnull id)responde [_secondaryResponders addObject:responder]; } -- (void)dealloc { - [_primaryResponders removeAllObjects]; - [_secondaryResponders removeAllObjects]; -} - - (void)handlePress:(nonnull FlutterUIPressProxy*)press nextAction:(nonnull void (^)())next API_AVAILABLE(ios(13.4)) { if (@available(iOS 13.4, *)) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm index 0cf1f07026aae..7ebd7c01a3ace 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterKeyboardManagerTest.mm @@ -43,10 +43,16 @@ @implementation FlutterKeyboardManagerTest OCMStrictProtocolMock(@protocol(FlutterKeyPrimaryResponder)); OCMStub([mock handlePress:[OCMArg any] callback:[OCMArg any]]) .andDo((^(NSInvocation* invocation) { - __unsafe_unretained FlutterUIPressProxy* press; - __unsafe_unretained FlutterAsyncKeyCallback callback; - [invocation getArgument:&press atIndex:2]; - [invocation getArgument:&callback atIndex:3]; + __unsafe_unretained FlutterUIPressProxy* pressUnsafe; + __unsafe_unretained FlutterAsyncKeyCallback callbackUnsafe; + + [invocation getArgument:&pressUnsafe atIndex:2]; + [invocation getArgument:&callbackUnsafe atIndex:3]; + + // Retain the unretained parameters so they can + // be run in the perform block when this invocation goes out of scope. + FlutterUIPressProxy* press = pressUnsafe; + FlutterAsyncKeyCallback callback = callbackUnsafe; CFRunLoopPerformBlock(CFRunLoopGetCurrent(), fml::MessageLoopDarwin::kMessageLoopCFRunLoopMode, ^() { callbackSetter(press, callback);