From 8a6bfbf613e3be707351948e9ef6a7006aea39e3 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 1 Jun 2022 05:28:17 -0700 Subject: [PATCH 1/7] Impl and test --- .../windows/keyboard_key_embedder_handler.cc | 18 ++++++++--- .../windows/keyboard_key_embedder_handler.h | 5 ++- .../windows/keyboard_win32_unittests.cc | 32 +++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index 47ac20ecb1d22..a2bc1266b1782 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -250,7 +250,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( // After this function, all critical keys except for the target key will have // their toggled state and pressed state matched with their true states. The // target key's pressed state will be updated immediately after this. - SynchronizeCritialPressedStates(key, type != kFlutterKeyEventTypeRepeat); + SynchronizeCritialPressedStates(key, physical_key, type); if (eventual_logical_record != 0) { pressingRecords_[physical_key] = eventual_logical_record; @@ -380,7 +380,10 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( int this_virtual_key, - bool pressed_state_will_change) { + int this_physical_key, + FlutterKeyEventType this_event_type) { + auto this_key_pressed_iter = pressingRecords_.find(this_physical_key); + bool this_key_pressed = this_key_pressed_iter != pressingRecords_.end(); for (auto& kv : critical_keys_) { UINT virtual_key = kv.first; CriticalKey& key_info = kv.second; @@ -394,8 +397,15 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( auto recorded_pressed_iter = pressingRecords_.find(key_info.physical_key); bool recorded_pressed = recorded_pressed_iter != pressingRecords_.end(); bool should_pressed = state & kStateMaskPressed; - if (virtual_key == this_virtual_key && pressed_state_will_change) { - should_pressed = !should_pressed; + bool pressed_state_will_change = false; + if (this_event_type == kFlutterKeyEventTypeDown) { + if (virtual_key == this_virtual_key) { + should_pressed = !should_pressed; + } + } else if (this_event_type == kFlutterKeyEventTypeUp) { + if (this_physical_key == key_info.physical_key) { + should_pressed = !should_pressed; + } } if (recorded_pressed != should_pressed) { if (recorded_pressed) { diff --git a/shell/platform/windows/keyboard_key_embedder_handler.h b/shell/platform/windows/keyboard_key_embedder_handler.h index 7add4247ac774..22a35ad5d51ef 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/shell/platform/windows/keyboard_key_embedder_handler.h @@ -117,7 +117,10 @@ class KeyboardKeyEmbedderHandler void SynchronizeCritialToggledStates(int virtual_key, bool is_down); // Check each key's state from |get_key_state_| and synthesize events // if their pressing states have been desynchronized. - void SynchronizeCritialPressedStates(int virtual_key, bool was_down); + void SynchronizeCritialPressedStates( + int this_virtual_key, + int this_physical_key, + FlutterKeyEventType this_event_type); // Wraps perform_send_event_ with state tracking. Use this instead of // |perform_send_event_| to send events to the framework. diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index b90f274285f6c..5854f9ec07c95 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -46,6 +46,7 @@ constexpr uint64_t kScanCodeKeyO = 0x18; constexpr uint64_t kScanCodeKeyQ = 0x10; constexpr uint64_t kScanCodeKeyW = 0x11; constexpr uint64_t kScanCodeDigit1 = 0x02; +constexpr uint64_t kScanCodeDigit2 = 0x03; constexpr uint64_t kScanCodeDigit6 = 0x07; // constexpr uint64_t kScanCodeNumpad1 = 0x4f; // constexpr uint64_t kScanCodeNumLock = 0x45; @@ -1915,6 +1916,37 @@ TEST(KeyboardTest, ImeExtendedEventsAreIgnored) { EXPECT_EQ(tester.RedispatchedMessageCountAndClear(), 0); } +TEST(KeyboardTest, UpOnlyImeEventsAreCorrectlyHandled) { + KeyboardTester tester; + tester.Responding(true); + + // US Keyboard layout. + + // Press CtrlRight in IME mode. + tester.InjectKeyboardChanges(std::vector{ + KeyStateChange{VK_LSHIFT, true, false}, + WmKeyDownInfo{VK_SHIFT, kScanCodeShiftLeft, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmKeyDownInfo{VK_PROCESSKEY, kScanCodeDigit2, kNotExtended, kWasUp}.Build( + kWmResultZero), + KeyStateChange{VK_LSHIFT, false, true}, + WmKeyUpInfo{VK_PROCESSKEY, kScanCodeShiftLeft, kNotExtended}.Build( + kWmResultZero), + WmKeyUpInfo{'2', kScanCodeDigit2, kNotExtended, kWasUp}.Build( + kWmResultZero)}); + + EXPECT_EQ(key_calls.size(), 4); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalShiftLeft, kLogicalShiftLeft, "", + kNotSynthesized); + EXPECT_CALL_IS_EVENT(key_calls[1], kFlutterKeyEventTypeDown, 0, 0, "", + kNotSynthesized); + EXPECT_CALL_IS_EVENT(key_calls[2], kFlutterKeyEventTypeUp, kPhysicalShiftLeft, kLogicalShiftLeft, "", + kNotSynthesized); + EXPECT_CALL_IS_EVENT(key_calls[3], kFlutterKeyEventTypeDown, 0, 0, "", + kNotSynthesized); + clear_key_calls(); +} + TEST(KeyboardTest, DisorderlyRespondedEvents) { KeyboardTester tester; From a9b2825e587a11fd2149f7bfa1e265bc8a4a7c28 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 1 Jun 2022 18:12:57 -0700 Subject: [PATCH 2/7] Format and some doc --- .../windows/keyboard_key_embedder_handler.h | 7 +++---- shell/platform/windows/keyboard_win32_unittests.cc | 13 ++++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.h b/shell/platform/windows/keyboard_key_embedder_handler.h index 22a35ad5d51ef..9d66a05bde573 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/shell/platform/windows/keyboard_key_embedder_handler.h @@ -117,10 +117,9 @@ class KeyboardKeyEmbedderHandler void SynchronizeCritialToggledStates(int virtual_key, bool is_down); // Check each key's state from |get_key_state_| and synthesize events // if their pressing states have been desynchronized. - void SynchronizeCritialPressedStates( - int this_virtual_key, - int this_physical_key, - FlutterKeyEventType this_event_type); + void SynchronizeCritialPressedStates(int this_virtual_key, + int this_physical_key, + FlutterKeyEventType this_event_type); // Wraps perform_send_event_ with state tracking. Use this instead of // |perform_send_event_| to send events to the framework. diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index 5854f9ec07c95..f1b395cf797ac 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -1916,6 +1916,12 @@ TEST(KeyboardTest, ImeExtendedEventsAreIgnored) { EXPECT_EQ(tester.RedispatchedMessageCountAndClear(), 0); } +// Ensures that synthesization works correctly when a Shift key is pressed and +// (only) its up event is labeled as an IME event (VK_PROCESSKEY). +// +// Regression test for https://github.com/flutter/flutter/issues/104169. These +// are real messages recorded when pressing Shift-2 using Microsoft Pinyin IME +// on Win 10 Enterprise, which crashed the app before the fix. TEST(KeyboardTest, UpOnlyImeEventsAreCorrectlyHandled) { KeyboardTester tester; tester.Responding(true); @@ -1936,12 +1942,13 @@ TEST(KeyboardTest, UpOnlyImeEventsAreCorrectlyHandled) { kWmResultZero)}); EXPECT_EQ(key_calls.size(), 4); - EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalShiftLeft, kLogicalShiftLeft, "", + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, + kPhysicalShiftLeft, kLogicalShiftLeft, "", kNotSynthesized); EXPECT_CALL_IS_EVENT(key_calls[1], kFlutterKeyEventTypeDown, 0, 0, "", kNotSynthesized); - EXPECT_CALL_IS_EVENT(key_calls[2], kFlutterKeyEventTypeUp, kPhysicalShiftLeft, kLogicalShiftLeft, "", - kNotSynthesized); + EXPECT_CALL_IS_EVENT(key_calls[2], kFlutterKeyEventTypeUp, kPhysicalShiftLeft, + kLogicalShiftLeft, "", kNotSynthesized); EXPECT_CALL_IS_EVENT(key_calls[3], kFlutterKeyEventTypeDown, 0, 0, "", kNotSynthesized); clear_key_calls(); From c1fd021f6ba3e2488457be26d8b30f4db210d887 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 3 Jun 2022 20:50:07 -0700 Subject: [PATCH 3/7] Rewrite algorithm --- .../windows/keyboard_key_embedder_handler.cc | 180 +++++++++++------- .../windows/keyboard_key_embedder_handler.h | 10 +- 2 files changed, 112 insertions(+), 78 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index a2bc1266b1782..611a5e6c488f5 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -162,23 +162,57 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( const uint64_t logical_key = GetLogicalKey(key, extended, scancode); assert(action == WM_KEYDOWN || action == WM_KEYUP || action == WM_SYSKEYDOWN || action == WM_SYSKEYUP); - const bool is_physical_down = action == WM_KEYDOWN || action == WM_SYSKEYDOWN; auto last_logical_record_iter = pressingRecords_.find(physical_key); const bool had_record = last_logical_record_iter != pressingRecords_.end(); const uint64_t last_logical_record = had_record ? last_logical_record_iter->second : 0; + // The logical key for the current "tap sequence". + // + // Key events are formed in tap sequences: down, repeats, up. The logical key + // stays consistent throughout a tap sequence, which is this value. + uint64_t sequence_logical_key = had_record ? last_logical_record : logical_key; + + if (sequence_logical_key == VK_PROCESSKEY) { + // VK_PROCESSKEY means that the key press is used by an IME. These key + // 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. + callback(true); + return; + } + + const bool is_physical_down = action == WM_KEYDOWN || action == WM_SYSKEYDOWN; + + UpdateLastSeenCritialKey(key, physical_key, sequence_logical_key); + // Synchronize the toggled states of critical keys (such as whether CapsLocks + // is enabled). Toggled states can only be changed upon a down event, so if + // the recorded toggled state does not match the true state, this function + // will synthesize (an up event if the key is recorded pressed, then) a down + // event. + // + // After this function, all critical keys will have their toggled state + // updated to the true state, while the critical keys whose toggled state have + // been changed will be pressed regardless of their true pressed state. + // Updating the pressed state will be done by SynchronizeCritialPressedStates. + SynchronizeCritialToggledStates(key, is_physical_down); + // Synchronize the pressed states of critical keys (such as whether CapsLocks + // is pressed). + // + // After this function, all critical keys except for the target key will have + // their toggled state and pressed state matched with their true states. The + // target key's pressed state will be updated immediately after this. + SynchronizeCritialPressedStates(key, physical_key, is_physical_down); + // The resulting event's `type`. FlutterKeyEventType type; - // The resulting event's `logical_key`. - uint64_t result_logical_key; + character = UndeadChar(character); + char character_bytes[kCharacterCacheSize]; // What pressingRecords_[physical_key] should be after the KeyboardHookImpl // returns (0 if the entry should be removed). uint64_t eventual_logical_record; - char character_bytes[kCharacterCacheSize]; - - character = UndeadChar(character); + uint64_t result_logical_key; if (is_physical_down) { if (had_record) { @@ -223,35 +257,6 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( } } - if (result_logical_key == VK_PROCESSKEY) { - // VK_PROCESSKEY means that the key press is used by an IME. These key - // 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. - callback(true); - return; - } - - UpdateLastSeenCritialKey(key, physical_key, result_logical_key); - // Synchronize the toggled states of critical keys (such as whether CapsLocks - // is enabled). Toggled states can only be changed upon a down event, so if - // the recorded toggled state does not match the true state, this function - // will synthesize (an up event if the key is recorded pressed, then) a down - // event. - // - // After this function, all critical keys will have their toggled state - // updated to the true state, while the critical keys whose toggled state have - // been changed will be pressed regardless of their true pressed state. - // Updating the pressed state will be done by SynchronizeCritialPressedStates. - SynchronizeCritialToggledStates(key, type == kFlutterKeyEventTypeDown); - // Synchronize the pressed states of critical keys (such as whether CapsLocks - // is pressed). - // - // After this function, all critical keys except for the target key will have - // their toggled state and pressed state matched with their true states. The - // target key's pressed state will be updated immediately after this. - SynchronizeCritialPressedStates(key, physical_key, type); - if (eventual_logical_record != 0) { pressingRecords_[physical_key] = eventual_logical_record; } else { @@ -333,57 +338,73 @@ void KeyboardKeyEmbedderHandler::UpdateLastSeenCritialKey( } void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( - int this_virtual_key, - bool is_down_event) { - for (auto& kv : critical_keys_) { - UINT virtual_key = kv.first; - CriticalKey& key_info = kv.second; - if (key_info.physical_key == 0) { + int event_virtual_key, + bool event_is_down) { + // NowState ----------------> PreEventState --------------> TrueState + // Synchronization Event + for (auto& target : critical_keys_) { + UINT target_virtual_key = target.first; + CriticalKey& target_info = target.second; + if (target_info.physical_key == 0) { // Never seen this key. continue; } - assert(key_info.logical_key != 0); + assert(target_info.logical_key != 0); // Check toggling state first, because it might alter pressing state. - if (key_info.check_toggled) { + if (target_info.check_toggled) { + const bool target_is_pressed = + pressingRecords_.find(target_info.physical_key) != + pressingRecords_.end(); // The togglable keys observe a 4-phase cycle: // // Phase# 0 1 2 3 // Event Down Up Down Up // Pressed 0 1 0 1 // Toggled 0 1 1 0 - SHORT state = get_key_state_(virtual_key); - bool should_toggled = state & kStateMaskToggled; - if (virtual_key == this_virtual_key && is_down_event) { - key_info.toggled_on = !key_info.toggled_on; + const bool true_toggled = get_key_state_(target_virtual_key) & kStateMaskToggled; + bool pre_event_toggled = true_toggled; + // Check if the main event's key is the key being checked. If it's the + // non-repeat down event, toggle the state. + if (target_virtual_key == event_virtual_key && !target_is_pressed && + event_is_down) { + pre_event_toggled = !pre_event_toggled; } - if (key_info.toggled_on != should_toggled) { + if (target_info.toggled_on != pre_event_toggled) { // If the key is pressed, release it first. - if (pressingRecords_.find(key_info.physical_key) != - pressingRecords_.end()) { + if (target_is_pressed) { SendEvent(SynthesizeSimpleEvent( - kFlutterKeyEventTypeUp, key_info.physical_key, - key_info.logical_key, empty_character), + kFlutterKeyEventTypeUp, target_info.physical_key, + target_info.logical_key, empty_character), nullptr, nullptr); } // Synchronizing toggle state always ends with the key being pressed. - pressingRecords_[key_info.physical_key] = key_info.logical_key; + pressingRecords_[target_info.physical_key] = target_info.logical_key; SendEvent(SynthesizeSimpleEvent(kFlutterKeyEventTypeDown, - key_info.physical_key, - key_info.logical_key, empty_character), + target_info.physical_key, + target_info.logical_key, empty_character), nullptr, nullptr); } - key_info.toggled_on = should_toggled; + target_info.toggled_on = true_toggled; } } } void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( - int this_virtual_key, + int event_virtual_key, int this_physical_key, - FlutterKeyEventType this_event_type) { - auto this_key_pressed_iter = pressingRecords_.find(this_physical_key); - bool this_key_pressed = this_key_pressed_iter != pressingRecords_.end(); + bool is_physical_down) { + // During an incoming event, there might be a synthesized Flutter event for + // each key of each pressing goal, followed by an eventual main Flutter + // event. + // + // NowState ----------------> PreEventState --------------> TrueState + // Synchronization Event + // + // The goal of the synchronization algorithm is to derive a pre-event state + // that can satisfy the true state (`true_pressed`) after the event, and that + // requires as few synthesized events based on the current state + // (`now_pressed`) as possible. for (auto& kv : critical_keys_) { UINT virtual_key = kv.first; CriticalKey& key_info = kv.second; @@ -393,29 +414,40 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( } assert(key_info.logical_key != 0); if (key_info.check_pressed) { - SHORT state = get_key_state_(virtual_key); - auto recorded_pressed_iter = pressingRecords_.find(key_info.physical_key); - bool recorded_pressed = recorded_pressed_iter != pressingRecords_.end(); - bool should_pressed = state & kStateMaskPressed; - bool pressed_state_will_change = false; - if (this_event_type == kFlutterKeyEventTypeDown) { - if (virtual_key == this_virtual_key) { - should_pressed = !should_pressed; + SHORT true_pressed = get_key_state_(virtual_key) & kStateMaskPressed; + auto pressing_record_iter = pressingRecords_.find(key_info.physical_key); + bool now_pressed = pressing_record_iter != pressingRecords_.end(); + bool pre_event_pressed = true_pressed; + // Check if the main event is the key being checked to get the correct + // target state. + if (is_physical_down) { + // For down events, this key is the event key if they have the same + // virtual key, because virtual key represents "functionality." + if (virtual_key == event_virtual_key) { + pre_event_pressed = false; } - } else if (this_event_type == kFlutterKeyEventTypeUp) { + } else { + // For up events, this key is the event key if they have the same + // physical key, because it is necessary to ensure that the physical + // key is correctly released. + // + // In that case, although the previous state should be pressed, don't + // synthesize a down event even if it's not. The later code will handle + // such cases by skipping abrupt up events. Obviously don't synthesize + // up events either. if (this_physical_key == key_info.physical_key) { - should_pressed = !should_pressed; + continue; } } - if (recorded_pressed != should_pressed) { - if (recorded_pressed) { - pressingRecords_.erase(recorded_pressed_iter); + if (now_pressed != pre_event_pressed) { + if (now_pressed) { + pressingRecords_.erase(pressing_record_iter); } else { pressingRecords_[key_info.physical_key] = key_info.logical_key; } const char* empty_character = ""; SendEvent( - SynthesizeSimpleEvent(recorded_pressed ? kFlutterKeyEventTypeUp + SynthesizeSimpleEvent(now_pressed ? kFlutterKeyEventTypeUp : kFlutterKeyEventTypeDown, key_info.physical_key, key_info.logical_key, empty_character), diff --git a/shell/platform/windows/keyboard_key_embedder_handler.h b/shell/platform/windows/keyboard_key_embedder_handler.h index 9d66a05bde573..4b56a00ab3f3a 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/shell/platform/windows/keyboard_key_embedder_handler.h @@ -114,12 +114,14 @@ class KeyboardKeyEmbedderHandler uint64_t logical_key); // Check each key's state from |get_key_state_| and synthesize events // if their toggling states have been desynchronized. - void SynchronizeCritialToggledStates(int virtual_key, bool is_down); + void SynchronizeCritialToggledStates(int this_virtual_key, + bool is_physical_down); // Check each key's state from |get_key_state_| and synthesize events // if their pressing states have been desynchronized. - void SynchronizeCritialPressedStates(int this_virtual_key, - int this_physical_key, - FlutterKeyEventType this_event_type); + void SynchronizeCritialPressedStates( + int this_virtual_key, + int this_physical_key, + bool is_physical_down); // Wraps perform_send_event_ with state tracking. Use this instead of // |perform_send_event_| to send events to the framework. From 7540f9821a57567485d3183d35d112f72827ab66 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 3 Jun 2022 20:59:00 -0700 Subject: [PATCH 4/7] Format --- .../windows/keyboard_key_embedder_handler.cc | 19 ++++++++++--------- .../windows/keyboard_key_embedder_handler.h | 7 +++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index 611a5e6c488f5..d7fbe6f422c30 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -172,7 +172,8 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( // // Key events are formed in tap sequences: down, repeats, up. The logical key // stays consistent throughout a tap sequence, which is this value. - uint64_t sequence_logical_key = had_record ? last_logical_record : logical_key; + uint64_t sequence_logical_key = + had_record ? last_logical_record : logical_key; if (sequence_logical_key == VK_PROCESSKEY) { // VK_PROCESSKEY means that the key press is used by an IME. These key @@ -362,7 +363,8 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( // Event Down Up Down Up // Pressed 0 1 0 1 // Toggled 0 1 1 0 - const bool true_toggled = get_key_state_(target_virtual_key) & kStateMaskToggled; + const bool true_toggled = + get_key_state_(target_virtual_key) & kStateMaskToggled; bool pre_event_toggled = true_toggled; // Check if the main event's key is the key being checked. If it's the // non-repeat down event, toggle the state. @@ -380,9 +382,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( } // Synchronizing toggle state always ends with the key being pressed. pressingRecords_[target_info.physical_key] = target_info.logical_key; - SendEvent(SynthesizeSimpleEvent(kFlutterKeyEventTypeDown, - target_info.physical_key, - target_info.logical_key, empty_character), + SendEvent(SynthesizeSimpleEvent( + kFlutterKeyEventTypeDown, target_info.physical_key, + target_info.logical_key, empty_character), nullptr, nullptr); } target_info.toggled_on = true_toggled; @@ -447,10 +449,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( } const char* empty_character = ""; SendEvent( - SynthesizeSimpleEvent(now_pressed ? kFlutterKeyEventTypeUp - : kFlutterKeyEventTypeDown, - key_info.physical_key, key_info.logical_key, - empty_character), + SynthesizeSimpleEvent( + now_pressed ? kFlutterKeyEventTypeUp : kFlutterKeyEventTypeDown, + key_info.physical_key, key_info.logical_key, empty_character), nullptr, nullptr); } } diff --git a/shell/platform/windows/keyboard_key_embedder_handler.h b/shell/platform/windows/keyboard_key_embedder_handler.h index 4b56a00ab3f3a..dbde3a7b7710a 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/shell/platform/windows/keyboard_key_embedder_handler.h @@ -118,10 +118,9 @@ class KeyboardKeyEmbedderHandler bool is_physical_down); // Check each key's state from |get_key_state_| and synthesize events // if their pressing states have been desynchronized. - void SynchronizeCritialPressedStates( - int this_virtual_key, - int this_physical_key, - bool is_physical_down); + void SynchronizeCritialPressedStates(int this_virtual_key, + int this_physical_key, + bool is_physical_down); // Wraps perform_send_event_ with state tracking. Use this instead of // |perform_send_event_| to send events to the framework. From 2cbd7ec804a3cd7192d3a09386e51688567b5273 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 3 Jun 2022 21:40:34 -0700 Subject: [PATCH 5/7] Simplify --- .../windows/keyboard_key_embedder_handler.cc | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index d7fbe6f422c30..e82efa870f555 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -343,19 +343,19 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( bool event_is_down) { // NowState ----------------> PreEventState --------------> TrueState // Synchronization Event - for (auto& target : critical_keys_) { - UINT target_virtual_key = target.first; - CriticalKey& target_info = target.second; - if (target_info.physical_key == 0) { + for (auto& kv : critical_keys_) { + UINT virtual_key = kv.first; + CriticalKey& key_info = kv.second; + if (key_info.physical_key == 0) { // Never seen this key. continue; } - assert(target_info.logical_key != 0); + assert(key_info.logical_key != 0); // Check toggling state first, because it might alter pressing state. - if (target_info.check_toggled) { + if (key_info.check_toggled) { const bool target_is_pressed = - pressingRecords_.find(target_info.physical_key) != + pressingRecords_.find(key_info.physical_key) != pressingRecords_.end(); // The togglable keys observe a 4-phase cycle: // @@ -364,30 +364,30 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( // Pressed 0 1 0 1 // Toggled 0 1 1 0 const bool true_toggled = - get_key_state_(target_virtual_key) & kStateMaskToggled; + get_key_state_(virtual_key) & kStateMaskToggled; bool pre_event_toggled = true_toggled; // Check if the main event's key is the key being checked. If it's the // non-repeat down event, toggle the state. - if (target_virtual_key == event_virtual_key && !target_is_pressed && + if (virtual_key == event_virtual_key && !target_is_pressed && event_is_down) { pre_event_toggled = !pre_event_toggled; } - if (target_info.toggled_on != pre_event_toggled) { + if (key_info.toggled_on != pre_event_toggled) { // If the key is pressed, release it first. if (target_is_pressed) { SendEvent(SynthesizeSimpleEvent( - kFlutterKeyEventTypeUp, target_info.physical_key, - target_info.logical_key, empty_character), + kFlutterKeyEventTypeUp, key_info.physical_key, + key_info.logical_key, empty_character), nullptr, nullptr); } // Synchronizing toggle state always ends with the key being pressed. - pressingRecords_[target_info.physical_key] = target_info.logical_key; + pressingRecords_[key_info.physical_key] = key_info.logical_key; SendEvent(SynthesizeSimpleEvent( - kFlutterKeyEventTypeDown, target_info.physical_key, - target_info.logical_key, empty_character), + kFlutterKeyEventTypeDown, key_info.physical_key, + key_info.logical_key, empty_character), nullptr, nullptr); } - target_info.toggled_on = true_toggled; + key_info.toggled_on = true_toggled; } } } From 9e90b2013532c98056aa2f6ce8f5eafbdb198f9a Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 3 Jun 2022 21:41:50 -0700 Subject: [PATCH 6/7] Fix --- shell/platform/windows/keyboard_key_embedder_handler.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index e82efa870f555..84d8fa7766572 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -363,8 +363,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( // Event Down Up Down Up // Pressed 0 1 0 1 // Toggled 0 1 1 0 - const bool true_toggled = - get_key_state_(virtual_key) & kStateMaskToggled; + const bool true_toggled = get_key_state_(virtual_key) & kStateMaskToggled; bool pre_event_toggled = true_toggled; // Check if the main event's key is the key being checked. If it's the // non-repeat down event, toggle the state. @@ -382,9 +381,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( } // Synchronizing toggle state always ends with the key being pressed. pressingRecords_[key_info.physical_key] = key_info.logical_key; - SendEvent(SynthesizeSimpleEvent( - kFlutterKeyEventTypeDown, key_info.physical_key, - key_info.logical_key, empty_character), + SendEvent(SynthesizeSimpleEvent(kFlutterKeyEventTypeDown, + key_info.physical_key, + key_info.logical_key, empty_character), nullptr, nullptr); } key_info.toggled_on = true_toggled; From 13f406c6da367fb98a9d4f5f3dbc7425ac22d8f9 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 15 Jun 2022 17:42:24 -0700 Subject: [PATCH 7/7] Fix params --- .../windows/keyboard_key_embedder_handler.cc | 20 +++++++++---------- .../windows/keyboard_key_embedder_handler.h | 10 +++++----- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index 84d8fa7766572..762051e723884 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -184,7 +184,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( return; } - const bool is_physical_down = action == WM_KEYDOWN || action == WM_SYSKEYDOWN; + const bool is_event_down = action == WM_KEYDOWN || action == WM_SYSKEYDOWN; UpdateLastSeenCritialKey(key, physical_key, sequence_logical_key); // Synchronize the toggled states of critical keys (such as whether CapsLocks @@ -197,14 +197,14 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( // updated to the true state, while the critical keys whose toggled state have // been changed will be pressed regardless of their true pressed state. // Updating the pressed state will be done by SynchronizeCritialPressedStates. - SynchronizeCritialToggledStates(key, is_physical_down); + SynchronizeCritialToggledStates(key, is_event_down); // Synchronize the pressed states of critical keys (such as whether CapsLocks // is pressed). // // After this function, all critical keys except for the target key will have // their toggled state and pressed state matched with their true states. The // target key's pressed state will be updated immediately after this. - SynchronizeCritialPressedStates(key, physical_key, is_physical_down); + SynchronizeCritialPressedStates(key, physical_key, is_event_down); // The resulting event's `type`. FlutterKeyEventType type; @@ -215,7 +215,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( uint64_t eventual_logical_record; uint64_t result_logical_key; - if (is_physical_down) { + if (is_event_down) { if (had_record) { if (was_down) { // A normal repeated key. @@ -340,7 +340,7 @@ void KeyboardKeyEmbedderHandler::UpdateLastSeenCritialKey( void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( int event_virtual_key, - bool event_is_down) { + bool is_event_down) { // NowState ----------------> PreEventState --------------> TrueState // Synchronization Event for (auto& kv : critical_keys_) { @@ -368,7 +368,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( // Check if the main event's key is the key being checked. If it's the // non-repeat down event, toggle the state. if (virtual_key == event_virtual_key && !target_is_pressed && - event_is_down) { + is_event_down) { pre_event_toggled = !pre_event_toggled; } if (key_info.toggled_on != pre_event_toggled) { @@ -393,8 +393,8 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( int event_virtual_key, - int this_physical_key, - bool is_physical_down) { + int event_physical_key, + bool is_event_down) { // During an incoming event, there might be a synthesized Flutter event for // each key of each pressing goal, followed by an eventual main Flutter // event. @@ -421,7 +421,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( bool pre_event_pressed = true_pressed; // Check if the main event is the key being checked to get the correct // target state. - if (is_physical_down) { + if (is_event_down) { // For down events, this key is the event key if they have the same // virtual key, because virtual key represents "functionality." if (virtual_key == event_virtual_key) { @@ -436,7 +436,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( // synthesize a down event even if it's not. The later code will handle // such cases by skipping abrupt up events. Obviously don't synthesize // up events either. - if (this_physical_key == key_info.physical_key) { + if (event_physical_key == key_info.physical_key) { continue; } } diff --git a/shell/platform/windows/keyboard_key_embedder_handler.h b/shell/platform/windows/keyboard_key_embedder_handler.h index dbde3a7b7710a..426a38d42deab 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/shell/platform/windows/keyboard_key_embedder_handler.h @@ -114,13 +114,13 @@ class KeyboardKeyEmbedderHandler uint64_t logical_key); // Check each key's state from |get_key_state_| and synthesize events // if their toggling states have been desynchronized. - void SynchronizeCritialToggledStates(int this_virtual_key, - bool is_physical_down); + void SynchronizeCritialToggledStates(int event_virtual_key, + bool is_event_down); // Check each key's state from |get_key_state_| and synthesize events // if their pressing states have been desynchronized. - void SynchronizeCritialPressedStates(int this_virtual_key, - int this_physical_key, - bool is_physical_down); + void SynchronizeCritialPressedStates(int event_virtual_key, + int event_physical_key, + bool is_event_down); // Wraps perform_send_event_ with state tracking. Use this instead of // |perform_send_event_| to send events to the framework.