diff --git a/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc b/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc index 78a4a528f522c..82d43d9637bf8 100644 --- a/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc +++ b/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc @@ -82,9 +82,9 @@ std::unique_ptr GetTestEngine() { [](FLUTTER_API_SYMBOL(FlutterEngine) engine, bool enabled) { return kSuccess; }; - MockEmbedderApiForKeyboard( - modifier, [] { return false; }, - [](const FlutterKeyEvent* event) { return false; }); + + MockEmbedderApiForKeyboard(modifier, + std::make_shared()); engine->RunWithEntrypoint(nullptr); return engine; diff --git a/shell/platform/windows/flutter_window_win32_unittests.cc b/shell/platform/windows/flutter_window_win32_unittests.cc index 1b4a77f13649a..ccfe35cdd45f5 100644 --- a/shell/platform/windows/flutter_window_win32_unittests.cc +++ b/shell/platform/windows/flutter_window_win32_unittests.cc @@ -229,12 +229,10 @@ class TestFlutterWindowsView : public FlutterWindowsView { public: TestFlutterWindowsView(std::unique_ptr window_binding, WPARAM virtual_key, - bool is_printable = true, - bool is_syskey = false) + bool is_printable = true) : FlutterWindowsView(std::move(window_binding)), virtual_key_(virtual_key), - is_printable_(is_printable), - is_syskey_(is_syskey) {} + is_printable_(is_printable) {} SpyKeyboardKeyHandler* key_event_handler; SpyTextInputPlugin* text_input_plugin; @@ -269,8 +267,7 @@ class TestFlutterWindowsView : public FlutterWindowsView { // "SendInput" directly to the window. const KEYBDINPUT kbdinput = pInputs->ki; const bool is_key_up = kbdinput.dwFlags & KEYEVENTF_KEYUP; - const UINT message = is_key_up ? (is_syskey_ ? WM_SYSKEYUP : WM_KEYUP) - : (is_syskey_ ? WM_SYSKEYDOWN : WM_KEYDOWN); + const UINT message = is_key_up ? WM_KEYUP : WM_KEYDOWN; const LPARAM lparam = CreateKeyEventLparam( kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY, is_key_up); @@ -295,7 +292,6 @@ class TestFlutterWindowsView : public FlutterWindowsView { std::vector pending_responds_; WPARAM virtual_key_; bool is_printable_; - bool is_syskey_; }; // The static value to return as the "handled" value from the framework for key @@ -315,9 +311,12 @@ std::unique_ptr GetTestEngine() { auto engine = std::make_unique(project); EngineModifier modifier(engine.get()); - MockEmbedderApiForKeyboard( - modifier, [] { return test_response; }, - [](const FlutterKeyEvent* event) { return false; }); + auto key_response_controller = std::make_shared(); + key_response_controller->SetChannelResponse( + [](MockKeyResponseController::ResponseCallback callback) { + callback(test_response); + }); + MockEmbedderApiForKeyboard(modifier, key_response_controller); return engine; } @@ -392,8 +391,7 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) { auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), virtual_key, false /* is_printable */, - true /* is_syskey */); + std::move(window_binding_handler), virtual_key, false /* is_printable */); win32window.SetView(&flutter_windows_view); LPARAM lparam = CreateKeyEventLparam(scan_code, false, false); @@ -406,9 +404,11 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) { false /* extended */, _)) .Times(2) .RetiresOnSaturation(); + // Syskey events are not redispatched, so TextInputPlugin, which relies on + // them to receive events, no longer works. EXPECT_CALL(*flutter_windows_view.text_input_plugin, KeyboardHook(_, _, _, _, _, _)) - .Times(1) + .Times(0) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_)).Times(0); EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_)).Times(0); diff --git a/shell/platform/windows/flutter_windows_view_unittests.cc b/shell/platform/windows/flutter_windows_view_unittests.cc index 1df7b193aae1f..fdd3dc2150b64 100644 --- a/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/shell/platform/windows/flutter_windows_view_unittests.cc @@ -65,17 +65,22 @@ std::unique_ptr GetTestEngine() { auto engine = std::make_unique(project); EngineModifier modifier(engine.get()); - MockEmbedderApiForKeyboard( - modifier, - [] { + + auto key_response_controller = std::make_shared(); + key_response_controller->SetChannelResponse( + [](MockKeyResponseController::ResponseCallback callback) { key_event_logs.push_back(kKeyEventFromChannel); - return test_response; - }, - [](const FlutterKeyEvent* event) { + callback(test_response); + }); + key_response_controller->SetEmbedderResponse( + [](const FlutterKeyEvent* event, + MockKeyResponseController::ResponseCallback callback) { key_event_logs.push_back(kKeyEventFromEmbedder); - return test_response; + callback(test_response); }); + MockEmbedderApiForKeyboard(modifier, key_response_controller); + engine->RunWithEntrypoint(nullptr); return engine; } diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index d28ee0a38bdf5..e9617464ccb6a 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -243,7 +243,9 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( if (eventual_logical_record != 0) { pressingRecords_[physical_key] = eventual_logical_record; } else { - pressingRecords_.erase(last_logical_record_iter); + auto record_iter = pressingRecords_.find(physical_key); + assert(record_iter != pressingRecords_.end()); + pressingRecords_.erase(record_iter); } if (result_logical_key == VK_PROCESSKEY) { diff --git a/shell/platform/windows/keyboard_key_handler.cc b/shell/platform/windows/keyboard_key_handler.cc index bb47b21736bb5..e43dfd182ac51 100644 --- a/shell/platform/windows/keyboard_key_handler.cc +++ b/shell/platform/windows/keyboard_key_handler.cc @@ -129,9 +129,9 @@ void KeyboardKeyHandler::DispatchEvent(const PendingEvent& event) { #else char32_t character = event.character; - if (event.action == WM_SYSKEYDOWN || event.action == WM_SYSKEYUP) { - return; - } + assert(event.action != WM_SYSKEYDOWN && event.action != WM_SYSKEYUP && + "Unexpectedly dispatching a SYS event. SYS events can't be dispatched " + "and should have been prevented in earlier code."); INPUT input_event{ .type = INPUT_KEYBOARD, @@ -272,13 +272,18 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, if (event.unreplied == 0) { std::unique_ptr event_ptr = std::move(*iter); pending_responds_.erase(iter); - // Don't dispatch handled events or dead key events. + // Don't dispatch handled events, and also ignore dead key events and + // sys events. // // Redispatching dead keys events makes Win32 ignore the dead key state // and redispatches a normal character without combining it with the - // next letter key. - const bool should_redispatch = - !event_ptr->any_handled && !_IsDeadKey(event_ptr->character); + // next letter key. Redispatching sys events is impossible due to + // the limitation of |SendInput|. + const bool is_syskey = + event.action == WM_SYSKEYDOWN || event.action == WM_SYSKEYUP; + const bool should_redispatch = !event_ptr->any_handled && + !_IsDeadKey(event_ptr->character) && + !is_syskey; if (should_redispatch) { RedispatchEvent(std::move(event_ptr)); } diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index aadca7ec69754..4bb9e522bdb63 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -174,10 +174,7 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, keyCode = ResolveKeyCode(keyCode, extended, scancode); const bool was_down = lparam & 0x40000000; bool is_syskey = message == WM_SYSKEYDOWN || message == WM_SYSKEYUP; - const int action = is_keydown_message - ? (is_syskey ? WM_SYSKEYDOWN : WM_KEYDOWN) - : (is_syskey ? WM_SYSKEYUP : WM_KEYUP); - if (window_delegate_->OnKey(keyCode, scancode, action, 0, extended, + if (window_delegate_->OnKey(keyCode, scancode, message, 0, extended, was_down)) { // For system keys, always pass them to the default WndProc so that keys // like the ALT-TAB or Kanji switches are processed correctly. diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index 2be6885057810..0ef5a1908071d 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -133,12 +133,6 @@ class TestKeystate { std::map state_; }; -typedef struct { - UINT cInputs; - KEYBDINPUT kbdinput; - int cbSize; -} SendInputInfo; - // A FlutterWindowsView that overrides the RegisterKeyboardHandlers function // to register the keyboard hook handlers that can be spied upon. class TestFlutterWindowsView : public FlutterWindowsView { @@ -150,18 +144,14 @@ class TestFlutterWindowsView : public FlutterWindowsView { // affect keyboard. : FlutterWindowsView( std::make_unique<::testing::NiceMock>()), - on_text_(std::move(on_text)), - redispatch_char(0) {} - - uint32_t redispatch_char; + on_text_(std::move(on_text)) {} void OnText(const std::u16string& text) override { on_text_(text); } int InjectPendingEvents(MockMessageQueue* queue, uint32_t redispatch_char) { std::vector messages; int num_pending_responds = pending_responds_.size(); - for (const SendInputInfo& input : pending_responds_) { - const KEYBDINPUT kbdinput = input.kbdinput; + for (const KEYBDINPUT& kbdinput : pending_responds_) { const UINT message = (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; const bool is_key_up = kbdinput.dwFlags & KEYEVENTF_KEYUP; @@ -180,8 +170,8 @@ class TestFlutterWindowsView : public FlutterWindowsView { } } - queue->InjectMessageList(messages.size(), messages.data()); pending_responds_.clear(); + queue->InjectMessageList(messages.size(), messages.data()); return num_pending_responds; } @@ -204,12 +194,14 @@ class TestFlutterWindowsView : public FlutterWindowsView { private: UINT SendInput(UINT cInputs, LPINPUT pInputs, int cbSize) { - pending_responds_.push_back({cInputs, pInputs->ki, cbSize}); + for (UINT input_idx = 0; input_idx < cInputs; input_idx += 1) { + pending_responds_.push_back(pInputs[input_idx].ki); + } return 1; } U16StringHandler on_text_; - std::vector pending_responds_; + std::vector pending_responds_; TestKeystate key_state_; }; @@ -238,11 +230,12 @@ void clear_key_calls() { key_calls.clear(); } -std::unique_ptr GetTestEngine(); - class KeyboardTester { public: - explicit KeyboardTester() { + using ResponseHandler = + std::function; + + explicit KeyboardTester() : callback_handler_(RespondValue(false)) { view_ = std::make_unique( [](const std::u16string& text) { key_calls.push_back(KeyCall{ @@ -250,7 +243,20 @@ class KeyboardTester { .text = text, }); }); - view_->SetEngine(std::move(GetTestEngine())); + view_->SetEngine(GetTestEngine( + [&callback_handler = callback_handler_]( + const FlutterKeyEvent* event, + MockKeyResponseController::ResponseCallback callback) { + FlutterKeyEvent clone_event = *event; + clone_event.character = event->character == nullptr + ? nullptr + : clone_string(event->character); + key_calls.push_back(KeyCall{ + .type = kKeyCallOnKey, + .key_event = clone_event, + }); + callback_handler(event, callback); + })); window_ = std::make_unique(view_.get()); } @@ -258,7 +264,18 @@ class KeyboardTester { view_->SetKeyState(key, pressed, toggled_on); } - void Responding(bool response) { test_response = response; } + void Responding(bool response) { callback_handler_ = RespondValue(response); } + + // Manually handle event callback of the onKeyData embedder API. + // + // On every onKeyData call, the |handler| will be invoked with the target + // key data and the result callback. Immediately calling the callback with + // a boolean is equivalent to setting |Responding| with the boolean. However, + // |LateResponding| allows storing the callback to call later. + void LateResponding( + MockKeyResponseController::EmbedderCallbackHandler handler) { + callback_handler_ = std::move(handler); + } void SetLayout(MapVkToCharHandler layout) { window_->SetLayout(layout); } @@ -285,47 +302,48 @@ class KeyboardTester { return view_->InjectPendingEvents(window_.get(), redispatch_char); } - static bool test_response; - private: std::unique_ptr view_; std::unique_ptr window_; -}; - -bool KeyboardTester::test_response = false; - -// Returns an engine instance configured with dummy project path values, and -// overridden methods for sending platform messages, so that the engine can -// respond as if the framework were connected. -std::unique_ptr GetTestEngine() { - FlutterDesktopEngineProperties properties = {}; - properties.assets_path = L"C:\\foo\\flutter_assets"; - properties.icu_data_path = L"C:\\foo\\icudtl.dat"; - properties.aot_library_path = L"C:\\foo\\aot.so"; - FlutterProjectBundle project(properties); - auto engine = std::make_unique(project); - - EngineModifier modifier(engine.get()); - - MockEmbedderApiForKeyboard( - modifier, [] { return KeyboardTester::test_response; }, - [](const FlutterKeyEvent* event) { - FlutterKeyEvent clone_event = *event; - clone_event.character = event->character == nullptr - ? nullptr - : clone_string(event->character); - key_calls.push_back(KeyCall{ - .type = kKeyCallOnKey, - .key_event = clone_event, - }); - return KeyboardTester::test_response; - }); + MockKeyResponseController::EmbedderCallbackHandler callback_handler_; + + // Returns an engine instance configured with dummy project path values, and + // overridden methods for sending platform messages, so that the engine can + // respond as if the framework were connected. + static std::unique_ptr GetTestEngine( + MockKeyResponseController::EmbedderCallbackHandler + embedder_callback_handler) { + FlutterDesktopEngineProperties properties = {}; + properties.assets_path = L"C:\\foo\\flutter_assets"; + properties.icu_data_path = L"C:\\foo\\icudtl.dat"; + properties.aot_library_path = L"C:\\foo\\aot.so"; + FlutterProjectBundle project(properties); + auto engine = std::make_unique(project); + + EngineModifier modifier(engine.get()); + + auto key_response_controller = + std::make_shared(); + key_response_controller->SetEmbedderResponse( + std::move(embedder_callback_handler)); + + MockEmbedderApiForKeyboard(modifier, key_response_controller); + + engine->RunWithEntrypoint(nullptr); + return engine; + } - engine->RunWithEntrypoint(nullptr); - return engine; -} + static MockKeyResponseController::EmbedderCallbackHandler RespondValue( + bool value) { + return [value](const FlutterKeyEvent* event, + MockKeyResponseController::ResponseCallback callback) { + callback(value); + }; + } +}; constexpr uint64_t kScanCodeKeyA = 0x1e; +constexpr uint64_t kScanCodeKeyB = 0x30; constexpr uint64_t kScanCodeKeyE = 0x12; constexpr uint64_t kScanCodeKeyQ = 0x10; constexpr uint64_t kScanCodeKeyW = 0x11; @@ -336,11 +354,12 @@ constexpr uint64_t kScanCodeDigit6 = 0x07; constexpr uint64_t kScanCodeControl = 0x1d; constexpr uint64_t kScanCodeAlt = 0x38; constexpr uint64_t kScanCodeShiftLeft = 0x2a; -// constexpr uint64_t kScanCodeShiftRight = 0x36; +constexpr uint64_t kScanCodeShiftRight = 0x36; constexpr uint64_t kScanCodeBracketLeft = 0x1a; constexpr uint64_t kVirtualDigit1 = 0x31; constexpr uint64_t kVirtualKeyA = 0x41; +constexpr uint64_t kVirtualKeyB = 0x42; constexpr uint64_t kVirtualKeyE = 0x45; constexpr uint64_t kVirtualKeyQ = 0x51; constexpr uint64_t kVirtualKeyW = 0x57; @@ -712,7 +731,7 @@ TEST(KeyboardTest, AltGrModifiedKey) { kNotSynthesized); clear_key_calls(); - tester.InjectPendingEvents(); + EXPECT_EQ(tester.InjectPendingEvents(), 2); EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); @@ -729,7 +748,7 @@ TEST(KeyboardTest, AltGrModifiedKey) { kLogicalKeyQ, "@", kNotSynthesized); clear_key_calls(); - tester.InjectPendingEvents('@'); + EXPECT_EQ(tester.InjectPendingEvents('@'), 1); EXPECT_EQ(key_calls.size(), 1); EXPECT_CALL_IS_TEXT(key_calls[0], u"@"); clear_key_calls(); @@ -744,12 +763,12 @@ TEST(KeyboardTest, AltGrModifiedKey) { kLogicalKeyQ, "", kNotSynthesized); clear_key_calls(); - tester.InjectPendingEvents(); + EXPECT_EQ(tester.InjectPendingEvents(), 1); EXPECT_EQ(key_calls.size(), 0); // Release AltGr. Win32 doesn't dispatch ControlLeft up. Instead Flutter will - // dispatch one. The AltGr is a system key, so will be handled by Win32's - // default WndProc. + // dispatch one. The AltGr is a system key, therefore will be handled by + // Win32's default WndProc. tester.InjectMessages( 1, WmSysKeyUpInfo{VK_MENU, kScanCodeAlt, kExtended}.Build(kWmResultDefault)); @@ -759,13 +778,132 @@ TEST(KeyboardTest, AltGrModifiedKey) { kLogicalAltRight, "", kNotSynthesized); clear_key_calls(); + // Dispatch the ControlLeft up event appended by Flutter. + tester.SetKeyState(VK_LCONTROL, false, true); + EXPECT_EQ(tester.InjectPendingEvents(), 1); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, + kPhysicalControlLeft, kLogicalControlLeft, "", + kNotSynthesized); + clear_key_calls(); +} + +// Test the following two key sequences at the same time: +// +// 1. Tap AltGr, then tap AltGr. +// 2. Tap AltGr, hold CtrlLeft, tap AltGr, release CtrlLeft. +// +// The two sequences are indistinguishable until the very end when a CtrlLeft +// up event might or might not follow. +// +// Sequence 1: CtrlLeft down, AltRight down, AltRight up +// Sequence 2: CtrlLeft down, AltRight down, AltRight up, CtrlLeft up +// +// This is because pressing AltGr alone causes Win32 to send a fake "CtrlLeft +// down" event first (see |IsKeyDownAltRight| for detailed explanation). +TEST(KeyboardTest, AltGrTwice) { + KeyboardTester tester; + tester.Responding(false); + + // 1. AltGr down. + + // The key down event causes a ControlLeft down and a AltRight (extended + // AltLeft) down. + tester.SetKeyState(VK_LCONTROL, true, true); + tester.InjectMessages( + 2, + WmKeyDownInfo{VK_LCONTROL, kScanCodeControl, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmKeyDownInfo{VK_MENU, kScanCodeAlt, kExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 2); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, + kPhysicalControlLeft, kLogicalControlLeft, "", + kNotSynthesized); + EXPECT_CALL_IS_EVENT(key_calls[1], kFlutterKeyEventTypeDown, + kPhysicalAltRight, kLogicalAltRight, "", + kNotSynthesized); + clear_key_calls(); + + EXPECT_EQ(tester.InjectPendingEvents(), 2); + EXPECT_EQ(key_calls.size(), 0); + + // 2. AltGr up. + + // The key up event only causes a AltRight (extended AltLeft) up. + tester.SetKeyState(VK_RMENU, false, true); + tester.InjectMessages( + 1, + WmSysKeyUpInfo{VK_MENU, kScanCodeAlt, kExtended}.Build(kWmResultDefault)); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, kPhysicalAltRight, + kLogicalAltRight, "", kNotSynthesized); + clear_key_calls(); + + // Dispatch the ControlLeft up event appended by Flutter. + tester.SetKeyState(VK_LCONTROL, false, true); + EXPECT_EQ(tester.InjectPendingEvents(), 1); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, + kPhysicalControlLeft, kLogicalControlLeft, "", + kNotSynthesized); + clear_key_calls(); + + // Redispatch the ControlLeft up event. + EXPECT_EQ(tester.InjectPendingEvents(), 1); + EXPECT_EQ(key_calls.size(), 0); + clear_key_calls(); + + // 3. AltGr down (or: ControlLeft down then AltRight down.) + + tester.SetKeyState(VK_LCONTROL, true, false); + tester.InjectMessages( + 2, + WmKeyDownInfo{VK_LCONTROL, kScanCodeControl, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmKeyDownInfo{VK_MENU, kScanCodeAlt, kExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 2); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, + kPhysicalControlLeft, kLogicalControlLeft, "", + kNotSynthesized); + EXPECT_CALL_IS_EVENT(key_calls[1], kFlutterKeyEventTypeDown, + kPhysicalAltRight, kLogicalAltRight, "", + kNotSynthesized); + clear_key_calls(); + + EXPECT_EQ(tester.InjectPendingEvents(), 2); + EXPECT_EQ(key_calls.size(), 0); + + // 4. AltGr up. + + // The key up event only causes a AltRight (extended AltLeft) up. + tester.SetKeyState(VK_RMENU, false, false); + tester.InjectMessages( + 1, + WmSysKeyUpInfo{VK_MENU, kScanCodeAlt, kExtended}.Build(kWmResultDefault)); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, kPhysicalAltRight, + kLogicalAltRight, "", kNotSynthesized); + clear_key_calls(); + + // Dispatch a ControlLeft up event from Flutter. tester.SetKeyState(VK_LCONTROL, false, false); - tester.InjectPendingEvents(); + EXPECT_EQ(tester.InjectPendingEvents(), 1); EXPECT_EQ(key_calls.size(), 1); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, kPhysicalControlLeft, kLogicalControlLeft, "", kNotSynthesized); clear_key_calls(); + + // 5. For key sequence 2: a real ControlLeft up. + tester.InjectMessages( + 1, WmKeyUpInfo{VK_LCONTROL, kScanCodeControl, kNotExtended}.Build( + kWmResultDefault)); + EXPECT_EQ(key_calls.size(), 0); + clear_key_calls(); } // This tests dead key ^ then E on a French keyboard, which should be combined @@ -1077,5 +1215,133 @@ TEST(KeyboardTest, MultibyteCharacter) { clear_key_calls(); } +// A key down event for shift right must not be redispatched even if +// the framework returns unhandled. +// +// The reason for this test is documented in |IsKeyDownShiftRight|. +TEST(KeyboardTest, NeverRedispatchShiftRightKeyDown) { + KeyboardTester tester; + tester.Responding(false); + + // Press ShiftRight and the delegate responds false. + tester.SetKeyState(VK_RSHIFT, true, true); + tester.InjectMessages( + 1, + WmKeyDownInfo{VK_SHIFT, kScanCodeShiftRight, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + clear_key_calls(); + + // Try to dispatch events. There should be nothing. + EXPECT_EQ(tester.InjectPendingEvents(), 0); + EXPECT_EQ(key_calls.size(), 0); +} + +TEST(KeyboardTest, DisorderlyRespondedEvents) { + KeyboardTester tester; + + // Store callbacks to manually call them. + std::vector recorded_callbacks; + tester.LateResponding( + [&recorded_callbacks]( + const FlutterKeyEvent* event, + MockKeyResponseController::ResponseCallback callback) { + recorded_callbacks.push_back(callback); + }); + + // Press A + tester.InjectMessages( + 2, + WmKeyDownInfo{kVirtualKeyA, kScanCodeKeyA, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmCharInfo{'a', kScanCodeKeyA, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + // Press B + tester.InjectMessages( + 2, + WmKeyDownInfo{kVirtualKeyB, kScanCodeKeyB, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmCharInfo{'b', kScanCodeKeyB, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 2); + EXPECT_EQ(recorded_callbacks.size(), 2); + clear_key_calls(); + + // Resolve the second event first to test disordered responses. + recorded_callbacks.back()(false); + + EXPECT_EQ(tester.InjectPendingEvents('b'), 1); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_TEXT(key_calls[0], u"b"); + clear_key_calls(); + + // Resolve the first event. + recorded_callbacks.front()(false); + + EXPECT_EQ(tester.InjectPendingEvents('a'), 1); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); + clear_key_calls(); +} + +// Regression test for a crash in an earlier implementation. +// +// In real life, the framework responds slowly. The next real event might +// arrive earlier than the framework response, and if the 2nd event has an +// identical hash as the one waiting for response, an earlier implementation +// will crash upon the response. +TEST(KeyboardTest, SlowFrameworkResponse) { + KeyboardTester tester; + + std::vector recorded_callbacks; + + // Store callbacks to manually call them. + tester.LateResponding( + [&recorded_callbacks]( + const FlutterKeyEvent* event, + MockKeyResponseController::ResponseCallback callback) { + recorded_callbacks.push_back(callback); + }); + + // Press A + tester.InjectMessages( + 2, + WmKeyDownInfo{kVirtualKeyA, kScanCodeKeyA, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmCharInfo{'a', kScanCodeKeyA, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + // Hold A + tester.InjectMessages( + 2, + WmKeyDownInfo{kVirtualKeyA, kScanCodeKeyA, kNotExtended, kWasDown}.Build( + kWmResultZero), + WmCharInfo{'a', kScanCodeKeyA, kNotExtended, kWasDown}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 2); + EXPECT_EQ(recorded_callbacks.size(), 2); + clear_key_calls(); + + // The first response. + recorded_callbacks.front()(false); + + EXPECT_EQ(tester.InjectPendingEvents('a'), 1); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); + clear_key_calls(); + + // The second response. + recorded_callbacks.back()(false); + + EXPECT_EQ(tester.InjectPendingEvents('a'), 1); + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); + clear_key_calls(); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/testing/test_keyboard.cc b/shell/platform/windows/testing/test_keyboard.cc index 01971f936bca4..b93308d7cc92d 100644 --- a/shell/platform/windows/testing/test_keyboard.cc +++ b/shell/platform/windows/testing/test_keyboard.cc @@ -85,8 +85,7 @@ LPARAM CreateKeyEventLparam(USHORT scancode, (LPARAM(scancode) << 16) | LPARAM(repeat_count)); } -static MockKeyEventChannelHandler stored_channel_handler; -static MockKeyEventEmbedderHandler stored_embedder_handler; +static std::shared_ptr stored_response_controller; // Set EngineModifier, listen to event messages that go through the channel and // the embedder API, while disabling other methods so that the engine can be @@ -94,11 +93,10 @@ static MockKeyEventEmbedderHandler stored_embedder_handler; // // The |channel_handler| and |embedder_handler| should return a boolean // indicating whether the framework decides to handle the event. -void MockEmbedderApiForKeyboard(EngineModifier& modifier, - MockKeyEventChannelHandler channel_handler, - MockKeyEventEmbedderHandler embedder_handler) { - stored_channel_handler = channel_handler; - stored_embedder_handler = embedder_handler; +void MockEmbedderApiForKeyboard( + EngineModifier& modifier, + std::shared_ptr response_controller) { + stored_response_controller = response_controller; // This mock handles channel messages. modifier.embedder_api().SendPlatformMessage = [](FLUTTER_API_SYMBOL(FlutterEngine) engine, @@ -107,29 +105,32 @@ void MockEmbedderApiForKeyboard(EngineModifier& modifier, return kSuccess; } if (std::string(message->channel) == std::string("flutter/keyevent")) { - bool result = stored_channel_handler(); - auto response = _keyHandlingResponse(result); - const TestResponseHandle* response_handle = - reinterpret_cast( - message->response_handle); - if (response_handle->callback != nullptr) { - response_handle->callback(response->data(), response->size(), - response_handle->user_data); - } + stored_response_controller->HandleChannelMessage( + [message](bool handled) { + auto response = _keyHandlingResponse(handled); + const TestResponseHandle* response_handle = + reinterpret_cast( + message->response_handle); + if (response_handle->callback != nullptr) { + response_handle->callback(response->data(), response->size(), + response_handle->user_data); + } + }); return kSuccess; } return kSuccess; }; - // This mock handles key events sent through the embedder API, - // and records it in `key_calls`. + // This mock handles key events sent through the embedder API. modifier.embedder_api().SendKeyEvent = [](FLUTTER_API_SYMBOL(FlutterEngine) engine, const FlutterKeyEvent* event, FlutterKeyEventCallback callback, void* user_data) { - bool result = stored_embedder_handler(event); - if (callback != nullptr) { - callback(result, user_data); - } + stored_response_controller->HandleEmbedderMessage( + event, [callback, user_data](bool handled) { + if (callback != nullptr) { + callback(handled, user_data); + } + }); return kSuccess; }; diff --git a/shell/platform/windows/testing/test_keyboard.h b/shell/platform/windows/testing/test_keyboard.h index 39d13c1dcd35e..1d0b8f02c5763 100644 --- a/shell/platform/windows/testing/test_keyboard.h +++ b/shell/platform/windows/testing/test_keyboard.h @@ -42,13 +42,51 @@ LPARAM CreateKeyEventLparam(USHORT scancode, bool context_code = 0, bool transition_state = 1); -typedef std::function MockKeyEventChannelHandler; -typedef std::function - MockKeyEventEmbedderHandler; +class MockKeyResponseController { + public: + using ResponseCallback = std::function; + using EmbedderCallbackHandler = + std::function; + using ChannelCallbackHandler = std::function; + + MockKeyResponseController() + : channel_response_(ChannelRespondFalse), + embedder_response_(EmbedderRespondFalse) {} + + void SetChannelResponse(ChannelCallbackHandler handler) { + channel_response_ = std::move(handler); + } + + void SetEmbedderResponse(EmbedderCallbackHandler handler) { + embedder_response_ = std::move(handler); + } + + void HandleChannelMessage(ResponseCallback callback) { + channel_response_(callback); + } + + void HandleEmbedderMessage(const FlutterKeyEvent* event, + ResponseCallback callback) { + embedder_response_(event, std::move(callback)); + } + + private: + EmbedderCallbackHandler embedder_response_; + ChannelCallbackHandler channel_response_; + + static void ChannelRespondFalse(ResponseCallback callback) { + callback(false); + } + + static void EmbedderRespondFalse(const FlutterKeyEvent* event, + ResponseCallback callback) { + callback(false); + } +}; -void MockEmbedderApiForKeyboard(EngineModifier& modifier, - MockKeyEventChannelHandler channel_handler, - MockKeyEventEmbedderHandler embedder_handler); +void MockEmbedderApiForKeyboard( + EngineModifier& modifier, + std::shared_ptr response_controller); // Simulate a message queue for WM messages. //