diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc index 56e97ba0cf..b2e64954a2 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc @@ -204,7 +204,7 @@ void KeyboardKeyChannelHandler::KeyboardHook( size_t reply_size) { auto decoded = flutter::JsonMessageCodec::GetInstance().DecodeMessage( reply, reply_size); - bool handled = (*decoded)[kHandledKey].GetBool(); + bool handled = decoded ? (*decoded)[kHandledKey].GetBool() : false; callback(handled); }); } diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler_unittests.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler_unittests.cc index 1359d524c2..e4063d2abf 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler_unittests.cc @@ -110,19 +110,16 @@ TEST(KeyboardKeyChannelHandlerTest, ExtendedKeysAreSentToRedispatch) { } TEST(KeyboardKeyChannelHandlerTest, DeadKeysDoNotCrash) { - auto handled_message = CreateResponse(true); - auto unhandled_message = CreateResponse(false); - int received_scancode = 0; - + bool received = false; TestBinaryMessenger messenger( - [&received_scancode, &handled_message, &unhandled_message]( - const std::string& channel, const uint8_t* message, - size_t message_size, BinaryReply reply) { + [&received](const std::string& channel, const uint8_t* message, + size_t message_size, BinaryReply reply) { if (channel == "flutter/keyevent") { auto message_doc = JsonMessageCodec::GetInstance().DecodeMessage( message, message_size); uint32_t character = (*message_doc)[kCharacterCodePointKey].GetUint(); EXPECT_EQ(character, (uint32_t)'^'); + received = true; } return true; }); @@ -133,6 +130,30 @@ TEST(KeyboardKeyChannelHandlerTest, DeadKeysDoNotCrash) { [](bool handled) {}); // EXPECT is done during the callback above. + EXPECT_TRUE(received); +} + +TEST(KeyboardKeyChannelHandlerTest, EmptyResponsesDoNotCrash) { + bool received = false; + TestBinaryMessenger messenger( + [&received](const std::string& channel, const uint8_t* message, + size_t message_size, BinaryReply reply) { + if (channel == "flutter/keyevent") { + std::string empty_message = ""; + std::vector empty_response(empty_message.begin(), + empty_message.end()); + reply(empty_response.data(), empty_response.size()); + received = true; + } + return true; + }); + + KeyboardKeyChannelHandler handler(&messenger); + handler.KeyboardHook(64, kUnhandledScanCode, WM_KEYDOWN, L'b', false, false, + [](bool handled) {}); + + // Passes if it does not crash. + EXPECT_TRUE(received); } } // namespace testing diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc index b30a17c279..a87a0f042b 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -179,9 +179,9 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( FlutterKeyEventType type; // The resulting event's `logical_key`. uint64_t result_logical_key; - // The next value of pressingRecords_[physical_key] (or to remove it). - uint64_t next_logical_record; - bool next_has_record = true; + // 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); @@ -193,7 +193,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( type = kFlutterKeyEventTypeRepeat; assert(had_record); ConvertUtf32ToUtf8_(character_bytes, character); - next_logical_record = last_logical_record; + eventual_logical_record = last_logical_record; result_logical_key = last_logical_record; } else { // A non-repeated key has been pressed that has the exact physical key @@ -208,7 +208,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( type = kFlutterKeyEventTypeDown; assert(!had_record); ConvertUtf32ToUtf8_(character_bytes, character); - next_logical_record = logical_key; + eventual_logical_record = logical_key; result_logical_key = logical_key; } } else { // isPhysicalDown is false @@ -224,22 +224,37 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( assert(had_record); // Up events never have character. character_bytes[0] = '\0'; - next_has_record = false; + eventual_logical_record = 0; result_logical_key = last_logical_record; } } UpdateLastSeenCritialKey(key, physical_key, result_logical_key); - SynchronizeCritialToggledStates(type == kFlutterKeyEventTypeDown ? key : 0); + // 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, type != kFlutterKeyEventTypeRepeat); - if (next_has_record) { - pressingRecords_[physical_key] = next_logical_record; + if (eventual_logical_record != 0) { + pressingRecords_[physical_key] = eventual_logical_record; } else { pressingRecords_.erase(last_logical_record_iter); } - SynchronizeCritialPressedStates(); - 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 @@ -322,7 +337,8 @@ void KeyboardKeyEmbedderHandler::UpdateLastSeenCritialKey( } void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( - int toggle_virtual_key) { + int this_virtual_key, + bool is_down_event) { // TODO(dkwingsmt) consider adding support for synchronizing key state for UWP // https://github.com/flutter/flutter/issues/70202 #ifdef WINUWP @@ -341,7 +357,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( if (key_info.check_toggled) { SHORT state = get_key_state_(virtual_key); bool should_toggled = state & kStateMaskToggled; - if (virtual_key == toggle_virtual_key) { + if (virtual_key == this_virtual_key && is_down_event) { key_info.toggled_on = !key_info.toggled_on; } if (key_info.toggled_on != should_toggled) { @@ -352,10 +368,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( kFlutterKeyEventTypeUp, key_info.physical_key, key_info.logical_key, empty_character), nullptr, nullptr); - } else { - // This key will always be pressed in the following synthesized event. - pressingRecords_[key_info.physical_key] = key_info.logical_key; } + // 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), @@ -367,7 +382,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates( #endif } -void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates() { +void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates( + int this_virtual_key, + bool pressed_state_will_change) { // TODO(dkwingsmt) consider adding support for synchronizing key state for UWP // https://github.com/flutter/flutter/issues/70202 #ifdef WINUWP @@ -386,16 +403,19 @@ 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; + } if (recorded_pressed != should_pressed) { - if (should_pressed) { - pressingRecords_[key_info.physical_key] = key_info.logical_key; - } else { + if (recorded_pressed) { pressingRecords_.erase(recorded_pressed_iter); + } else { + pressingRecords_[key_info.physical_key] = key_info.logical_key; } const char* empty_character = ""; SendEvent( - SynthesizeSimpleEvent(should_pressed ? kFlutterKeyEventTypeDown - : kFlutterKeyEventTypeUp, + SynthesizeSimpleEvent(recorded_pressed ? kFlutterKeyEventTypeUp + : kFlutterKeyEventTypeDown, key_info.physical_key, key_info.logical_key, empty_character), nullptr, nullptr); diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.h b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.h index d7fc69ede9..1d2ea9de0b 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.h +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.h @@ -107,10 +107,10 @@ 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); + 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(); + void SynchronizeCritialPressedStates(int virtual_key, bool was_down); // Wraps perform_send_event_ with state tracking. Use this instead of // |perform_send_event_| to send events to the framework. diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc index 4012f0435e..2527fbbed5 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc @@ -323,8 +323,9 @@ TEST(KeyboardKeyEmbedderHandlerTest, ImeEventsAreIgnored) { EXPECT_EQ(last_handled, true); } -// Test if modifier keys that are told apart by the extended bit -// can be identified. +// Test if modifier keys that are told apart by the extended bit can be +// identified. (Their physical keys must be searched with the extended bit +// considered.) TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) { TestKeystate key_state; std::vector results; @@ -343,7 +344,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) { last_handled = false; key_state.Set(VK_LCONTROL, true); handler->KeyboardHook( - VK_CONTROL, kScanCodeControl, WM_KEYDOWN, 0, false, false, + VK_LCONTROL, kScanCodeControl, WM_KEYDOWN, 0, false, false, [&last_handled](bool handled) { last_handled = handled; }); EXPECT_EQ(last_handled, false); EXPECT_EQ(results.size(), 1); @@ -362,7 +363,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) { last_handled = false; key_state.Set(VK_RCONTROL, true); handler->KeyboardHook( - VK_CONTROL, kScanCodeControl, WM_KEYDOWN, 0, true, true, + VK_RCONTROL, kScanCodeControl, WM_KEYDOWN, 0, true, true, [&last_handled](bool handled) { last_handled = handled; }); EXPECT_EQ(last_handled, false); EXPECT_EQ(results.size(), 1); @@ -381,7 +382,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) { last_handled = false; key_state.Set(VK_LCONTROL, false); handler->KeyboardHook( - VK_CONTROL, kScanCodeControl, WM_KEYUP, 0, false, true, + VK_LCONTROL, kScanCodeControl, WM_KEYUP, 0, false, true, [&last_handled](bool handled) { last_handled = handled; }); EXPECT_EQ(last_handled, false); EXPECT_EQ(results.size(), 1); @@ -400,7 +401,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) { last_handled = false; key_state.Set(VK_RCONTROL, false); handler->KeyboardHook( - VK_CONTROL, kScanCodeControl, WM_KEYUP, 0, true, true, + VK_RCONTROL, kScanCodeControl, WM_KEYUP, 0, true, true, [&last_handled](bool handled) { last_handled = handled; }); EXPECT_EQ(last_handled, false); EXPECT_EQ(results.size(), 1); @@ -852,7 +853,8 @@ TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncToggledState) { event->callback(false, event->user_data); } -TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncToggledStateByItself) { +TEST(KeyboardKeyEmbedderHandlerTest, + SynthesizeForDesyncToggledStateByItselfsUp) { TestKeystate key_state; std::vector results; TestFlutterKeyEvent* event; @@ -911,6 +913,61 @@ TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncToggledStateByItself) { EXPECT_EQ(last_handled, true); } +TEST(KeyboardKeyEmbedderHandlerTest, + SynthesizeForDesyncToggledStateByItselfsDown) { + TestKeystate key_state; + std::vector results; + TestFlutterKeyEvent* event; + bool last_handled = false; + + // NumLock is started up and disabled + key_state.Set(VK_NUMLOCK, false, 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()); + + // NumLock is toggled somewhere else + // key_state.Set(VK_NUMLOCK, false, true); + + // NumLock is pressed + key_state.Set(VK_NUMLOCK, true, false); + handler->KeyboardHook( + VK_NUMLOCK, kScanCodeNumLock, WM_KEYDOWN, 0, true, false, + [&last_handled](bool handled) { last_handled = handled; }); + EXPECT_EQ(last_handled, false); + EXPECT_EQ(results.size(), 3); + event = &results[0]; + EXPECT_EQ(event->type, kFlutterKeyEventTypeDown); + EXPECT_EQ(event->physical, kPhysicalNumLock); + EXPECT_EQ(event->logical, kLogicalNumLock); + EXPECT_STREQ(event->character, ""); + EXPECT_EQ(event->synthesized, true); + EXPECT_EQ(event->callback, nullptr); + + event = &results[1]; + EXPECT_EQ(event->type, kFlutterKeyEventTypeUp); + EXPECT_EQ(event->physical, kPhysicalNumLock); + EXPECT_EQ(event->logical, kLogicalNumLock); + EXPECT_STREQ(event->character, ""); + EXPECT_EQ(event->synthesized, true); + EXPECT_EQ(event->callback, nullptr); + + event = &results[2]; + EXPECT_EQ(event->type, kFlutterKeyEventTypeDown); + EXPECT_EQ(event->physical, kPhysicalNumLock); + EXPECT_EQ(event->logical, kLogicalNumLock); + EXPECT_STREQ(event->character, ""); + EXPECT_EQ(event->synthesized, false); + + last_handled = false; + event->callback(true, event->user_data); + EXPECT_EQ(last_handled, true); +} + TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeWithInitialTogglingState) { TestKeystate key_state; std::vector results;