From bc964cfd5e66beb9e89ab5bb449d05eb83ad36e7 Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Wed, 28 Jul 2021 13:59:28 -0400 Subject: [PATCH 1/9] Remove iPadOS mouse pointer if no longer connected --- .../framework/Source/FlutterViewController.mm | 29 +++++++- .../ScenariosUITests/StatusBarTest.m | 6 +- .../ScenariosUITests/iPadGestureTests.m | 67 +++++++++++++++++-- .../lib/src/touches_scenario.dart | 14 +++- 4 files changed, 103 insertions(+), 13 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index ae68d3c089dab..30c8f5b30b880 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -891,10 +891,30 @@ - (void)dispatchTouches:(NSSet*)touches } const CGFloat scale = [UIScreen mainScreen].scale; - auto packet = std::make_unique(touches.count); + + bool will_drop_hover = NO; + + if (@available(iOS 13.4, *)) { + if ([UIFocusSystem focusSystemForEnvironment:self.view] == nil) { + // There is no focus environment, the pointer is no longer attached + will_drop_hover = YES; + } + } + + auto packet = std::make_unique(will_drop_hover ? touches.count + 1 + : touches.count); size_t pointer_index = 0; + if (@available(iOS 13.4, *)) { + if (will_drop_hover) { + _mouseState.flutter_state_is_added = false; + flutter::PointerData drop_hover_pointer_data = [self generatePointerDataForMouse]; + drop_hover_pointer_data.change = flutter::PointerData::Change::kRemove; + packet->SetPointerData(pointer_index++, drop_hover_pointer_data); + } + } + for (UITouch* touch in touches) { CGPoint windowCoordinates = [touch locationInView:self.view]; @@ -1682,7 +1702,8 @@ - (BOOL)isPresentingViewController { pointer_data.kind = flutter::PointerData::DeviceKind::kMouse; pointer_data.change = _mouseState.flutter_state_is_added ? flutter::PointerData::Change::kAdd : flutter::PointerData::Change::kHover; - pointer_data.pointer_identifier = reinterpret_cast(_pointerInteraction.get()); + pointer_data.time_stamp = [[NSProcessInfo processInfo] systemUptime] * kMicrosecondsPerSecond; + pointer_data.device = reinterpret_cast(_pointerInteraction.get()); pointer_data.physical_x = _mouseState.location.x; pointer_data.physical_y = _mouseState.location.y; @@ -1704,6 +1725,8 @@ - (UIPointerRegion*)pointerInteraction:(UIPointerInteraction*)interaction packet->SetPointerData(/*index=*/0, pointer_data); [_engine.get() dispatchPointerDataPacket:std::move(packet)]; + + _mouseState.flutter_state_is_added = true; } return nil; } @@ -1735,6 +1758,8 @@ - (void)scrollEvent:(UIPanGestureRecognizer*)recognizer API_AVAILABLE(ios(13.4)) packet->SetPointerData(/*index=*/0, pointer_data); [_engine.get() dispatchPointerDataPacket:std::move(packet)]; + + _mouseState.flutter_state_is_added = true; } #pragma mark - State Restoration diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m index 6f9ce64058653..f357b34b5fa3b 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m @@ -30,13 +30,13 @@ - (void)testTapStatusBar { [[self.application.statusBars firstMatch] tap]; } - XCUIElement* addTextField = self.application.textFields[@"PointerChange.add:0"]; + XCUIElement* addTextField = self.application.textFields[@"0,PointerChange.add,d=0,b=0"]; BOOL exists = [addTextField waitForExistenceWithTimeout:1]; XCTAssertTrue(exists, @""); - XCUIElement* downTextField = self.application.textFields[@"PointerChange.down:0"]; + XCUIElement* downTextField = self.application.textFields[@"1,PointerChange.down,d=0,b=0"]; exists = [downTextField waitForExistenceWithTimeout:1]; XCTAssertTrue(exists, @""); - XCUIElement* upTextField = self.application.textFields[@"PointerChange.up:0"]; + XCUIElement* upTextField = self.application.textFields[@"2,PointerChange.up,d=0,b=0"]; exists = [upTextField waitForExistenceWithTimeout:1]; XCTAssertTrue(exists, @""); } diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m index 57149ea741831..92029480c1f84 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m @@ -62,24 +62,77 @@ - (void)testPointerButtons { [flutterView tap]; // Initial add event should have buttons = 0 - XCTAssertTrue([app.textFields[@"PointerChange.add:0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur"); + XCTAssertTrue([app.textFields[@"0,PointerChange.add,d=0,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a normal tap"); // Normal tap should have buttons = 0, the flutter framework will ensure it has buttons = 1 - XCTAssertTrue([app.textFields[@"PointerChange.down:0"] waitForExistenceWithTimeout:1], + XCTAssertTrue([app.textFields[@"1,PointerChange.down,d=0,b=0"] waitForExistenceWithTimeout:1], @"PointerChange.down event did not occur for a normal tap"); - XCTAssertTrue([app.textFields[@"PointerChange.up:0"] waitForExistenceWithTimeout:1], + XCTAssertTrue([app.textFields[@"2,PointerChange.up,d=0,b=0"] waitForExistenceWithTimeout:1], @"PointerChange.up event did not occur for a normal tap"); SEL rightClick = @selector(rightClick); XCTAssertTrue([flutterView respondsToSelector:rightClick], @"If supportsPointerInteraction is true, this should be true too."); [flutterView performSelector:rightClick]; - // Since each touch is its own device, we can't distinguish the other add event(s) + // Right-clicking will trigger the hover pointer as well + XCTAssertTrue([app.textFields[@"3,PointerChange.add,d=1,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a right-click"); + XCTAssertTrue([app.textFields[@"4,PointerChange.add,d=2,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a right-click"); // Right click should have buttons = 2 - XCTAssertTrue([app.textFields[@"PointerChange.down:2"] waitForExistenceWithTimeout:1], + XCTAssertTrue([app.textFields[@"5,PointerChange.down,d=2,b=2"] waitForExistenceWithTimeout:1], @"PointerChange.down event did not occur for a right-click"); - XCTAssertTrue([app.textFields[@"PointerChange.up:2"] waitForExistenceWithTimeout:1], + XCTAssertTrue([app.textFields[@"6,PointerChange.up,d=2,b=2"] waitForExistenceWithTimeout:1], @"PointerChange.up event did not occur for a right-click"); } + +- (void)testPointerHover { + BOOL supportsPointerInteraction = NO; + SEL supportsPointerInteractionSelector = @selector(supportsPointerInteraction); + if ([XCUIDevice.sharedDevice respondsToSelector:supportsPointerInteractionSelector]) { + supportsPointerInteraction = + performBoolSelector(XCUIDevice.sharedDevice, supportsPointerInteractionSelector); + } + XCTSkipUnless(supportsPointerInteraction, "Device does not support pointer interaction."); + XCUIApplication* app = [[XCUIApplication alloc] init]; + app.launchArguments = @[ @"--pointer-events" ]; + [app launch]; + + NSPredicate* predicateToFindFlutterView = + [NSPredicate predicateWithBlock:^BOOL(id _Nullable evaluatedObject, + NSDictionary* _Nullable bindings) { + XCUIElement* element = evaluatedObject; + return [element.identifier hasPrefix:@"flutter_view"]; + }]; + XCUIElement* flutterView = [[app descendantsMatchingType:XCUIElementTypeAny] + elementMatchingPredicate:predicateToFindFlutterView]; + if (![flutterView waitForExistenceWithTimeout:kSecondsToWaitForFlutterView]) { + NSLog(@"%@", app.debugDescription); + XCTFail(@"Failed due to not able to find any flutterView with %@ seconds", + @(kSecondsToWaitForFlutterView)); + } + + XCTAssertNotNil(flutterView); + + SEL hover = @selector(hover); + XCTAssertTrue([flutterView respondsToSelector:hover], + @"If supportsPointerInteraction is true, this should be true too."); + [flutterView performSelector:hover]; + [NSThread sleepForTimeInterval:1.0]; + XCTAssertTrue([app.textFields[@"0,PointerChange.add,d=0,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a hover"); + [flutterView tap]; + XCTAssertTrue([app.textFields[@"1,PointerChange.add,d=1,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a tap"); + XCTAssertTrue([app.textFields[@"2,PointerChange.down,d=1,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.down event did not occur for a tap"); + XCTAssertTrue([app.textFields[@"3,PointerChange.up,d=1,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.up event did not occur for a tap"); + XCTAssertTrue([app.textFields[@"4,PointerChange.remove,d=0,b=0"] waitForExistenceWithTimeout:1], + @"The hover pointer was not removed after a tap"); + [flutterView performSelector:hover]; + XCTAssertTrue([app.textFields[@"5,PointerChange.add,d=0,b=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a subsequent hover"); +} #pragma clang diagnostic pop @end diff --git a/testing/scenario_app/lib/src/touches_scenario.dart b/testing/scenario_app/lib/src/touches_scenario.dart index c6764780d72f0..7b7e402c66bf8 100644 --- a/testing/scenario_app/lib/src/touches_scenario.dart +++ b/testing/scenario_app/lib/src/touches_scenario.dart @@ -9,6 +9,9 @@ import 'scenario.dart'; /// A scenario that sends back messages when touches are received. class TouchesScenario extends Scenario { + final Map _knownDevices = {}; + int _sequenceNo = 0; + /// Constructor for `TouchesScenario`. TouchesScenario(PlatformDispatcher dispatcher) : super(dispatcher); @@ -23,13 +26,22 @@ class TouchesScenario extends Scenario { @override void onPointerDataPacket(PointerDataPacket packet) { for (final PointerData datum in packet.data) { + final int deviceId = + _knownDevices.putIfAbsent(datum.device, () => _knownDevices.length); sendJsonMessage( dispatcher: dispatcher, channel: 'display_data', json: { - 'data': datum.change.toString() + ':' + datum.buttons.toString(), + 'data': _sequenceNo.toString() + + ',' + + datum.change.toString() + + ',d=' + + deviceId.toString() + + ',b=' + + datum.buttons.toString(), }, ); + _sequenceNo++; } } } From 29185ad3494b842ba1d2b18c6e02f66f7cb55cb5 Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Wed, 8 Sep 2021 08:08:57 -0400 Subject: [PATCH 2/9] Update touches_scenario data format --- .../ScenariosUITests/StatusBarTest.m | 8 ++- .../ScenariosUITests/iPadGestureTests.m | 65 +++++++++++-------- .../lib/src/touches_scenario.dart | 4 +- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m index f357b34b5fa3b..0422adcb8c124 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/StatusBarTest.m @@ -30,13 +30,15 @@ - (void)testTapStatusBar { [[self.application.statusBars firstMatch] tap]; } - XCUIElement* addTextField = self.application.textFields[@"0,PointerChange.add,d=0,b=0"]; + XCUIElement* addTextField = + self.application.textFields[@"0,PointerChange.add,device=0,buttons=0"]; BOOL exists = [addTextField waitForExistenceWithTimeout:1]; XCTAssertTrue(exists, @""); - XCUIElement* downTextField = self.application.textFields[@"1,PointerChange.down,d=0,b=0"]; + XCUIElement* downTextField = + self.application.textFields[@"1,PointerChange.down,device=0,buttons=0"]; exists = [downTextField waitForExistenceWithTimeout:1]; XCTAssertTrue(exists, @""); - XCUIElement* upTextField = self.application.textFields[@"2,PointerChange.up,d=0,b=0"]; + XCUIElement* upTextField = self.application.textFields[@"2,PointerChange.up,device=0,buttons=0"]; exists = [upTextField waitForExistenceWithTimeout:1]; XCTAssertTrue(exists, @""); } diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m index 92029480c1f84..0f061f3f33ba7 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m @@ -62,27 +62,34 @@ - (void)testPointerButtons { [flutterView tap]; // Initial add event should have buttons = 0 - XCTAssertTrue([app.textFields[@"0,PointerChange.add,d=0,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a normal tap"); + XCTAssertTrue( + [app.textFields[@"0,PointerChange.add,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a normal tap"); // Normal tap should have buttons = 0, the flutter framework will ensure it has buttons = 1 - XCTAssertTrue([app.textFields[@"1,PointerChange.down,d=0,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.down event did not occur for a normal tap"); - XCTAssertTrue([app.textFields[@"2,PointerChange.up,d=0,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.up event did not occur for a normal tap"); + XCTAssertTrue( + [app.textFields[@"1,PointerChange.down,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.down event did not occur for a normal tap"); + XCTAssertTrue( + [app.textFields[@"2,PointerChange.up,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.up event did not occur for a normal tap"); SEL rightClick = @selector(rightClick); XCTAssertTrue([flutterView respondsToSelector:rightClick], @"If supportsPointerInteraction is true, this should be true too."); [flutterView performSelector:rightClick]; // Right-clicking will trigger the hover pointer as well - XCTAssertTrue([app.textFields[@"3,PointerChange.add,d=1,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a right-click"); - XCTAssertTrue([app.textFields[@"4,PointerChange.add,d=2,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a right-click"); + XCTAssertTrue( + [app.textFields[@"3,PointerChange.add,device=1,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a right-click"); + XCTAssertTrue( + [app.textFields[@"4,PointerChange.add,device=2,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a right-click"); // Right click should have buttons = 2 - XCTAssertTrue([app.textFields[@"5,PointerChange.down,d=2,b=2"] waitForExistenceWithTimeout:1], - @"PointerChange.down event did not occur for a right-click"); - XCTAssertTrue([app.textFields[@"6,PointerChange.up,d=2,b=2"] waitForExistenceWithTimeout:1], - @"PointerChange.up event did not occur for a right-click"); + XCTAssertTrue( + [app.textFields[@"5,PointerChange.down,device=2,buttons=2"] waitForExistenceWithTimeout:1], + @"PointerChange.down event did not occur for a right-click"); + XCTAssertTrue( + [app.textFields[@"6,PointerChange.up,device=2,buttons=2"] waitForExistenceWithTimeout:1], + @"PointerChange.up event did not occur for a right-click"); } - (void)testPointerHover { @@ -118,20 +125,26 @@ - (void)testPointerHover { @"If supportsPointerInteraction is true, this should be true too."); [flutterView performSelector:hover]; [NSThread sleepForTimeInterval:1.0]; - XCTAssertTrue([app.textFields[@"0,PointerChange.add,d=0,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a hover"); + XCTAssertTrue( + [app.textFields[@"0,PointerChange.add,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a hover"); [flutterView tap]; - XCTAssertTrue([app.textFields[@"1,PointerChange.add,d=1,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a tap"); - XCTAssertTrue([app.textFields[@"2,PointerChange.down,d=1,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.down event did not occur for a tap"); - XCTAssertTrue([app.textFields[@"3,PointerChange.up,d=1,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.up event did not occur for a tap"); - XCTAssertTrue([app.textFields[@"4,PointerChange.remove,d=0,b=0"] waitForExistenceWithTimeout:1], - @"The hover pointer was not removed after a tap"); + XCTAssertTrue( + [app.textFields[@"1,PointerChange.add,device=1,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a tap"); + XCTAssertTrue( + [app.textFields[@"2,PointerChange.down,device=1,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.down event did not occur for a tap"); + XCTAssertTrue( + [app.textFields[@"3,PointerChange.up,device=1,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.up event did not occur for a tap"); + XCTAssertTrue( + [app.textFields[@"4,PointerChange.remove,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"The hover pointer was not removed after a tap"); [flutterView performSelector:hover]; - XCTAssertTrue([app.textFields[@"5,PointerChange.add,d=0,b=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a subsequent hover"); + XCTAssertTrue( + [app.textFields[@"5,PointerChange.add,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.add event did not occur for a subsequent hover"); } #pragma clang diagnostic pop diff --git a/testing/scenario_app/lib/src/touches_scenario.dart b/testing/scenario_app/lib/src/touches_scenario.dart index 7b7e402c66bf8..be0f9551308d4 100644 --- a/testing/scenario_app/lib/src/touches_scenario.dart +++ b/testing/scenario_app/lib/src/touches_scenario.dart @@ -35,9 +35,9 @@ class TouchesScenario extends Scenario { 'data': _sequenceNo.toString() + ',' + datum.change.toString() + - ',d=' + + ',device=' + deviceId.toString() + - ',b=' + + ',buttons=' + datum.buttons.toString(), }, ); From 3845191a57b94bb68b1670db8f2e9d2cf7b7c688 Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Wed, 8 Sep 2021 08:16:44 -0400 Subject: [PATCH 3/9] Fix hover pointer add/remove logic --- .../darwin/ios/framework/Source/FlutterViewController.mm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 30c8f5b30b880..6fbca8a459b48 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -895,7 +895,8 @@ - (void)dispatchTouches:(NSSet*)touches bool will_drop_hover = NO; if (@available(iOS 13.4, *)) { - if ([UIFocusSystem focusSystemForEnvironment:self.view] == nil) { + if (_mouseState.flutter_state_is_added && + [UIFocusSystem focusSystemForEnvironment:self.view] == nil) { // There is no focus environment, the pointer is no longer attached will_drop_hover = YES; } @@ -1700,8 +1701,8 @@ - (BOOL)isPresentingViewController { pointer_data.Clear(); pointer_data.kind = flutter::PointerData::DeviceKind::kMouse; - pointer_data.change = _mouseState.flutter_state_is_added ? flutter::PointerData::Change::kAdd - : flutter::PointerData::Change::kHover; + pointer_data.change = _mouseState.flutter_state_is_added ? flutter::PointerData::Change::kHover + : flutter::PointerData::Change::kAdd; pointer_data.time_stamp = [[NSProcessInfo processInfo] systemUptime] * kMicrosecondsPerSecond; pointer_data.device = reinterpret_cast(_pointerInteraction.get()); From eeb5e97981e6ad15e7d3bd449904ed9bb09123ed Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Sun, 19 Sep 2021 12:09:36 -0400 Subject: [PATCH 4/9] Poll to remove mouse pointer instead of checking on tap --- .../framework/Source/FlutterViewController.mm | 59 ++++++++++++------- .../ScenariosUITests/iPadGestureTests.m | 21 ++----- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 6fbca8a459b48..01a3a27d4896e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -42,6 +42,9 @@ // Struct holding the mouse state. The engine doesn't keep track of which // mouse buttons have been pressed, so it's the embedding's responsibility. typedef struct MouseState { + // True if at least one mouse event has been received. + bool is_valid; + // True if the last event sent to Flutter had at least one mouse button. // pressed. bool flutter_state_is_down = false; @@ -252,6 +255,13 @@ - (void)performCommonViewControllerInitialization { _statusBarStyle = UIStatusBarStyleDefault; [self setupNotificationCenterObservers]; + if (@available(iOS 13.4, *)) { + [NSTimer scheduledTimerWithTimeInterval:0.5 + repeats:YES + block:^(NSTimer* _Nonnull timer) { + [self handleDetachingMouse]; + }]; + } } - (FlutterEngine*)engine { @@ -780,6 +790,31 @@ - (void)flushOngoingTouches { } } +- (void)handleDetachingMouse API_AVAILABLE(ios(13.4)) { + if (!_mouseState.is_valid) { + return; + } + if (_mouseState.flutter_state_is_added && + [UIFocusSystem focusSystemForEnvironment:self.view] == nil) { + // There is no focus environment, therefore the mouse is no longer attached + auto packet = std::make_unique(1); + flutter::PointerData pointer_data = [self generatePointerDataForMouse]; + pointer_data.change = flutter::PointerData::Change::kRemove; + packet->SetPointerData(0, pointer_data); + [_engine.get() dispatchPointerDataPacket:std::move(packet)]; + _mouseState.flutter_state_is_added = false; + } else if (!_mouseState.flutter_state_is_added && + [UIFocusSystem focusSystemForEnvironment:self.view] != nil) { + // There is now a focus environment, therefore the mouse should be re-added + auto packet = std::make_unique(1); + flutter::PointerData pointer_data = [self generatePointerDataForMouse]; + pointer_data.change = flutter::PointerData::Change::kAdd; + packet->SetPointerData(0, pointer_data); + [_engine.get() dispatchPointerDataPacket:std::move(packet)]; + _mouseState.flutter_state_is_added = true; + } +} + - (void)deregisterNotifications { [[NSNotificationCenter defaultCenter] postNotificationName:FlutterViewControllerWillDealloc object:self @@ -891,31 +926,10 @@ - (void)dispatchTouches:(NSSet*)touches } const CGFloat scale = [UIScreen mainScreen].scale; - - bool will_drop_hover = NO; - - if (@available(iOS 13.4, *)) { - if (_mouseState.flutter_state_is_added && - [UIFocusSystem focusSystemForEnvironment:self.view] == nil) { - // There is no focus environment, the pointer is no longer attached - will_drop_hover = YES; - } - } - - auto packet = std::make_unique(will_drop_hover ? touches.count + 1 - : touches.count); + auto packet = std::make_unique(touches.count); size_t pointer_index = 0; - if (@available(iOS 13.4, *)) { - if (will_drop_hover) { - _mouseState.flutter_state_is_added = false; - flutter::PointerData drop_hover_pointer_data = [self generatePointerDataForMouse]; - drop_hover_pointer_data.change = flutter::PointerData::Change::kRemove; - packet->SetPointerData(pointer_index++, drop_hover_pointer_data); - } - } - for (UITouch* touch in touches) { CGPoint windowCoordinates = [touch locationInView:self.view]; @@ -1728,6 +1742,7 @@ - (UIPointerRegion*)pointerInteraction:(UIPointerInteraction*)interaction [_engine.get() dispatchPointerDataPacket:std::move(packet)]; _mouseState.flutter_state_is_added = true; + _mouseState.is_valid = true; } return nil; } diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m index 0f061f3f33ba7..6e342c8587551 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m @@ -128,23 +128,10 @@ - (void)testPointerHover { XCTAssertTrue( [app.textFields[@"0,PointerChange.add,device=0,buttons=0"] waitForExistenceWithTimeout:1], @"PointerChange.add event did not occur for a hover"); - [flutterView tap]; - XCTAssertTrue( - [app.textFields[@"1,PointerChange.add,device=1,buttons=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a tap"); - XCTAssertTrue( - [app.textFields[@"2,PointerChange.down,device=1,buttons=0"] waitForExistenceWithTimeout:1], - @"PointerChange.down event did not occur for a tap"); - XCTAssertTrue( - [app.textFields[@"3,PointerChange.up,device=1,buttons=0"] waitForExistenceWithTimeout:1], - @"PointerChange.up event did not occur for a tap"); - XCTAssertTrue( - [app.textFields[@"4,PointerChange.remove,device=0,buttons=0"] waitForExistenceWithTimeout:1], - @"The hover pointer was not removed after a tap"); - [flutterView performSelector:hover]; - XCTAssertTrue( - [app.textFields[@"5,PointerChange.add,device=0,buttons=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a subsequent hover"); + // TODO: The Xcode 13 simulator is currently broken for hover events + // It's not known whether the simulator iPad will act as if it has a keyboard or not + // So once the simulator is fixed, this test should be updated to ensure it + // tests the behavior of the hover pointer properly. } #pragma clang diagnostic pop From fa6b8464538a31573473ca177c04d8ca843e2a3f Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Mon, 20 Sep 2021 01:56:11 -0400 Subject: [PATCH 5/9] Fix retain cycle --- .../framework/Source/FlutterViewController.mm | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 01a3a27d4896e..133f7b91fdb5c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -124,6 +124,7 @@ @implementation FlutterViewController { fml::scoped_nsobject _panGestureRecognizer API_AVAILABLE(ios(13.4)); fml::scoped_nsobject _keyboardAnimationView; MouseState _mouseState; + fml::scoped_nsobject _mousePollTimer API_AVAILABLE(ios(13.4)); } @synthesize displayingFlutterUI = _displayingFlutterUI; @@ -256,11 +257,13 @@ - (void)performCommonViewControllerInitialization { [self setupNotificationCenterObservers]; if (@available(iOS 13.4, *)) { - [NSTimer scheduledTimerWithTimeInterval:0.5 - repeats:YES - block:^(NSTimer* _Nonnull timer) { - [self handleDetachingMouse]; - }]; + auto weakSelf = [self getWeakPtr]; + _mousePollTimer.reset([[NSTimer scheduledTimerWithTimeInterval:0.5 + repeats:YES + block:^(NSTimer* _Nonnull timer) { + [weakSelf.get() + handleDetachingMouse]; + }] retain]); } } @@ -831,6 +834,9 @@ - (void)dealloc { [self deregisterNotifications]; [_displayLink release]; + if (@available(iOS 13.4, *)) { + [_mousePollTimer.get() invalidate]; + } [super dealloc]; } From 7d9c4fd6c3cb14ec25e791a4a09e240de729d774 Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Sun, 7 Nov 2021 18:16:51 -0500 Subject: [PATCH 6/9] Use UIHoverGestureRecognizer instead of UIPointerInteraction --- .../framework/Headers/FlutterViewController.h | 2 +- .../framework/Source/FlutterViewController.mm | 118 ++++++------------ .../ScenariosUITests/iPadGestureTests.m | 86 ++++++++++--- 3 files changed, 111 insertions(+), 95 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h b/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h index 7a5219fb71927..9f71eeccb1a35 100644 --- a/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h +++ b/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h @@ -50,7 +50,7 @@ extern NSNotificationName const FlutterSemanticsUpdateNotification; FLUTTER_DARWIN_EXPORT #ifdef __IPHONE_13_4 @interface FlutterViewController - : UIViewController + : UIViewController #else @interface FlutterViewController : UIViewController #endif diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 133f7b91fdb5c..107a77172e9be 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -39,29 +39,13 @@ NSNotificationName const FlutterViewControllerShowHomeIndicator = @"FlutterViewControllerShowHomeIndicator"; -// Struct holding the mouse state. The engine doesn't keep track of which -// mouse buttons have been pressed, so it's the embedding's responsibility. +// Struct holding the mouse state. typedef struct MouseState { - // True if at least one mouse event has been received. - bool is_valid; - - // True if the last event sent to Flutter had at least one mouse button. - // pressed. - bool flutter_state_is_down = false; - - // True if kAdd has been sent to Flutter. Used to determine whether - // to send a kAdd event before sending an incoming mouse event, since - // Flutter expects pointers to be added before events are sent for them. - bool flutter_state_is_added = false; - // Current coordinate of the mouse cursor in physical device pixels. CGPoint location = CGPointZero; // Last reported translation for an in-flight pan gesture in physical device pixels. CGPoint last_translation = CGPointZero; - - // The currently pressed buttons, as represented in FlutterPointerEvent. - uint64_t buttons = 0; } MouseState; // This is left a FlutterBinaryMessenger privately for now to give people a chance to notice the @@ -120,11 +104,10 @@ @implementation FlutterViewController { // UIScrollView with height zero and a content offset so we can get those events. See also: // https://github.com/flutter/flutter/issues/35050 fml::scoped_nsobject _scrollView; - fml::scoped_nsobject _pointerInteraction API_AVAILABLE(ios(13.4)); + fml::scoped_nsobject _hoverGestureRecognizer API_AVAILABLE(ios(13.4)); fml::scoped_nsobject _panGestureRecognizer API_AVAILABLE(ios(13.4)); fml::scoped_nsobject _keyboardAnimationView; MouseState _mouseState; - fml::scoped_nsobject _mousePollTimer API_AVAILABLE(ios(13.4)); } @synthesize displayingFlutterUI = _displayingFlutterUI; @@ -256,15 +239,6 @@ - (void)performCommonViewControllerInitialization { _statusBarStyle = UIStatusBarStyleDefault; [self setupNotificationCenterObservers]; - if (@available(iOS 13.4, *)) { - auto weakSelf = [self getWeakPtr]; - _mousePollTimer.reset([[NSTimer scheduledTimerWithTimeInterval:0.5 - repeats:YES - block:^(NSTimer* _Nonnull timer) { - [weakSelf.get() - handleDetachingMouse]; - }] retain]); - } } - (FlutterEngine*)engine { @@ -671,11 +645,14 @@ - (void)viewDidLoad { } if (@available(iOS 13.4, *)) { - _pointerInteraction.reset([[UIPointerInteraction alloc] initWithDelegate:self]); - [self.view addInteraction:_pointerInteraction]; + _hoverGestureRecognizer.reset( + [[UIHoverGestureRecognizer alloc] initWithTarget:self action:@selector(hoverEvent:)]); + _hoverGestureRecognizer.get().delegate = self; + [_flutterView.get() addGestureRecognizer:_hoverGestureRecognizer.get()]; _panGestureRecognizer.reset( [[UIPanGestureRecognizer alloc] initWithTarget:self action:@selector(scrollEvent:)]); + _panGestureRecognizer.get().delegate = self; _panGestureRecognizer.get().allowedScrollTypesMask = UIScrollTypeMaskAll; _panGestureRecognizer.get().allowedTouchTypes = @[ @(UITouchTypeIndirectPointer) ]; [_flutterView.get() addGestureRecognizer:_panGestureRecognizer.get()]; @@ -793,31 +770,6 @@ - (void)flushOngoingTouches { } } -- (void)handleDetachingMouse API_AVAILABLE(ios(13.4)) { - if (!_mouseState.is_valid) { - return; - } - if (_mouseState.flutter_state_is_added && - [UIFocusSystem focusSystemForEnvironment:self.view] == nil) { - // There is no focus environment, therefore the mouse is no longer attached - auto packet = std::make_unique(1); - flutter::PointerData pointer_data = [self generatePointerDataForMouse]; - pointer_data.change = flutter::PointerData::Change::kRemove; - packet->SetPointerData(0, pointer_data); - [_engine.get() dispatchPointerDataPacket:std::move(packet)]; - _mouseState.flutter_state_is_added = false; - } else if (!_mouseState.flutter_state_is_added && - [UIFocusSystem focusSystemForEnvironment:self.view] != nil) { - // There is now a focus environment, therefore the mouse should be re-added - auto packet = std::make_unique(1); - flutter::PointerData pointer_data = [self generatePointerDataForMouse]; - pointer_data.change = flutter::PointerData::Change::kAdd; - packet->SetPointerData(0, pointer_data); - [_engine.get() dispatchPointerDataPacket:std::move(packet)]; - _mouseState.flutter_state_is_added = true; - } -} - - (void)deregisterNotifications { [[NSNotificationCenter defaultCenter] postNotificationName:FlutterViewControllerWillDealloc object:self @@ -834,9 +786,6 @@ - (void)dealloc { [self deregisterNotifications]; [_displayLink release]; - if (@available(iOS 13.4, *)) { - [_mousePollTimer.get() invalidate]; - } [super dealloc]; } @@ -1721,10 +1670,26 @@ - (BOOL)isPresentingViewController { pointer_data.Clear(); pointer_data.kind = flutter::PointerData::DeviceKind::kMouse; - pointer_data.change = _mouseState.flutter_state_is_added ? flutter::PointerData::Change::kHover - : flutter::PointerData::Change::kAdd; + switch (_hoverGestureRecognizer.get().state) { + case UIGestureRecognizerStateBegan: + pointer_data.change = flutter::PointerData::Change::kAdd; + break; + case UIGestureRecognizerStateChanged: + pointer_data.change = flutter::PointerData::Change::kHover; + break; + case UIGestureRecognizerStateEnded: + case UIGestureRecognizerStateCancelled: + pointer_data.change = flutter::PointerData::Change::kRemove; + break; + default: + FML_LOG(ERROR) << "Unhandled hover phase: " << _hoverGestureRecognizer.get().state; + // Sending kHover is the least harmful thing to do here + // But this state is not expected to ever be reached. + pointer_data.change = flutter::PointerData::Change::kHover; + break; + } pointer_data.time_stamp = [[NSProcessInfo processInfo] systemUptime] * kMicrosecondsPerSecond; - pointer_data.device = reinterpret_cast(_pointerInteraction.get()); + pointer_data.device = reinterpret_cast(_hoverGestureRecognizer.get()); pointer_data.physical_x = _mouseState.location.x; pointer_data.physical_y = _mouseState.location.y; @@ -1732,25 +1697,24 @@ - (BOOL)isPresentingViewController { return pointer_data; } -- (UIPointerRegion*)pointerInteraction:(UIPointerInteraction*)interaction - regionForRequest:(UIPointerRegionRequest*)request - defaultRegion:(UIPointerRegion*)defaultRegion API_AVAILABLE(ios(13.4)) { - if (request != nil) { - auto packet = std::make_unique(1); - const CGFloat scale = [UIScreen mainScreen].scale; - _mouseState.location = {request.location.x * scale, request.location.y * scale}; +- (BOOL)gestureRecognizer:(UIGestureRecognizer*)gestureRecognizer + shouldRecognizeSimultaneouslyWithGestureRecognizer:(UIGestureRecognizer*)otherGestureRecognizer + API_AVAILABLE(ios(13.4)) { + return YES; +} - flutter::PointerData pointer_data = [self generatePointerDataForMouse]; +- (void)hoverEvent:(UIPanGestureRecognizer*)recognizer API_AVAILABLE(ios(13.4)) { + auto packet = std::make_unique(1); + CGPoint location = [recognizer locationInView:self.view]; + CGFloat scale = [UIScreen mainScreen].scale; + _mouseState.location = {location.x * scale, location.y * scale}; - pointer_data.signal_kind = flutter::PointerData::SignalKind::kNone; - packet->SetPointerData(/*index=*/0, pointer_data); + flutter::PointerData pointer_data = [self generatePointerDataForMouse]; - [_engine.get() dispatchPointerDataPacket:std::move(packet)]; + pointer_data.signal_kind = flutter::PointerData::SignalKind::kNone; + packet->SetPointerData(/*index=*/0, pointer_data); - _mouseState.flutter_state_is_added = true; - _mouseState.is_valid = true; - } - return nil; + [_engine.get() dispatchPointerDataPacket:std::move(packet)]; } - (void)scrollEvent:(UIPanGestureRecognizer*)recognizer API_AVAILABLE(ios(13.4)) { @@ -1780,8 +1744,6 @@ - (void)scrollEvent:(UIPanGestureRecognizer*)recognizer API_AVAILABLE(ios(13.4)) packet->SetPointerData(/*index=*/0, pointer_data); [_engine.get() dispatchPointerDataPacket:std::move(packet)]; - - _mouseState.flutter_state_is_added = true; } #pragma mark - State Restoration diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m index 6e342c8587551..5ec40d8177216 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m @@ -28,6 +28,13 @@ static BOOL performBoolSelector(id target, SEL selector) { return returnValue; } +static int assertOneMessageAndGetSequenceNo(NSMutableDictionary* messages, NSString* message) { + NSMutableArray* matchingMessages = [messages objectForKey:message]; + XCTAssertNotNil(matchingMessages, @"Did not receive \"%@\" message", message); + XCTAssertEqual([matchingMessages count], 1, @"More than one \"%@\" message", message); + return [[matchingMessages firstObject] intValue]; +} + // TODO(85810): Remove reflection in this test when Xcode version is upgraded to 13. #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wundeclared-selector" @@ -76,20 +83,58 @@ - (void)testPointerButtons { XCTAssertTrue([flutterView respondsToSelector:rightClick], @"If supportsPointerInteraction is true, this should be true too."); [flutterView performSelector:rightClick]; - // Right-clicking will trigger the hover pointer as well + // On simulated right click, a hover also occurs, so the hover pointer is added XCTAssertTrue( [app.textFields[@"3,PointerChange.add,device=1,buttons=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a right-click"); - XCTAssertTrue( - [app.textFields[@"4,PointerChange.add,device=2,buttons=0"] waitForExistenceWithTimeout:1], - @"PointerChange.add event did not occur for a right-click"); + @"PointerChange.add event did not occur for a right-click's hover pointer"); + [NSThread sleepForTimeInterval:5.0]; // The hover pointer is removed after ~3.5 seconds, this + // ensures all events have been received + // The hover events are interspersed with the right-click events in a varying order + // Ensure the individual orderings are respected without hardcoding the absolute sequence + NSMutableDictionary* messages = [NSMutableDictionary dictionary]; + for (XCUIElement* element in [app.textFields allElementsBoundByIndex]) { + NSString* rawMessage = element.value; + NSUInteger commaIndex = [rawMessage rangeOfString:@","].location; + NSInteger messageSequenceNo = + [[rawMessage substringWithRange:NSMakeRange(0, commaIndex)] integerValue]; + NSString* message = [rawMessage + substringWithRange:NSMakeRange(commaIndex + 1, [rawMessage length] - (commaIndex + 1))]; + NSMutableArray* messageSequenceNoList = [messages objectForKey:message]; + if (messageSequenceNoList == nil) { + messageSequenceNoList = [[NSMutableArray alloc] init]; + [messages setObject:messageSequenceNoList forKey:message]; + } + [messageSequenceNoList addObject:[NSNumber numberWithInteger:messageSequenceNo]]; + } + // The number of hover events is not consistent + NSMutableArray* hoverSeqNos = [messages objectForKey:@"PointerChange.hover,device=1,buttons=0"]; + int hoverRemovedSeqNo = + assertOneMessageAndGetSequenceNo(messages, @"PointerChange.remove,device=1,buttons=0"); // Right click should have buttons = 2 - XCTAssertTrue( - [app.textFields[@"5,PointerChange.down,device=2,buttons=2"] waitForExistenceWithTimeout:1], - @"PointerChange.down event did not occur for a right-click"); - XCTAssertTrue( - [app.textFields[@"6,PointerChange.up,device=2,buttons=2"] waitForExistenceWithTimeout:1], - @"PointerChange.up event did not occur for a right-click"); + int rightClickAddedSeqNo, rightClickDownSeqNo, rightClickUpSeqNo; + if ([messages objectForKey:@"PointerChange.add,device=2,buttons=0"] == nil) { + // Sometimes the tap pointer has the same device as the right-click (the UITouch is reused) + rightClickAddedSeqNo = 0; + rightClickDownSeqNo = + assertOneMessageAndGetSequenceNo(messages, @"PointerChange.down,device=0,buttons=2"); + rightClickUpSeqNo = + assertOneMessageAndGetSequenceNo(messages, @"PointerChange.up,device=0,buttons=2"); + } else { + rightClickAddedSeqNo = + assertOneMessageAndGetSequenceNo(messages, @"PointerChange.add,device=2,buttons=0"); + rightClickDownSeqNo = + assertOneMessageAndGetSequenceNo(messages, @"PointerChange.down,device=2,buttons=2"); + rightClickUpSeqNo = + assertOneMessageAndGetSequenceNo(messages, @"PointerChange.up,device=2,buttons=2"); + } + XCTAssertGreaterThan(rightClickDownSeqNo, rightClickAddedSeqNo, + @"Right-click pointer was down before add"); + XCTAssertGreaterThan(rightClickUpSeqNo, rightClickDownSeqNo, + @"Right-click pointer was up before down"); + XCTAssertGreaterThan([[hoverSeqNos firstObject] intValue], 3, + @"Right-click pointer caused hover before hover pointer add"); + XCTAssertGreaterThan(hoverRemovedSeqNo, [[hoverSeqNos lastObject] intValue], + @"Right-click pointer's hover pointer had hover event after it was removed"); } - (void)testPointerHover { @@ -124,14 +169,23 @@ - (void)testPointerHover { XCTAssertTrue([flutterView respondsToSelector:hover], @"If supportsPointerInteraction is true, this should be true too."); [flutterView performSelector:hover]; - [NSThread sleepForTimeInterval:1.0]; XCTAssertTrue( [app.textFields[@"0,PointerChange.add,device=0,buttons=0"] waitForExistenceWithTimeout:1], @"PointerChange.add event did not occur for a hover"); - // TODO: The Xcode 13 simulator is currently broken for hover events - // It's not known whether the simulator iPad will act as if it has a keyboard or not - // So once the simulator is fixed, this test should be updated to ensure it - // tests the behavior of the hover pointer properly. + XCTAssertTrue( + [app.textFields[@"1,PointerChange.hover,device=0,buttons=0"] waitForExistenceWithTimeout:1], + @"PointerChange.hover event did not occur for a hover"); + // The number of hover events fired is not always the same + int i = 2; + while ( + [app.textFields[[NSString stringWithFormat:@"%d,PointerChange.hover,device=0,buttons=0", i]] + waitForExistenceWithTimeout:1]) { + i++; + } + NSString* removeMessage = + [NSString stringWithFormat:@"%d,PointerChange.remove,device=0,buttons=0", i]; + XCTAssertTrue([app.textFields[removeMessage] waitForExistenceWithTimeout:1], + @"PointerChange.remove event did not occur for a hover"); } #pragma clang diagnostic pop From 2b26b23471540537c5bacaef01f118e04d0cbe4c Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Thu, 11 Nov 2021 18:49:24 -0500 Subject: [PATCH 7/9] Remove log message, clear delegates on dealloc --- .../darwin/ios/framework/Source/FlutterViewController.mm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 107a77172e9be..825e56b84a563 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -786,6 +786,9 @@ - (void)dealloc { [self deregisterNotifications]; [_displayLink release]; + _scrollView.get().delegate = nil; + _hoverGestureRecognizer.get().delegate = nil; + _panGestureRecognizer.get().delegate = nil; [super dealloc]; } @@ -1682,7 +1685,6 @@ - (BOOL)isPresentingViewController { pointer_data.change = flutter::PointerData::Change::kRemove; break; default: - FML_LOG(ERROR) << "Unhandled hover phase: " << _hoverGestureRecognizer.get().state; // Sending kHover is the least harmful thing to do here // But this state is not expected to ever be reached. pointer_data.change = flutter::PointerData::Change::kHover; From 821c4b55ac8d1778365b692c448d3ccd4544c34e Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Mon, 15 Nov 2021 17:48:09 -0500 Subject: [PATCH 8/9] Address test feedback about syntax and readability --- .../ScenariosUITests/iPadGestureTests.m | 104 +++++++++--------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m index 5ec40d8177216..d84a8ee810e0c 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m @@ -28,11 +28,11 @@ static BOOL performBoolSelector(id target, SEL selector) { return returnValue; } -static int assertOneMessageAndGetSequenceNo(NSMutableDictionary* messages, NSString* message) { - NSMutableArray* matchingMessages = [messages objectForKey:message]; +static int assertOneMessageAndGetSequenceNumber(NSMutableDictionary* messages, NSString* message) { + NSMutableArray* matchingMessages = messages[message]; XCTAssertNotNil(matchingMessages, @"Did not receive \"%@\" message", message); - XCTAssertEqual([matchingMessages count], 1, @"More than one \"%@\" message", message); - return [[matchingMessages firstObject] intValue]; + XCTAssertEqual(matchingMessages.count, 1, @"More than one \"%@\" message", message); + return matchingMessages.firstObject.intValue; } // TODO(85810): Remove reflection in this test when Xcode version is upgraded to 13. @@ -52,11 +52,7 @@ - (void)testPointerButtons { [app launch]; NSPredicate* predicateToFindFlutterView = - [NSPredicate predicateWithBlock:^BOOL(id _Nullable evaluatedObject, - NSDictionary* _Nullable bindings) { - XCUIElement* element = evaluatedObject; - return [element.identifier hasPrefix:@"flutter_view"]; - }]; + [NSPredicate predicateWithFormat:@"identifier BEGINSWITH 'flutter_view'"]; XCUIElement* flutterView = [[app descendantsMatchingType:XCUIElementTypeAny] elementMatchingPredicate:predicateToFindFlutterView]; if (![flutterView waitForExistenceWithTimeout:kSecondsToWaitForFlutterView]) { @@ -87,54 +83,64 @@ - (void)testPointerButtons { XCTAssertTrue( [app.textFields[@"3,PointerChange.add,device=1,buttons=0"] waitForExistenceWithTimeout:1], @"PointerChange.add event did not occur for a right-click's hover pointer"); - [NSThread sleepForTimeInterval:5.0]; // The hover pointer is removed after ~3.5 seconds, this - // ensures all events have been received + + // The hover pointer is removed after ~3.5 seconds, this ensures that all events are received + XCTestExpectation* sleepExpectation = [self expectationWithDescription:@"never fires"]; + sleepExpectation.inverted = true; + [self waitForExpectations:@[ sleepExpectation ] timeout:5.0]; + // The hover events are interspersed with the right-click events in a varying order // Ensure the individual orderings are respected without hardcoding the absolute sequence - NSMutableDictionary* messages = [NSMutableDictionary dictionary]; + NSMutableDictionary*>* messages = + [[NSMutableDictionary alloc] init]; for (XCUIElement* element in [app.textFields allElementsBoundByIndex]) { NSString* rawMessage = element.value; + // Parse out the sequence number NSUInteger commaIndex = [rawMessage rangeOfString:@","].location; - NSInteger messageSequenceNo = - [[rawMessage substringWithRange:NSMakeRange(0, commaIndex)] integerValue]; + NSInteger messageSequenceNumber = + [rawMessage substringWithRange:NSMakeRange(0, commaIndex)].integerValue; + // Parse out the rest of the message NSString* message = [rawMessage - substringWithRange:NSMakeRange(commaIndex + 1, [rawMessage length] - (commaIndex + 1))]; - NSMutableArray* messageSequenceNoList = [messages objectForKey:message]; - if (messageSequenceNoList == nil) { - messageSequenceNoList = [[NSMutableArray alloc] init]; - [messages setObject:messageSequenceNoList forKey:message]; + substringWithRange:NSMakeRange(commaIndex + 1, rawMessage.length - (commaIndex + 1))]; + NSMutableArray* messageSequenceNumberList = messages[message]; + if (messageSequenceNumberList == nil) { + messageSequenceNumberList = [[NSMutableArray alloc] init]; + messages[message] = messageSequenceNumberList; } - [messageSequenceNoList addObject:[NSNumber numberWithInteger:messageSequenceNo]]; + [messageSequenceNumberList addObject:@(messageSequenceNumber)]; } - // The number of hover events is not consistent - NSMutableArray* hoverSeqNos = [messages objectForKey:@"PointerChange.hover,device=1,buttons=0"]; - int hoverRemovedSeqNo = - assertOneMessageAndGetSequenceNo(messages, @"PointerChange.remove,device=1,buttons=0"); + // The number of hover events is not consistent, there could be one or many + NSMutableArray* hoverSequenceNumbers = + messages[@"PointerChange.hover,device=1,buttons=0"]; + int hoverRemovedSequenceNumber = + assertOneMessageAndGetSequenceNumber(messages, @"PointerChange.remove,device=1,buttons=0"); // Right click should have buttons = 2 - int rightClickAddedSeqNo, rightClickDownSeqNo, rightClickUpSeqNo; - if ([messages objectForKey:@"PointerChange.add,device=2,buttons=0"] == nil) { + int rightClickAddedSequenceNumber; + int rightClickDownSequenceNumber; + int rightClickUpSequenceNumber; + if (messages[@"PointerChange.add,device=2,buttons=0"] == nil) { // Sometimes the tap pointer has the same device as the right-click (the UITouch is reused) - rightClickAddedSeqNo = 0; - rightClickDownSeqNo = - assertOneMessageAndGetSequenceNo(messages, @"PointerChange.down,device=0,buttons=2"); - rightClickUpSeqNo = - assertOneMessageAndGetSequenceNo(messages, @"PointerChange.up,device=0,buttons=2"); + rightClickAddedSequenceNumber = 0; + rightClickDownSequenceNumber = + assertOneMessageAndGetSequenceNumber(messages, @"PointerChange.down,device=0,buttons=2"); + rightClickUpSequenceNumber = + assertOneMessageAndGetSequenceNumber(messages, @"PointerChange.up,device=0,buttons=2"); } else { - rightClickAddedSeqNo = - assertOneMessageAndGetSequenceNo(messages, @"PointerChange.add,device=2,buttons=0"); - rightClickDownSeqNo = - assertOneMessageAndGetSequenceNo(messages, @"PointerChange.down,device=2,buttons=2"); - rightClickUpSeqNo = - assertOneMessageAndGetSequenceNo(messages, @"PointerChange.up,device=2,buttons=2"); + rightClickAddedSequenceNumber = + assertOneMessageAndGetSequenceNumber(messages, @"PointerChange.add,device=2,buttons=0"); + rightClickDownSequenceNumber = + assertOneMessageAndGetSequenceNumber(messages, @"PointerChange.down,device=2,buttons=2"); + rightClickUpSequenceNumber = + assertOneMessageAndGetSequenceNumber(messages, @"PointerChange.up,device=2,buttons=2"); } - XCTAssertGreaterThan(rightClickDownSeqNo, rightClickAddedSeqNo, - @"Right-click pointer was down before add"); - XCTAssertGreaterThan(rightClickUpSeqNo, rightClickDownSeqNo, - @"Right-click pointer was up before down"); - XCTAssertGreaterThan([[hoverSeqNos firstObject] intValue], 3, - @"Right-click pointer caused hover before hover pointer add"); - XCTAssertGreaterThan(hoverRemovedSeqNo, [[hoverSeqNos lastObject] intValue], - @"Right-click pointer's hover pointer had hover event after it was removed"); + XCTAssertGreaterThan(rightClickDownSequenceNumber, rightClickAddedSequenceNumber, + @"Right-click pointer was pressed before it was added"); + XCTAssertGreaterThan(rightClickUpSequenceNumber, rightClickDownSequenceNumber, + @"Right-click pointer was released before it was pressed"); + XCTAssertGreaterThan([[hoverSequenceNumbers firstObject] intValue], 3, + @"Hover occured before hover pointer was added"); + XCTAssertGreaterThan(hoverRemovedSequenceNumber, [[hoverSequenceNumbers lastObject] intValue], + @"Hover occured after hover pointer was removed"); } - (void)testPointerHover { @@ -150,11 +156,7 @@ - (void)testPointerHover { [app launch]; NSPredicate* predicateToFindFlutterView = - [NSPredicate predicateWithBlock:^BOOL(id _Nullable evaluatedObject, - NSDictionary* _Nullable bindings) { - XCUIElement* element = evaluatedObject; - return [element.identifier hasPrefix:@"flutter_view"]; - }]; + [NSPredicate predicateWithFormat:@"identifier BEGINSWITH 'flutter_view'"]; XCUIElement* flutterView = [[app descendantsMatchingType:XCUIElementTypeAny] elementMatchingPredicate:predicateToFindFlutterView]; if (![flutterView waitForExistenceWithTimeout:kSecondsToWaitForFlutterView]) { @@ -176,7 +178,7 @@ - (void)testPointerHover { [app.textFields[@"1,PointerChange.hover,device=0,buttons=0"] waitForExistenceWithTimeout:1], @"PointerChange.hover event did not occur for a hover"); // The number of hover events fired is not always the same - int i = 2; + NSUInteger i = 2; while ( [app.textFields[[NSString stringWithFormat:@"%d,PointerChange.hover,device=0,buttons=0", i]] waitForExistenceWithTimeout:1]) { From 1752ef521066a49b87beab33b45ea8697ef9e17d Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Thu, 25 Nov 2021 11:36:12 -0500 Subject: [PATCH 9/9] Use NSPredicate to locate hover events --- .../ScenariosUITests/iPadGestureTests.m | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m index d84a8ee810e0c..685d7f70178c8 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/iPadGestureTests.m @@ -178,14 +178,21 @@ - (void)testPointerHover { [app.textFields[@"1,PointerChange.hover,device=0,buttons=0"] waitForExistenceWithTimeout:1], @"PointerChange.hover event did not occur for a hover"); // The number of hover events fired is not always the same - NSUInteger i = 2; - while ( - [app.textFields[[NSString stringWithFormat:@"%d,PointerChange.hover,device=0,buttons=0", i]] - waitForExistenceWithTimeout:1]) { - i++; + NSInteger lastHoverSequenceNumber = -1; + NSPredicate* predicateToFindHoverEvents = + [NSPredicate predicateWithFormat:@"value ENDSWITH ',PointerChange.hover,device=0,buttons=0'"]; + for (XCUIElement* textField in + [[app.textFields matchingPredicate:predicateToFindHoverEvents] allElementsBoundByIndex]) { + NSInteger messageSequenceNumber = + [[textField.value componentsSeparatedByString:@","] firstObject].integerValue; + if (messageSequenceNumber > lastHoverSequenceNumber) { + lastHoverSequenceNumber = messageSequenceNumber; + } } - NSString* removeMessage = - [NSString stringWithFormat:@"%d,PointerChange.remove,device=0,buttons=0", i]; + XCTAssertNotEqual(lastHoverSequenceNumber, -1, + @"PointerChange.hover event did not occur for a hover"); + NSString* removeMessage = [NSString + stringWithFormat:@"%d,PointerChange.remove,device=0,buttons=0", lastHoverSequenceNumber + 1]; XCTAssertTrue([app.textFields[removeMessage] waitForExistenceWithTimeout:1], @"PointerChange.remove event did not occur for a hover"); }