From 33d6143c4e8c5bfeddf859dbc04cd8d2fc679529 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 27 Jul 2021 17:52:34 -0700 Subject: [PATCH 1/8] Windows --- .../windows/keyboard_key_embedder_handler.cc | 20 +++- .../windows/keyboard_key_embedder_handler.h | 11 +++ ...keyboard_key_embedder_handler_unittests.cc | 99 ++++++++++++++++++- 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index 66595efcb9ed5..b1aa06d1bbddb 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -26,6 +26,7 @@ constexpr size_t kCharacterCacheSize = 8; constexpr SHORT kStateMaskToggled = 0x01; constexpr SHORT kStateMaskPressed = 0x80; +const char* empty_character = ""; } // namespace KeyboardKeyEmbedderHandler::KeyboardKeyEmbedderHandler( @@ -155,6 +156,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHook( // as a currently pressed one, usually indicating multiple keyboards are // pressing keys with the same physical key, or the up event was lost // during a loss of focus. The down event is ignored. + sendEvent_(CreateEmptyEvent(), nullptr, nullptr); callback(true); return; } @@ -171,6 +173,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHook( // The physical key has been released before. It might indicate a missed // event due to loss of focus, or multiple keyboards pressed keys with the // same physical key. Ignore the up event. + sendEvent_(CreateEmptyEvent(), nullptr, nullptr); callback(true); return; } else { @@ -200,6 +203,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHook( // presses are considered handled and not sent to Flutter. These events must // be filtered by result_logical_key because the key up event of such // presses uses the "original" logical key. + sendEvent_(CreateEmptyEvent(), nullptr, nullptr); callback(true); return; } @@ -272,7 +276,6 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( key_info.toggled_on = !key_info.toggled_on; } if (key_info.toggled_on != should_toggled) { - const char* empty_character = ""; // If the key is pressed, release it first. if (pressingRecords_.find(key_info.physical_key) != pressingRecords_.end()) { @@ -390,6 +393,21 @@ void KeyboardKeyEmbedderHandler::ConvertUtf32ToUtf8_(char* out, char32_t ch) { strcpy_s(out, kCharacterCacheSize, Utf8FromUtf16(text).c_str()); } +FlutterKeyEvent KeyboardKeyEmbedderHandler::CreateEmptyEvent() { + return FlutterKeyEvent{ + .struct_size = sizeof(FlutterKeyEvent), + .timestamp = static_cast( + std::chrono::duration_cast( + std::chrono::high_resolution_clock::now().time_since_epoch()) + .count()), + .type = kFlutterKeyEventTypeDown, + .physical = 0, + .logical = 0, + .character = empty_character, + .synthesized = false, + }; +} + FlutterKeyEvent KeyboardKeyEmbedderHandler::SynthesizeSimpleEvent( FlutterKeyEventType type, uint64_t physical, diff --git a/shell/platform/windows/keyboard_key_embedder_handler.h b/shell/platform/windows/keyboard_key_embedder_handler.h index e0132bf2b4c50..04cf500178d9c 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/shell/platform/windows/keyboard_key_embedder_handler.h @@ -21,6 +21,12 @@ namespace {} // namespace // converted |FlutterKeyEvent|s through the embedder API. // // This class communicates with the HardwareKeyboard API in the framework. +// +// Every key event must result in at least one FlutterKeyEvent, even an empty +// one (both logical and physical IDs are 0). This ensures that raw key +// messages are always preceded by key data so that the transit mode is +// correctly inferred. (Technically only the first key event needs so, but for +// simplicity.) class KeyboardKeyEmbedderHandler : public KeyboardKeyHandler::KeyboardKeyHandlerDelegate { public: @@ -117,6 +123,11 @@ class KeyboardKeyEmbedderHandler static uint64_t GetLogicalKey(int key, bool extended, int scancode); static void HandleResponse(bool handled, void* user_data); static void ConvertUtf32ToUtf8_(char* out, char32_t ch); + // Create an empty event. + // + // This is used when no key data needs to be sent. For the reason, see the + // |KeyboardKeyEmbedderHandler| class. + static FlutterKeyEvent CreateEmptyEvent(); static FlutterKeyEvent SynthesizeSimpleEvent(FlutterKeyEventType type, uint64_t physical, uint64_t logical, diff --git a/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc b/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc index ddae527ec9549..c6f405faf6edd 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc @@ -247,6 +247,15 @@ TEST(KeyboardKeyEmbedderHandlerTest, ImeEventsAreIgnored) { [&last_handled](bool handled) { last_handled = handled; }); EXPECT_EQ(last_handled, true); + // The A key down should yield an empty event. + EXPECT_EQ(results.size(), 1); + event = &results[0]; + EXPECT_EQ(event->physical, 0); + EXPECT_EQ(event->logical, 0); + EXPECT_EQ(event->callback, nullptr); + results.clear(); + + // Release A in an IME last_handled = false; handler->KeyboardHook( // The up event for an IME press has a normal virtual key. @@ -254,8 +263,13 @@ TEST(KeyboardKeyEmbedderHandlerTest, ImeEventsAreIgnored) { [&last_handled](bool handled) { last_handled = handled; }); EXPECT_EQ(last_handled, true); - // The entire A press does not yield events. - EXPECT_EQ(results.size(), 0); + // The A key up should yield an empty event. + EXPECT_EQ(results.size(), 1); + event = &results[0]; + EXPECT_EQ(event->physical, 0); + EXPECT_EQ(event->logical, 0); + EXPECT_EQ(event->callback, nullptr); + results.clear(); // Press A out of an IME key_state.Set(kVirtualKeyA, true); @@ -469,6 +483,54 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByVirtualKey) { results.clear(); } +TEST(KeyboardKeyEmbedderHandlerTest, RepeatedDownIsIgnored) { + TestKeystate key_state; + std::vector results; + TestFlutterKeyEvent* event; + bool last_handled = false; + + std::unique_ptr handler = + std::make_unique( + [&results](const FlutterKeyEvent& event, + FlutterKeyEventCallback callback, void* user_data) { + results.emplace_back(event, callback, user_data); + }, + key_state.Getter()); + last_handled = false; + + // Press A (should yield a normal event) + handler->KeyboardHook( + kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false, + [&last_handled](bool handled) { last_handled = handled; }); + EXPECT_EQ(last_handled, false); + EXPECT_EQ(results.size(), 1); + event = &results[0]; + EXPECT_EQ(event->type, kFlutterKeyEventTypeDown); + EXPECT_EQ(event->physical, kPhysicalKeyA); + EXPECT_EQ(event->logical, kLogicalKeyA); + EXPECT_STREQ(event->character, "a"); + EXPECT_EQ(event->synthesized, false); + + event->callback(true, event->user_data); + EXPECT_EQ(last_handled, true); + results.clear(); + + // KeyA's key up is missed. + + // Press A again (should yield an empty event) + last_handled = false; + handler->KeyboardHook( + kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false, + [&last_handled](bool handled) { last_handled = handled; }); + EXPECT_EQ(last_handled, true); + EXPECT_EQ(results.size(), 1); + event = &results[0]; + EXPECT_EQ(event->physical, 0); + EXPECT_EQ(event->logical, 0); + EXPECT_EQ(event->callback, nullptr); + results.clear(); +} + TEST(KeyboardKeyEmbedderHandlerTest, AbruptRepeatIsConvertedToDown) { TestKeystate key_state; std::vector results; @@ -523,6 +585,39 @@ TEST(KeyboardKeyEmbedderHandlerTest, AbruptRepeatIsConvertedToDown) { results.clear(); } +TEST(KeyboardKeyEmbedderHandlerTest, AbruptUpIsIgnored) { + TestKeystate key_state; + std::vector results; + TestFlutterKeyEvent* event; + bool last_handled = false; + + std::unique_ptr handler = + std::make_unique( + [&results](const FlutterKeyEvent& event, + FlutterKeyEventCallback callback, void* user_data) { + results.emplace_back(event, callback, user_data); + }, + key_state.Getter()); + last_handled = false; + + // KeyA's key down is missed. + + key_state.Set(kVirtualKeyA, true); + + // Press A again (should yield an empty event) + last_handled = false; + handler->KeyboardHook( + kVirtualKeyA, kScanCodeKeyA, WM_KEYUP, 'a', false, true, + [&last_handled](bool handled) { last_handled = handled; }); + EXPECT_EQ(last_handled, true); + EXPECT_EQ(results.size(), 1); + event = &results[0]; + EXPECT_EQ(event->physical, 0); + EXPECT_EQ(event->logical, 0); + EXPECT_EQ(event->callback, nullptr); + results.clear(); +} + TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncPressingState) { TestKeystate key_state; std::vector results; From 940ecc88f6c06bc2bac909a1cacd15b9df9eef59 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 28 Jul 2021 15:06:58 -0700 Subject: [PATCH 2/8] Web test --- lib/web_ui/lib/src/engine/keyboard_binding.dart | 11 +++++++++++ lib/web_ui/test/keyboard_converter_test.dart | 12 ++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/keyboard_binding.dart b/lib/web_ui/lib/src/engine/keyboard_binding.dart index 8b74094900b80..25ecba8e56dbd 100644 --- a/lib/web_ui/lib/src/engine/keyboard_binding.dart +++ b/lib/web_ui/lib/src/engine/keyboard_binding.dart @@ -76,6 +76,15 @@ const int _kDeadKeyShift = 0x20000000; const int _kDeadKeyAlt = 0x40000000; const int _kDeadKeyMeta = 0x80000000; +const ui.KeyData _emptyKeyData = ui.KeyData( + type: ui.KeyEventType.down, + timeStamp: Duration.zero, + logical: 0, + physical: 0, + character: null, + synthesized: false, +); + typedef DispatchKeyData = bool Function(ui.KeyData data); /// Converts a floating number timestamp (in milliseconds) to a [Duration] by @@ -400,6 +409,7 @@ class KeyboardConverter { // a currently pressed one, usually indicating multiple keyboards are // pressing keys with the same physical key, or the up event was lost // during a loss of focus. The down event is ignored. + dispatchKeyData(_emptyKeyData); return; } } else { @@ -412,6 +422,7 @@ class KeyboardConverter { if (lastLogicalRecord == null) { // The physical key has been released before. It indicates multiple // keyboards pressed keys with the same physical key. Ignore the up event. + dispatchKeyData(_emptyKeyData); return; } diff --git a/lib/web_ui/test/keyboard_converter_test.dart b/lib/web_ui/test/keyboard_converter_test.dart index 30d2fbeadb93b..b471bc0e6aef3 100644 --- a/lib/web_ui/test/keyboard_converter_test.dart +++ b/lib/web_ui/test/keyboard_converter_test.dart @@ -369,8 +369,10 @@ void testMain() { converter.handleEvent(keyDownEvent('ShiftLeft', 'Shift', kShift, kLocationLeft) ..onPreventDefault = onPreventDefault ); - expect(keyDataList, isEmpty); - expect(preventedDefault, isFalse); + expect(keyDataList, hasLength(1)); + expect(keyDataList[0].physical, 0); + expect(keyDataList[0].logical, 0); + expect(preventedDefault, isTrue); converter.handleEvent(keyUpEvent('ShiftLeft', 'Shift', 0, kLocationLeft) ..onPreventDefault = onPreventDefault @@ -398,8 +400,10 @@ void testMain() { converter.handleEvent(keyUpEvent('ShiftRight', 'Shift', 0, kLocationRight) ..onPreventDefault = onPreventDefault ); - expect(keyDataList, isEmpty); - expect(preventedDefault, isFalse); + expect(keyDataList, hasLength(1)); + expect(keyDataList[0].physical, 0); + expect(keyDataList[0].logical, 0); + expect(preventedDefault, isTrue); }); test('Conflict from multiple keyboards do not crash', () { From 9091ef654f826d973e1f6f1423abe6bfa81e4023 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 29 Jul 2021 00:21:04 -0700 Subject: [PATCH 3/8] Web and macOS --- .../lib/src/engine/keyboard_binding.dart | 2 ++ lib/web_ui/test/keyboard_converter_test.dart | 1 + .../Source/FlutterEmbedderKeyResponder.mm | 25 +++++++++++++++ .../FlutterEmbedderKeyResponderUnittests.mm | 31 ++++++++++++++++--- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/keyboard_binding.dart b/lib/web_ui/lib/src/engine/keyboard_binding.dart index 25ecba8e56dbd..6cfe91218bfd4 100644 --- a/lib/web_ui/lib/src/engine/keyboard_binding.dart +++ b/lib/web_ui/lib/src/engine/keyboard_binding.dart @@ -410,6 +410,7 @@ class KeyboardConverter { // pressing keys with the same physical key, or the up event was lost // during a loss of focus. The down event is ignored. dispatchKeyData(_emptyKeyData); + event.preventDefault(); return; } } else { @@ -423,6 +424,7 @@ class KeyboardConverter { // The physical key has been released before. It indicates multiple // keyboards pressed keys with the same physical key. Ignore the up event. dispatchKeyData(_emptyKeyData); + event.preventDefault(); return; } diff --git a/lib/web_ui/test/keyboard_converter_test.dart b/lib/web_ui/test/keyboard_converter_test.dart index ca9abdb767ee4..86037bdaf5a05 100644 --- a/lib/web_ui/test/keyboard_converter_test.dart +++ b/lib/web_ui/test/keyboard_converter_test.dart @@ -374,6 +374,7 @@ void testMain() { expect(keyDataList[0].logical, 0); expect(preventedDefault, isTrue); + keyDataList.clear(); converter.handleEvent(keyUpEvent('ShiftLeft', 'Shift', 0, kLocationLeft) ..onPreventDefault = onPreventDefault ); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index faae4348ceec0..263c8d11d3921 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -423,6 +423,13 @@ - (void)sendModifierEventOfType:(BOOL)isDownEvent keyCode:(unsigned short)keyCode callback:(nullable FlutterKeyCallbackGuard*)callback; +/** + * Send an empty key event. + * + * The event is always synthesized. + */ +- (void)sendEmptyEvent; + /** * Processes a down event from the system. */ @@ -579,6 +586,7 @@ - (void)sendModifierEventOfType:(BOOL)isDownEvent uint64_t logicalKey = GetLogicalKeyForModifier(keyCode, physicalKey); if (physicalKey == 0 || logicalKey == 0) { NSLog(@"Unrecognized modifier key: keyCode 0x%hx, physical key 0x%llx", keyCode, physicalKey); + [self sendEmptyEvent]; [callback resolveTo:TRUE]; return; } @@ -599,6 +607,20 @@ - (void)sendModifierEventOfType:(BOOL)isDownEvent } } +- (void)sendEmptyEvent { + FlutterKeyEvent flutterEvent = { + .struct_size = sizeof(FlutterKeyEvent), + .timestamp = 0, + .type = kFlutterKeyEventTypeDown, + .physical = 0, + .logical = 0, + .character = nil, + .synthesized = false, + }; + _sendEvent(flutterEvent, nullptr, nullptr); +} + + - (void)handleDownEvent:(NSEvent*)event callback:(FlutterKeyCallbackGuard*)callback { uint64_t physicalKey = GetPhysicalKeyForKeyCode(event.keyCode); uint64_t logicalKey = GetLogicalKeyForEvent(event, physicalKey); @@ -611,6 +633,7 @@ - (void)handleDownEvent:(NSEvent*)event callback:(FlutterKeyCallbackGuard*)callb // key up event to the window where the corresponding key down occurred. // However this might happen in add-to-app scenarios if the focus is changed // from the native view to the Flutter view amid the key tap. + [self sendEmptyEvent]; [callback resolveTo:TRUE]; return; } @@ -643,6 +666,7 @@ - (void)handleUpEvent:(NSEvent*)event callback:(FlutterKeyCallbackGuard*)callbac // key up event to the window where the corresponding key down occurred. // However this might happen in add-to-app scenarios if the focus is changed // from the native view to the Flutter view amid the key tap. + [self sendEmptyEvent]; [callback resolveTo:TRUE]; return; } @@ -669,6 +693,7 @@ - (void)handleCapsLockEvent:(NSEvent*)event callback:(FlutterKeyCallbackGuard*)c [self sendCapsLockTapWithTimestamp:event.timestamp callback:callback]; _lastModifierFlagsOfInterest = _lastModifierFlagsOfInterest ^ NSEventModifierFlagCapsLock; } else { + [self sendEmptyEvent]; [callback resolveTo:TRUE]; } } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm index d7b6f93f5cac7..38c7ccdd2a9f2 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponderUnittests.mm @@ -302,9 +302,16 @@ - (void)dealloc { last_handled = handled; }]; - EXPECT_EQ([events count], 0u); + EXPECT_EQ([events count], 1u); + EXPECT_EQ(last_handled, TRUE); + event = [events lastObject].data; + EXPECT_EQ(event->physical, 0ull); + EXPECT_EQ(event->logical, 0ull); + EXPECT_FALSE([[events lastObject] hasCallback]); EXPECT_EQ(last_handled, TRUE); + [events removeAllObjects]; + last_handled = FALSE; [responder handleEvent:keyEvent(NSEventTypeKeyUp, 0x100, @"a", @"a", FALSE, kKeyCodeKeyA) callback:^(BOOL handled) { @@ -327,6 +334,7 @@ - (void)dealloc { TEST(FlutterEmbedderKeyResponderUnittests, IgnoreDuplicateUpEvent) { __block NSMutableArray* events = [[NSMutableArray alloc] init]; + FlutterKeyEvent* event; __block BOOL last_handled = TRUE; FlutterEmbedderKeyResponder* responder = [[FlutterEmbedderKeyResponder alloc] @@ -343,8 +351,15 @@ - (void)dealloc { last_handled = handled; }]; - EXPECT_EQ([events count], 0u); + EXPECT_EQ([events count], 1u); + EXPECT_EQ(last_handled, TRUE); + event = [events lastObject].data; + EXPECT_EQ(event->physical, 0ull); + EXPECT_EQ(event->logical, 0ull); + EXPECT_FALSE([[events lastObject] hasCallback]); EXPECT_EQ(last_handled, TRUE); + + [events removeAllObjects]; } // Press L shift, A, then release L shift then A, on an US keyboard. @@ -1042,6 +1057,7 @@ - (void)dealloc { // Press the CapsLock key when CapsLock state is desynchronized TEST(FlutterEmbedderKeyResponderUnittests, SynchronizeCapsLockStateOnCapsLock) { __block NSMutableArray* events = [[NSMutableArray alloc] init]; + FlutterKeyEvent* event; __block BOOL last_handled = TRUE; id keyEventCallback = ^(BOOL handled) { last_handled = handled; @@ -1056,13 +1072,20 @@ - (void)dealloc { }]; // In: CapsLock down - // Out: + // Out: (empty) last_handled = FALSE; [responder handleEvent:keyEvent(NSEventTypeFlagsChanged, 0x100, @"", @"", FALSE, kKeyCodeCapsLock) callback:keyEventCallback]; - EXPECT_EQ([events count], 0u); + EXPECT_EQ([events count], 1u); + EXPECT_EQ(last_handled, TRUE); + event = [events lastObject].data; + EXPECT_EQ(event->physical, 0ull); + EXPECT_EQ(event->logical, 0ull); + EXPECT_FALSE([[events lastObject] hasCallback]); EXPECT_EQ(last_handled, TRUE); + + [events removeAllObjects]; } // Press the CapsLock key when CapsLock state is desynchronized From f0f806d4c941b1b76c34c74731372625ff72eaa3 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 29 Jul 2021 21:47:38 -0700 Subject: [PATCH 4/8] Format --- .../darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index 263c8d11d3921..640b80ef1d9a6 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -620,7 +620,6 @@ - (void)sendEmptyEvent { _sendEvent(flutterEvent, nullptr, nullptr); } - - (void)handleDownEvent:(NSEvent*)event callback:(FlutterKeyCallbackGuard*)callback { uint64_t physicalKey = GetPhysicalKeyForKeyCode(event.keyCode); uint64_t logicalKey = GetLogicalKeyForEvent(event, physicalKey); From aeb78360bef5585b6c59ce8b73012fd2267a6d62 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 29 Jul 2021 23:52:05 -0700 Subject: [PATCH 5/8] iOS --- .../Source/FlutterEmbedderKeyResponder.mm | 24 +++++++ .../Source/FlutterEmbedderKeyResponderTest.mm | 63 ++++++++++++------- .../Source/FlutterViewControllerTest.mm | 4 +- .../Source/FlutterEmbedderKeyResponder.mm | 4 +- 4 files changed, 70 insertions(+), 25 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponder.mm index 45f76397382b2..ddf21772a7e05 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponder.mm @@ -426,6 +426,15 @@ - (void)synthesizeCapsLockTapWithTimestamp:(NSTimeInterval)timestamp; - (void)sendPrimaryFlutterEvent:(const FlutterKeyEvent&)event callback:(nonnull FlutterKeyCallbackGuard*)callback; +/** + * Send an empty key event. + * + * The event is never synthesized, and never expects an event result. An empty + * event is sent when no other events should be sent, such as upon back-to-back + * keydown events of the same key. + */ +- (void)sendEmptyEvent; + /** * Send a key event for a modifier key. */ @@ -619,6 +628,19 @@ - (void)sendPrimaryFlutterEvent:(const FlutterKeyEvent&)event _sendEvent(event, HandleResponse, pending); } +- (void)sendEmptyEvent { + FlutterKeyEvent event = { + .struct_size = sizeof(FlutterKeyEvent), + .timestamp = 0, + .type = kFlutterKeyEventTypeDown, + .physical = 0, + .logical = 0, + .character = nil, + .synthesized = false, + }; + _sendEvent(event, nil, nil); +} + - (void)synthesizeModifierEventOfType:(BOOL)isDownEvent timestamp:(NSTimeInterval)timestamp keyCode:(UInt32)keyCode { @@ -659,6 +681,7 @@ - (void)handlePressBegin:(nonnull FlutterUIPressProxy*)press // However this might happen in add-to-app scenarios if the focus is changed // from the native view to the Flutter view amid the key tap. [callback resolveTo:TRUE]; + [self sendEmptyEvent]; return; } } @@ -695,6 +718,7 @@ - (void)handlePressEnd:(nonnull FlutterUIPressProxy*)press // However this might happen in add-to-app scenarios if the focus is changed // from the native view to the Flutter view amid the key tap. [callback resolveTo:TRUE]; + [self sendEmptyEvent]; return; } [self updateKey:physicalKey asPressed:0]; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponderTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponderTest.mm index 6d1f45e2babc0..6de6c840486be 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponderTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponderTest.mm @@ -301,9 +301,16 @@ - (void)testIgnoreDuplicateDownEvent API_AVAILABLE(ios(13.4)) { last_handled = handled; }]; - XCTAssertEqual([events count], 0u); + XCTAssertEqual([events count], 1u); + event = [events lastObject].data; + XCTAssertEqual(event->physical, 0ull); + XCTAssertEqual(event->logical, 0ull); + XCTAssertEqual(event->synthesized, false); + XCTAssertFalse([[events lastObject] hasCallback]); XCTAssertEqual(last_handled, TRUE); + [events removeAllObjects]; + last_handled = FALSE; [responder handlePress:keyUpEvent(kKeyCodeKeyA, kModifierFlagNone, 123.0f) callback:^(BOOL handled) { @@ -327,6 +334,7 @@ - (void)testIgnoreDuplicateDownEvent API_AVAILABLE(ios(13.4)) { - (void)testIgnoreAbruptUpEvent API_AVAILABLE(ios(13.4)) { __block NSMutableArray* events = [[NSMutableArray alloc] init]; __block BOOL last_handled = TRUE; + FlutterKeyEvent* event; FlutterEmbedderKeyResponder* responder = [[FlutterEmbedderKeyResponder alloc] initWithSendEvent:^(const FlutterKeyEvent& event, _Nullable FlutterKeyEventCallback callback, @@ -342,8 +350,15 @@ - (void)testIgnoreAbruptUpEvent API_AVAILABLE(ios(13.4)) { last_handled = handled; }]; - XCTAssertEqual([events count], 0u); + XCTAssertEqual([events count], 1u); + event = [events lastObject].data; + XCTAssertEqual(event->physical, 0ull); + XCTAssertEqual(event->logical, 0ull); + XCTAssertEqual(event->synthesized, false); + XCTAssertFalse([[events lastObject] hasCallback]); XCTAssertEqual(last_handled, TRUE); + + [events removeAllObjects]; } // Press R-Shift, A, then release R-Shift then A, on a US keyboard. @@ -433,6 +448,10 @@ - (void)testToggleModifiersDuringKeyTap API_AVAILABLE(ios(13.4)) { - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { __block NSMutableArray* events = [[NSMutableArray alloc] init]; FlutterKeyEvent* event; + __block BOOL last_handled = TRUE; + id keyEventCallback = ^(BOOL handled) { + last_handled = handled; + }; FlutterEmbedderKeyResponder* responder = [[FlutterEmbedderKeyResponder alloc] initWithSendEvent:^(const FlutterKeyEvent& event, _Nullable FlutterKeyEventCallback callback, @@ -448,8 +467,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // Numpad 1 // OS provides: char: "1", code: 0x59, modifiers: 0x200000 [responder handlePress:keyDownEvent(kKeyCodeNumpad1, kModifierFlagNumPadKey, 123.0, "1", "1") - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -465,8 +483,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // Fn Key (sends HID undefined) // OS provides: char: nil, keycode: 0x3, modifiers: 0x0 [responder handlePress:keyDownEvent(kKeyCodeUndefined, kModifierFlagNone, 123.0) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -482,8 +499,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // F1 Down // OS provides: char: UIKeyInputF1, code: 0x3a, modifiers: 0x0 [responder handlePress:keyDownEvent(kKeyCodeF1, kModifierFlagNone, 123.0f, "\\^P", "\\^P") - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -499,8 +515,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // KeyA Down // OS provides: char: "q", code: 0x4, modifiers: 0x0 [responder handlePress:keyDownEvent(kKeyCodeKeyA, kModifierFlagNone, 123.0f, "a", "a") - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -516,8 +531,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // ShiftLeft Down // OS Provides: char: nil, code: 0xe1, modifiers: 0x20000 [responder handlePress:keyDownEvent(kKeyCodeShiftLeft, kModifierFlagShiftAny, 123.0f) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -532,8 +546,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // Numpad 1 Up // OS provides: char: "1", code: 0x59, modifiers: 0x200000 [responder handlePress:keyUpEvent(kKeyCodeNumpad1, kModifierFlagNumPadKey, 123.0f) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 2u); @@ -559,8 +572,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // F1 Up // OS provides: char: UIKeyInputF1, code: 0x3a, modifiers: 0x0 [responder handlePress:keyUpEvent(kKeyCodeF1, kModifierFlagNone, 123.0f) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -576,8 +588,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // Fn Key (sends HID undefined) // OS provides: char: nil, code: 0x3, modifiers: 0x0 [responder handlePress:keyUpEvent(kKeyCodeUndefined, kModifierFlagNone, 123.0) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -592,8 +603,7 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // KeyA Up // OS provides: char: "a", code: 0x4, modifiers: 0x0 [responder handlePress:keyUpEvent(kKeyCodeKeyA, kModifierFlagNone, 123.0f) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; XCTAssertEqual([events count], 1u); event = [events lastObject].data; @@ -609,10 +619,17 @@ - (void)testSpecialModiferFlags API_AVAILABLE(ios(13.4)) { // ShiftLeft Up // OS provides: char: nil, code: 0xe1, modifiers: 0x20000 [responder handlePress:keyUpEvent(kKeyCodeShiftLeft, kModifierFlagShiftAny, 123.0f) - callback:^(BOOL handled){ - }]; + callback:keyEventCallback]; - XCTAssertEqual([events count], 0u); + XCTAssertEqual([events count], 1u); + event = [events lastObject].data; + XCTAssertEqual(event->physical, 0ull); + XCTAssertEqual(event->logical, 0ull); + XCTAssertEqual(event->synthesized, false); + XCTAssertFalse([[events lastObject] hasCallback]); + XCTAssertEqual(last_handled, TRUE); + + [events removeAllObjects]; } - (void)testIdentifyLeftAndRightModifiers API_AVAILABLE(ios(13.4)) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm index 4cac7322b0e67..ecc522e292764 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm @@ -54,7 +54,9 @@ - (void)notifyLowMemory { - (void)sendKeyEvent:(const FlutterKeyEvent&)event callback:(FlutterKeyEventCallback)callback userData:(void*)userData API_AVAILABLE(ios(9.0)) { - NSAssert(callback != nullptr, @"Invalid callback"); + if (callback == nil) + return; + // NSAssert(callback != nullptr, @"Invalid callback"); // Response is async, so we have to post it to the run loop instead of calling // it directly. CFRunLoopPerformBlock(CFRunLoopGetCurrent(), kCFRunLoopDefaultMode, ^() { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm index 640b80ef1d9a6..4e80d4228fe71 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderKeyResponder.mm @@ -426,7 +426,9 @@ - (void)sendModifierEventOfType:(BOOL)isDownEvent /** * Send an empty key event. * - * The event is always synthesized. + * The event is never synthesized, and never expects an event result. An empty + * event is sent when no other events should be sent, such as upon back-to-back + * keydown events of the same key. */ - (void)sendEmptyEvent; From c9febb74943a9ddf968c7f689d0198c24a8fbd72 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 30 Jul 2021 01:54:42 -0700 Subject: [PATCH 6/8] linux and doc --- shell/platform/embedder/embedder.h | 14 +++++++++++ .../linux/fl_key_embedder_responder.cc | 12 ++++++++++ .../linux/fl_key_embedder_responder_test.cc | 24 +++++++++++++++---- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index c630c61b0f205..0a63b5580e680 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -693,6 +693,14 @@ typedef enum { /// released. /// * All events throughout a key press sequence shall have the same `physical` /// and `logical`. Having different `character`s is allowed. +/// +/// A `FlutterKeyEvent` with `physical` 0 and `logical` 0 is an empty event. +/// This is the only case either `physical` or `logical` can be 0. An empty +/// event must be sent if a key message should be converted to no +/// `FlutterKeyEvent`s, for example, when a key down message is received for a +/// key that has already been pressed according to the record. This is to ensure +/// some `FlutterKeyEvent` arrives at the framework before raw key message. +/// See https://github.com/flutter/flutter/issues/87230. typedef struct { /// The size of this struct. Must be sizeof(FlutterKeyEvent). size_t struct_size; @@ -706,11 +714,17 @@ typedef struct { /// /// For the full definition and list of pre-defined physical keys, see /// `PhysicalKeyboardKey` from the framework. + /// + /// The only case that `physical` might be 0 is when this is an empty event. + /// See `FlutterKeyEvent` for introduction. uint64_t physical; /// The key ID for the logical key of this event. /// /// For the full definition and a list of pre-defined logical keys, see /// `LogicalKeyboardKey` from the framework. + /// + /// The only case that `logical` might be 0 is when this is an empty event. + /// See `FlutterKeyEvent` for introduction. uint64_t logical; /// Null-terminated character input from the event. Can be null. Ignored for /// up events. diff --git a/shell/platform/linux/fl_key_embedder_responder.cc b/shell/platform/linux/fl_key_embedder_responder.cc index a464f325dc6e9..5a46b4cd21a95 100644 --- a/shell/platform/linux/fl_key_embedder_responder.cc +++ b/shell/platform/linux/fl_key_embedder_responder.cc @@ -14,6 +14,16 @@ constexpr uint64_t kMicrosecondsPerMillisecond = 1000; +static const FlutterKeyEvent empty_event { + .struct_size = sizeof(FlutterKeyEvent), + .timestamp = 0, + .type = kFlutterKeyEventTypeDown, + .physical = 0, + .logical = 0, + .character = nullptr, + .synthesized = false, +}; + // Look up a hash table that maps a uint64_t to a uint64_t. // // Returns 0 if not found. @@ -712,6 +722,7 @@ static void fl_key_embedder_responder_handle_event( // pressed one, usually indicating multiple keyboards are pressing keys // with the same physical key, or the up event was lost during a loss of // focus. The down event is ignored. + fl_engine_send_key_event(self->engine, &empty_event, nullptr, nullptr); callback(true, user_data); return; } else { @@ -724,6 +735,7 @@ static void fl_key_embedder_responder_handle_event( // The physical key has been released before. It might indicate a missed // event due to loss of focus, or multiple keyboards pressed keys with the // same physical key. Ignore the up event. + fl_engine_send_key_event(self->engine, &empty_event, nullptr, nullptr); callback(true, user_data); return; } else { diff --git a/shell/platform/linux/fl_key_embedder_responder_test.cc b/shell/platform/linux/fl_key_embedder_responder_test.cc index 8aee6a92032f4..2af2d2531dc93 100644 --- a/shell/platform/linux/fl_key_embedder_responder_test.cc +++ b/shell/platform/linux/fl_key_embedder_responder_test.cc @@ -878,14 +878,23 @@ TEST(FlKeyEmbedderResponderTest, IgnoreDuplicateDownEvent) { // Press KeyA again (with different logical key, which is not necessari but // for coverage). - g_expected_handled = true; // The ignored event is always handled. + g_expected_handled = true; // The empty event is always handled. fl_key_responder_handle_event( responder, fl_key_event_new_by_mock(102, kPress, GDK_KEY_q, kKeyCodeKeyA, 0, kIsNotModifier), verify_response_handled, &user_data); - EXPECT_EQ(g_call_records->len, 0u); + EXPECT_EQ(g_call_records->len, 1u); + + record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 0)); + EXPECT_EQ(record->event->physical, 0ull); + EXPECT_EQ(record->event->logical, 0ull); + EXPECT_STREQ(record->event->character, nullptr); + EXPECT_EQ(record->event->synthesized, false); + EXPECT_EQ(record->callback, nullptr); + + g_ptr_array_clear(g_call_records); // Release KeyA fl_key_responder_handle_event( @@ -913,14 +922,21 @@ TEST(FlKeyEmbedderResponderTest, IgnoreAbruptUpEvent) { int user_data = 123; // Arbitrary user data // Release KeyA before it was even pressed. - g_expected_handled = true; // The ignored event is always handled. + g_expected_handled = true; // The empty event is always handled. fl_key_responder_handle_event( responder, fl_key_event_new_by_mock(103, kRelease, GDK_KEY_q, kKeyCodeKeyA, 0, kIsNotModifier), verify_response_handled, &user_data); - EXPECT_EQ(g_call_records->len, 0u); + EXPECT_EQ(g_call_records->len, 1u); + + record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 0)); + EXPECT_EQ(record->event->physical, 0ull); + EXPECT_EQ(record->event->logical, 0ull); + EXPECT_STREQ(record->event->character, nullptr); + EXPECT_EQ(record->event->synthesized, false); + EXPECT_EQ(record->callback, nullptr); clear_g_call_records(); g_object_unref(engine); From 5f2da33568047eb6773069e7d012b82964682c2e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 30 Jul 2021 01:55:56 -0700 Subject: [PATCH 7/8] Format --- .../platform/linux/fl_key_embedder_responder.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/shell/platform/linux/fl_key_embedder_responder.cc b/shell/platform/linux/fl_key_embedder_responder.cc index 5a46b4cd21a95..56c27b6c72d7b 100644 --- a/shell/platform/linux/fl_key_embedder_responder.cc +++ b/shell/platform/linux/fl_key_embedder_responder.cc @@ -14,14 +14,14 @@ constexpr uint64_t kMicrosecondsPerMillisecond = 1000; -static const FlutterKeyEvent empty_event { - .struct_size = sizeof(FlutterKeyEvent), - .timestamp = 0, - .type = kFlutterKeyEventTypeDown, - .physical = 0, - .logical = 0, - .character = nullptr, - .synthesized = false, +static const FlutterKeyEvent empty_event{ + .struct_size = sizeof(FlutterKeyEvent), + .timestamp = 0, + .type = kFlutterKeyEventTypeDown, + .physical = 0, + .logical = 0, + .character = nullptr, + .synthesized = false, }; // Look up a hash table that maps a uint64_t to a uint64_t. From ea3bed2c0b5f7f289cc1bba6e1fd581cb50bfb9a Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 30 Jul 2021 02:02:12 -0700 Subject: [PATCH 8/8] Fix --- .../linux/fl_key_embedder_responder_test.cc | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/shell/platform/linux/fl_key_embedder_responder_test.cc b/shell/platform/linux/fl_key_embedder_responder_test.cc index 2af2d2531dc93..71743d529f55b 100644 --- a/shell/platform/linux/fl_key_embedder_responder_test.cc +++ b/shell/platform/linux/fl_key_embedder_responder_test.cc @@ -914,6 +914,8 @@ TEST(FlKeyEmbedderResponderTest, IgnoreDuplicateDownEvent) { } TEST(FlKeyEmbedderResponderTest, IgnoreAbruptUpEvent) { + FlKeyEmbedderCallRecord* record; + EXPECT_EQ(g_call_records, nullptr); g_call_records = g_ptr_array_new_with_free_func(g_object_unref); FlEngine* engine = make_mock_engine_with_records(); @@ -1055,11 +1057,11 @@ TEST(FlKeyEmbedderResponderTest, SynthesizeForDesyncPressingStateOnSelfEvents) { kKeyCodeControlRight, state, kIsModifier), verify_response_handled, &user_data); - // A ControlLeft down is synthesized, with no non-synthesized event. + // A ControlLeft down is synthesized, with an empty event. // Reason: The ControlLeft down is synthesized to synchronize the state // showing Control as pressed. The ControlRight event is ignored because // the event is considered a duplicate up event. - EXPECT_EQ(g_call_records->len, 1u); + EXPECT_EQ(g_call_records->len, 2u); record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 0)); EXPECT_EQ(record->event->timestamp, 105000); EXPECT_EQ(record->event->type, kFlutterKeyEventTypeDown); @@ -1068,6 +1070,13 @@ TEST(FlKeyEmbedderResponderTest, SynthesizeForDesyncPressingStateOnSelfEvents) { EXPECT_STREQ(record->event->character, nullptr); EXPECT_EQ(record->event->synthesized, true); + record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 1)); + EXPECT_EQ(record->event->physical, 0ull); + EXPECT_EQ(record->event->logical, 0ull); + EXPECT_STREQ(record->event->character, nullptr); + EXPECT_EQ(record->event->synthesized, false); + EXPECT_EQ(record->callback, nullptr); + g_ptr_array_clear(g_call_records); clear_g_call_records(); @@ -1368,7 +1377,7 @@ TEST(FlKeyEmbedderResponderTest, SynthesizeForDesyncLockModeOnNonSelfEvents) { g_ptr_array_clear(g_call_records); // Release NumLock. Since the previous event should have synthesized NumLock - // to be released, this should result in no events. + // to be released, this should result in only an empty event. g_expected_handled = true; fl_key_responder_handle_event( responder, @@ -1376,7 +1385,13 @@ TEST(FlKeyEmbedderResponderTest, SynthesizeForDesyncLockModeOnNonSelfEvents) { state, kIsModifier), verify_response_handled, &user_data); - EXPECT_EQ(g_call_records->len, 0u); + EXPECT_EQ(g_call_records->len, 1u); + record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 0)); + EXPECT_EQ(record->event->physical, 0ull); + EXPECT_EQ(record->event->logical, 0ull); + EXPECT_STREQ(record->event->character, nullptr); + EXPECT_EQ(record->event->synthesized, false); + EXPECT_EQ(record->callback, nullptr); clear_g_call_records(); g_object_unref(engine); @@ -1507,7 +1522,7 @@ TEST(FlKeyEmbedderResponderTest, SynthesizationOccursOnIgnoredEvents) { kIsNotModifier), verify_response_handled, &user_data); - EXPECT_EQ(g_call_records->len, 2u); + EXPECT_EQ(g_call_records->len, 3u); record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 0)); EXPECT_EQ(record->event->timestamp, 101000); EXPECT_EQ(record->event->type, kFlutterKeyEventTypeDown); @@ -1524,6 +1539,13 @@ TEST(FlKeyEmbedderResponderTest, SynthesizationOccursOnIgnoredEvents) { EXPECT_STREQ(record->event->character, nullptr); EXPECT_EQ(record->event->synthesized, true); + record = FL_KEY_EMBEDDER_CALL_RECORD(g_ptr_array_index(g_call_records, 2)); + EXPECT_EQ(record->event->physical, 0ull); + EXPECT_EQ(record->event->logical, 0ull); + EXPECT_STREQ(record->event->character, nullptr); + EXPECT_EQ(record->event->synthesized, false); + EXPECT_EQ(record->callback, nullptr); + g_ptr_array_clear(g_call_records); clear_g_call_records();