From ed653b207c6988e1e1638ac32559f99d67a9976d Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 6 Jan 2022 03:14:45 -0800 Subject: [PATCH 1/9] Compilable --- .../platform/windows/flutter_window_win32.cc | 9 +- shell/platform/windows/flutter_window_win32.h | 5 +- .../windows/flutter_window_win32_unittests.cc | 172 +++--- .../platform/windows/flutter_windows_view.cc | 45 +- shell/platform/windows/flutter_windows_view.h | 11 +- .../windows/flutter_windows_view_unittests.cc | 9 +- .../platform/windows/keyboard_handler_base.h | 10 +- .../platform/windows/keyboard_key_handler.cc | 227 +------- shell/platform/windows/keyboard_key_handler.h | 80 +-- .../windows/keyboard_key_handler_unittests.cc | 515 +++--------------- .../windows/keyboard_manager_win32.cc | 293 ++++++++-- .../platform/windows/keyboard_manager_win32.h | 92 +++- .../windows/keyboard_win32_unittests.cc | 91 ++-- .../windows/testing/mock_window_win32.h | 9 +- .../windows/window_binding_handler_delegate.h | 9 +- shell/platform/windows/window_win32.cc | 6 + shell/platform/windows/window_win32.h | 5 + .../windows/window_win32_unittests.cc | 14 +- 18 files changed, 638 insertions(+), 964 deletions(-) diff --git a/shell/platform/windows/flutter_window_win32.cc b/shell/platform/windows/flutter_window_win32.cc index 8c72d6b273094..15c87d6ed7016 100644 --- a/shell/platform/windows/flutter_window_win32.cc +++ b/shell/platform/windows/flutter_window_win32.cc @@ -184,14 +184,15 @@ void FlutterWindowWin32::OnText(const std::u16string& text) { binding_handler_delegate_->OnText(text); } -bool FlutterWindowWin32::OnKey(int key, +void FlutterWindowWin32::OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) { - return binding_handler_delegate_->OnKey(key, scancode, action, character, - extended, was_down); + bool was_down, + KeyEventCallback callback) { + binding_handler_delegate_->OnKey(key, scancode, action, character, extended, + was_down, std::move(callback)); } void FlutterWindowWin32::OnComposeBegin() { diff --git a/shell/platform/windows/flutter_window_win32.h b/shell/platform/windows/flutter_window_win32.h index 584daa4a932dc..fdcd2740298ce 100644 --- a/shell/platform/windows/flutter_window_win32.h +++ b/shell/platform/windows/flutter_window_win32.h @@ -67,12 +67,13 @@ class FlutterWindowWin32 : public WindowWin32, public WindowBindingHandler { void OnText(const std::u16string& text) override; // |WindowWin32| - bool OnKey(int key, + void OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) override; + bool was_down, + KeyEventCallback callback) override; // |WindowWin32| void OnComposeBegin() override; diff --git a/shell/platform/windows/flutter_window_win32_unittests.cc b/shell/platform/windows/flutter_window_win32_unittests.cc index 799dce73c2faf..505e9afc7f245 100644 --- a/shell/platform/windows/flutter_window_win32_unittests.cc +++ b/shell/platform/windows/flutter_window_win32_unittests.cc @@ -34,23 +34,23 @@ static constexpr int32_t kDefaultPointerDeviceId = 0; // key event handler. class SpyKeyboardKeyHandler : public KeyboardHandlerBase { public: - SpyKeyboardKeyHandler(flutter::BinaryMessenger* messenger, - KeyboardKeyHandler::EventDispatcher dispatch_event) { - real_implementation_ = std::make_unique(dispatch_event); + SpyKeyboardKeyHandler(flutter::BinaryMessenger* messenger) { + real_implementation_ = std::make_unique(); real_implementation_->AddDelegate( std::make_unique(messenger)); - ON_CALL(*this, KeyboardHook(_, _, _, _, _, _)) + ON_CALL(*this, KeyboardHook(_, _, _, _, _, _, _)) .WillByDefault(Invoke(real_implementation_.get(), &KeyboardKeyHandler::KeyboardHook)); } - MOCK_METHOD6(KeyboardHook, - bool(int key, + MOCK_METHOD7(KeyboardHook, + void(int key, int scancode, int action, char32_t character, bool extended, - bool was_down)); + bool was_down, + KeyEventCallback callback)); MOCK_METHOD0(ComposeBeginHook, void()); MOCK_METHOD0(ComposeCommitHook, void()); MOCK_METHOD0(ComposeEndHook, void()); @@ -101,7 +101,10 @@ class SpyTextInputPlugin : public TextInputPlugin, class MockFlutterWindowWin32 : public FlutterWindowWin32, public MockMessageQueue { public: - MockFlutterWindowWin32() : FlutterWindowWin32(800, 600) { + MockFlutterWindowWin32(WPARAM virtual_key = 0, bool is_printable = true) + : virtual_key_(virtual_key), + is_printable_(is_printable), + FlutterWindowWin32(800, 600) { ON_CALL(*this, GetDpiScale()) .WillByDefault(Return(this->FlutterWindowWin32::GetDpiScale())); } @@ -151,14 +154,16 @@ class MockFlutterWindowWin32 : public FlutterWindowWin32, MOCK_METHOD0(OnResetImeComposing, void()); protected: - virtual BOOL Win32PeekMessage(LPMSG lpMsg, - UINT wMsgFilterMin, - UINT wMsgFilterMax, - UINT wRemoveMsg) override { + // |KeyboardManagerWin32::WindowDelegate| + BOOL Win32PeekMessage(LPMSG lpMsg, + UINT wMsgFilterMin, + UINT wMsgFilterMax, + UINT wRemoveMsg) override { return MockMessageQueue::Win32PeekMessage(lpMsg, wMsgFilterMin, wMsgFilterMax, wRemoveMsg); } + // |KeyboardManagerWin32::WindowDelegate| LRESULT Win32DefWindowProc(HWND hWnd, UINT Msg, WPARAM wParam, @@ -166,12 +171,51 @@ class MockFlutterWindowWin32 : public FlutterWindowWin32, return kWmResultDefault; } - private: + // |KeyboardManagerWin32::WindowDelegate| + UINT Win32DispatchEvent(UINT cInputs, LPINPUT pInputs, int cbSize) override { + for (UINT input_idx = 0; input_idx < cInputs; input_idx += 1) { + SendInput(pInputs[input_idx].ki); + } + return 1; + } + + // |MockMessageQueue| LRESULT Win32SendMessage(UINT const message, WPARAM const wparam, LPARAM const lparam) override { return HandleMessage(message, wparam, lparam); } + + private: + UINT SendInput(KEYBDINPUT kbdinput) { + // Simulate the event loop by just sending the event sent to + // "SendInput" directly to the window. + const bool is_key_up = kbdinput.dwFlags & KEYEVENTF_KEYUP; + const UINT message = is_key_up ? WM_KEYUP : WM_KEYDOWN; + + const LPARAM lparam = CreateKeyEventLparam( + kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY, is_key_up); + // Windows would normally fill in the virtual key code for us, so we + // simulate it for the test with the key we know is in the test. The + // KBDINPUT we're passed doesn't have it filled in (on purpose, so that + // Windows will fill it in). + // + // TODO(dkwingsmt): Don't check the message results for redispatched + // messages for now, because making them work takes non-trivial rework + // to our current structure. https://github.com/flutter/flutter/issues/87843 + // If this is resolved, change them to kWmResultDefault. + pending_responds_.push_back( + Win32Message{message, virtual_key_, lparam, kWmResultDontCheck}); + if (is_printable_ && (kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { + pending_responds_.push_back( + Win32Message{WM_CHAR, virtual_key_, lparam, kWmResultDontCheck}); + } + return 1; + } + + std::vector pending_responds_; + WPARAM virtual_key_; + bool is_printable_; }; class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate { @@ -201,7 +245,14 @@ class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate { FlutterPointerMouseButtons)); MOCK_METHOD2(OnPointerLeave, void(FlutterPointerDeviceKind, int32_t)); MOCK_METHOD1(OnText, void(const std::u16string&)); - MOCK_METHOD6(OnKey, bool(int, int, int, char32_t, bool, bool)); + MOCK_METHOD7(OnKey, + void(int, + int, + int, + char32_t, + bool, + bool, + KeyboardManagerWin32::KeyEventCallback)); MOCK_METHOD0(OnComposeBegin, void()); MOCK_METHOD0(OnComposeCommit, void()); MOCK_METHOD0(OnComposeEnd, void()); @@ -223,32 +274,19 @@ class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate { // to register the keyboard hook handlers that can be spied upon. class TestFlutterWindowsView : public FlutterWindowsView { public: - TestFlutterWindowsView(std::unique_ptr window_binding, - WPARAM virtual_key, - bool is_printable = true) - : FlutterWindowsView(std::move(window_binding)), - virtual_key_(virtual_key), - is_printable_(is_printable) {} + TestFlutterWindowsView(std::unique_ptr window_binding) + : FlutterWindowsView(std::move(window_binding)) {} SpyKeyboardKeyHandler* key_event_handler; SpyTextInputPlugin* text_input_plugin; - void InjectPendingEvents(MockFlutterWindowWin32* win32window) { - win32window->InjectMessageList(pending_responds_.size(), - pending_responds_.data()); - pending_responds_.clear(); - } - protected: std::unique_ptr CreateKeyboardKeyHandler( flutter::BinaryMessenger* messenger, - flutter::KeyboardKeyHandler::EventDispatcher dispatch_event, flutter::KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state) override { - auto spy_key_event_handler = std::make_unique( - messenger, [this](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { - return this->SendInput(cInputs, pInputs, cbSize); - }); + auto spy_key_event_handler = + std::make_unique(messenger); key_event_handler = spy_key_event_handler.get(); return spy_key_event_handler; } @@ -260,38 +298,6 @@ class TestFlutterWindowsView : public FlutterWindowsView { text_input_plugin = spy_key_event_handler.get(); return spy_key_event_handler; } - - private: - UINT SendInput(UINT cInputs, LPINPUT pInputs, int cbSize) { - // Simulate the event loop by just sending the event sent to - // "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 ? WM_KEYUP : WM_KEYDOWN; - - const LPARAM lparam = CreateKeyEventLparam( - kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY, is_key_up); - // Windows would normally fill in the virtual key code for us, so we - // simulate it for the test with the key we know is in the test. The - // KBDINPUT we're passed doesn't have it filled in (on purpose, so that - // Windows will fill it in). - // - // TODO(dkwingsmt): Don't check the message results for redispatched - // messages for now, because making them work takes non-trivial rework - // to our current structure. https://github.com/flutter/flutter/issues/87843 - // If this is resolved, change them to kWmResultDefault. - pending_responds_.push_back( - Win32Message{message, virtual_key_, lparam, kWmResultDontCheck}); - if (is_printable_ && (kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { - pending_responds_.push_back( - Win32Message{WM_CHAR, virtual_key_, lparam, kWmResultDontCheck}); - } - return 1; - } - - std::vector pending_responds_; - WPARAM virtual_key_; - bool is_printable_; }; // The static value to return as the "handled" value from the framework for key @@ -335,11 +341,11 @@ TEST(FlutterWindowWin32Test, NonPrintableKeyDownPropagation) { constexpr WPARAM virtual_key = VK_LEFT; constexpr WPARAM scan_code = 10; constexpr char32_t character = 0; - MockFlutterWindowWin32 win32window; + MockFlutterWindowWin32 win32window(virtual_key, false /* is_printable */); auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), virtual_key, false /* is_printable */); + std::move(window_binding_handler)); win32window.SetView(&flutter_windows_view); LPARAM lparam = CreateKeyEventLparam(scan_code, false, false); @@ -349,7 +355,7 @@ TEST(FlutterWindowWin32Test, NonPrintableKeyDownPropagation) { flutter_windows_view.SetEngine(std::move(GetTestEngine())); EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_KEYDOWN, character, - false /* extended */, _)) + false /* extended */, _, _)) .Times(2) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, @@ -359,7 +365,6 @@ TEST(FlutterWindowWin32Test, NonPrintableKeyDownPropagation) { EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_)).Times(0); win32window.InjectMessages(1, Win32Message{WM_KEYDOWN, virtual_key, lparam}); - flutter_windows_view.InjectPendingEvents(&win32window); } // Test an event handled by the framework @@ -367,7 +372,7 @@ TEST(FlutterWindowWin32Test, NonPrintableKeyDownPropagation) { test_response = true; EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_KEYDOWN, character, - false /* extended */, false /* PrevState */)) + false /* extended */, false /* PrevState */, _)) .Times(1) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, @@ -375,7 +380,6 @@ TEST(FlutterWindowWin32Test, NonPrintableKeyDownPropagation) { .Times(0); win32window.InjectMessages(1, Win32Message{WM_KEYDOWN, virtual_key, lparam}); - flutter_windows_view.InjectPendingEvents(&win32window); } } @@ -386,11 +390,11 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) { constexpr WPARAM virtual_key = VK_LEFT; constexpr WPARAM scan_code = 10; constexpr char32_t character = 0; - MockFlutterWindowWin32 win32window; + MockFlutterWindowWin32 win32window(virtual_key, false /* is_printable */); auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), virtual_key, false /* is_printable */); + std::move(window_binding_handler)); win32window.SetView(&flutter_windows_view); LPARAM lparam = CreateKeyEventLparam(scan_code, false, false); @@ -400,7 +404,7 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) { flutter_windows_view.SetEngine(std::move(GetTestEngine())); EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_SYSKEYDOWN, character, - false /* extended */, _)) + false /* extended */, _, _)) .Times(2) .RetiresOnSaturation(); // Syskey events are not redispatched, so TextInputPlugin, which relies on @@ -412,21 +416,19 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) { EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_)).Times(0); win32window.InjectMessages( 1, Win32Message{WM_SYSKEYDOWN, virtual_key, lparam, kWmResultDefault}); - flutter_windows_view.InjectPendingEvents(&win32window); } // Test an event handled by the framework { test_response = true; EXPECT_CALL(*flutter_windows_view.key_event_handler, - KeyboardHook(_, _, _, _, _, _)) + KeyboardHook(_, _, _, _, _, _, _)) .Times(0); EXPECT_CALL(*flutter_windows_view.text_input_plugin, KeyboardHook(_, _, _, _, _, _)) .Times(0); win32window.InjectMessages( 1, Win32Message{WM_SYSKEYDOWN, virtual_key, lparam, kWmResultDefault}); - flutter_windows_view.InjectPendingEvents(&win32window); } } @@ -440,11 +442,11 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) { constexpr WPARAM scan_code = 30; constexpr char32_t character = 65; - MockFlutterWindowWin32 win32window; + MockFlutterWindowWin32 win32window(virtual_key, true /* is_printable */); auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), virtual_key, true /* is_printable */); + std::move(window_binding_handler)); win32window.SetView(&flutter_windows_view); LPARAM lparam = CreateKeyEventLparam(scan_code, false, false); flutter_windows_view.SetEngine(std::move(GetTestEngine())); @@ -454,7 +456,7 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) { test_response = false; EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_KEYDOWN, character, - false, false)) + false, false, _)) .Times(2) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, @@ -466,7 +468,6 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) { .RetiresOnSaturation(); win32window.InjectMessages(2, Win32Message{WM_KEYDOWN, virtual_key, lparam}, Win32Message{WM_CHAR, virtual_key, lparam}); - flutter_windows_view.InjectPendingEvents(&win32window); } // Test an event handled by the framework @@ -474,7 +475,7 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) { test_response = true; EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_KEYDOWN, character, - false, false)) + false, false, _)) .Times(1) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, @@ -483,7 +484,6 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) { EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_)).Times(0); win32window.InjectMessages(2, Win32Message{WM_KEYDOWN, virtual_key, lparam}, Win32Message{WM_CHAR, virtual_key, lparam}); - flutter_windows_view.InjectPendingEvents(&win32window); } } @@ -494,11 +494,11 @@ TEST(FlutterWindowWin32Test, ModifierKeyDownPropagation) { constexpr WPARAM virtual_key = VK_LSHIFT; constexpr WPARAM scan_code = 0x2a; constexpr char32_t character = 0; - MockFlutterWindowWin32 win32window; + MockFlutterWindowWin32 win32window(virtual_key, false /* is_printable */); auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), virtual_key, false /* is_printable */); + std::move(window_binding_handler)); win32window.SetView(&flutter_windows_view); LPARAM lparam = CreateKeyEventLparam(scan_code, false, false); @@ -508,7 +508,7 @@ TEST(FlutterWindowWin32Test, ModifierKeyDownPropagation) { flutter_windows_view.SetEngine(std::move(GetTestEngine())); EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_KEYDOWN, character, - false /* extended */, false)) + false /* extended */, false, _)) .Times(2) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, @@ -518,7 +518,6 @@ TEST(FlutterWindowWin32Test, ModifierKeyDownPropagation) { EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_)).Times(0); EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), 0); - flutter_windows_view.InjectPendingEvents(&win32window); } // Test an event handled by the framework @@ -526,7 +525,7 @@ TEST(FlutterWindowWin32Test, ModifierKeyDownPropagation) { test_response = true; EXPECT_CALL(*flutter_windows_view.key_event_handler, KeyboardHook(virtual_key, scan_code, WM_KEYDOWN, character, - false /* extended */, false)) + false /* extended */, false, _)) .Times(1) .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, @@ -534,7 +533,6 @@ TEST(FlutterWindowWin32Test, ModifierKeyDownPropagation) { .Times(0); EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), 0); - flutter_windows_view.InjectPendingEvents(&win32window); } } diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index e4fe3b287de08..72be7f43bb534 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -70,18 +70,8 @@ void FlutterWindowsView::SetEngine( std::unique_ptr FlutterWindowsView::CreateKeyboardKeyHandler( BinaryMessenger* messenger, - KeyboardKeyHandler::EventDispatcher dispatch_event, KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state) { - // There must be only one handler that receives |SendInput|, i.e. only one - // handler that might redispatch events. (See the documentation of - // |KeyboardKeyHandler| to learn about redispatching.) - // - // Whether an event is a redispatched event is decided by calculating the hash - // of the event. In order to allow the same real event in the future, the - // handler is "toggled" when events pass through, therefore the redispatching - // algorithm does not allow more than 1 handler that takes |SendInput|. - auto keyboard_key_handler = - std::make_unique(dispatch_event); + auto keyboard_key_handler = std::make_unique(); keyboard_key_handler->AddDelegate( std::make_unique( [this](const FlutterKeyEvent& event, FlutterKeyEventCallback callback, @@ -207,13 +197,14 @@ void FlutterWindowsView::OnText(const std::u16string& text) { SendText(text); } -bool FlutterWindowsView::OnKey(int key, +void FlutterWindowsView::OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) { - return SendKey(key, scancode, action, character, extended, was_down); + bool was_down, + KeyEventCallback callback) { + SendKey(key, scancode, action, character, extended, was_down, callback); } void FlutterWindowsView::OnComposeBegin() { @@ -267,14 +258,12 @@ void FlutterWindowsView::OnResetImeComposing() { void FlutterWindowsView::InitializeKeyboard() { auto internal_plugin_messenger = internal_plugin_registrar_->messenger(); #ifdef WINUWP - KeyboardKeyHandler::EventDispatcher dispatch_event = nullptr; KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state = nullptr; #else - KeyboardKeyHandler::EventDispatcher dispatch_event = SendInput; KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state = GetKeyState; #endif keyboard_key_handler_ = std::move(CreateKeyboardKeyHandler( - internal_plugin_messenger, dispatch_event, get_key_state)); + internal_plugin_messenger, get_key_state)); text_input_plugin_ = std::move(CreateTextInputPlugin(internal_plugin_messenger)); } @@ -383,21 +372,21 @@ void FlutterWindowsView::SendText(const std::u16string& text) { text_input_plugin_->TextHook(text); } -bool FlutterWindowsView::SendKey(int key, +void FlutterWindowsView::SendKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) { - if (keyboard_key_handler_->KeyboardHook(key, scancode, action, character, - extended, was_down)) { - return true; - } - - text_input_plugin_->KeyboardHook(key, scancode, action, character, extended, - was_down); - - return false; + bool was_down, + KeyEventCallback callback) { + keyboard_key_handler_->KeyboardHook( + key, scancode, action, character, extended, was_down, [&](bool handled) { + if (!handled) { + text_input_plugin_->KeyboardHook(key, scancode, action, character, + extended, was_down); + } + return handled; + }); } void FlutterWindowsView::SendComposeBegin() { diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index 4a9b9a4c15558..e0374a7d1a094 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -127,12 +127,13 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, void OnText(const std::u16string&) override; // |WindowBindingHandlerDelegate| - bool OnKey(int key, + void OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) override; + bool was_down, + KeyEventCallback callback) override; // |WindowBindingHandlerDelegate| void OnComposeBegin() override; @@ -179,7 +180,6 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, // functions in unit tests. virtual std::unique_ptr CreateKeyboardKeyHandler( BinaryMessenger* messenger, - KeyboardKeyHandler::EventDispatcher dispatch_event, KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state); // Called to create text input plugin. @@ -251,12 +251,13 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, void SendText(const std::u16string&); // Reports a raw keyboard message to Flutter engine. - bool SendKey(int key, + void SendKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down); + bool was_down, + KeyEventCallback callback); // Reports an IME compose begin event. // diff --git a/shell/platform/windows/flutter_windows_view_unittests.cc b/shell/platform/windows/flutter_windows_view_unittests.cc index fdd3dc2150b64..073cbc1dbaa6d 100644 --- a/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/shell/platform/windows/flutter_windows_view_unittests.cc @@ -97,7 +97,8 @@ TEST(FlutterWindowsViewTest, KeySequence) { FlutterWindowsView view(std::move(window_binding_handler)); view.SetEngine(std::move(engine)); - view.OnKey(kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false); + view.OnKey(kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false, + [](bool handled) {}); EXPECT_EQ(key_event_logs.size(), 2); EXPECT_EQ(key_event_logs[0], kKeyEventFromEmbedder); @@ -118,7 +119,8 @@ TEST(FlutterWindowsViewTest, RestartClearsKeyboardState) { // Receives a KeyA down. Events are dispatched and decided unhandled. Now the // keyboard key handler is waiting for the redispatched event. - view.OnKey(kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false); + view.OnKey(kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false, + [](bool handled) {}); EXPECT_EQ(key_event_logs.size(), 2); EXPECT_EQ(key_event_logs[0], kKeyEventFromEmbedder); EXPECT_EQ(key_event_logs[1], kKeyEventFromChannel); @@ -129,7 +131,8 @@ TEST(FlutterWindowsViewTest, RestartClearsKeyboardState) { // Receives another KeyA down. If the state had not been cleared, this event // will be considered the redispatched event and ignored. - view.OnKey(kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false); + view.OnKey(kVirtualKeyA, kScanCodeKeyA, WM_KEYDOWN, 'a', false, false, + [](bool handled) {}); EXPECT_EQ(key_event_logs.size(), 2); EXPECT_EQ(key_event_logs[0], kKeyEventFromEmbedder); EXPECT_EQ(key_event_logs[1], kKeyEventFromChannel); diff --git a/shell/platform/windows/keyboard_handler_base.h b/shell/platform/windows/keyboard_handler_base.h index 4e7b5c44ba4a7..d27ae1a67f872 100644 --- a/shell/platform/windows/keyboard_handler_base.h +++ b/shell/platform/windows/keyboard_handler_base.h @@ -17,18 +17,18 @@ namespace flutter { // are called on each handler. class KeyboardHandlerBase { public: + using KeyEventCallback = std::function; + virtual ~KeyboardHandlerBase() = default; // A function for hooking into keyboard input. - // - // Returns true if the key event has been handled, to indicate that other - // handlers should not be called for this event. - virtual bool KeyboardHook(int key, + virtual void KeyboardHook(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) = 0; + bool was_down, + KeyEventCallback callback) = 0; }; } // namespace flutter diff --git a/shell/platform/windows/keyboard_key_handler.cc b/shell/platform/windows/keyboard_key_handler.cc index 3ae7d988f19d5..4439af700736c 100644 --- a/shell/platform/windows/keyboard_key_handler.cc +++ b/shell/platform/windows/keyboard_key_handler.cc @@ -8,7 +8,6 @@ #include -#include "flutter/shell/platform/common/json_message_codec.h" #include "flutter/shell/platform/windows/keyboard_win32_common.h" namespace flutter { @@ -19,93 +18,12 @@ namespace { // emitting a warning on the console about unhandled events. static constexpr int kMaxPendingEvents = 1000; -// Returns if a character sent by Win32 is a dead key. -bool _IsDeadKey(uint32_t ch) { - return (ch & kDeadKeyCharMask) != 0; -} - -// Returns true if this key is a key down event of ShiftRight. -// -// This is a temporary solution to -// https://github.com/flutter/flutter/issues/81674, and forces ShiftRight -// KeyDown events to not be redispatched regardless of the framework's response. -// -// If a ShiftRight KeyDown event is not handled by the framework and is -// redispatched, Win32 will not send its following KeyUp event and keeps -// recording ShiftRight as being pressed. -static bool IsKeyDownShiftRight(int virtual_key, bool was_down) { -#ifdef WINUWP - return false; -#else - return virtual_key == VK_RSHIFT && !was_down; -#endif -} - -// Returns true if this key is an AltRight key down event. -// -// This is used to resolve an issue where an AltGr press causes CtrlLeft to hang -// when pressed, as reported in https://github.com/flutter/flutter/issues/78005. -// -// When AltGr is pressed (in a supporting layout such as Spanish), Win32 first -// fires a fake CtrlLeft down event, then an AltRight down event. -// This is significant because this fake CtrlLeft down event will not be paired -// with a up event, which is fine until Flutter redispatches the CtrlDown -// event, which Win32 then interprets as a real event, leaving both Win32 and -// the Flutter framework thinking that CtrlLeft is still pressed. -// -// To resolve this, Flutter recognizes this fake CtrlLeft down event using the -// following AltRight down event. Flutter then synthesizes a CtrlLeft key up -// event immediately after the corresponding AltRight key up event. -// -// One catch is that it is impossible to distinguish the fake CtrlLeft down -// from a normal CtrlLeft down (followed by a AltRight down), since they -// contain the exactly same information, including the GetKeyState result. -// Fortunately, this will require the two events to occur *really* close, which -// would be rare, and a misrecognition would only cause a minor consequence -// where the CtrlLeft is released early; the later, real, CtrlLeft up event will -// be ignored. -static bool IsKeyDownAltRight(int action, int virtual_key, bool extended) { -#ifdef WINUWP - return false; -#else - return virtual_key == VK_RMENU && extended && - (action == WM_KEYDOWN || action == WM_SYSKEYDOWN); -#endif -} - -// Returns true if this key is a key up event of AltRight. -// -// This is used to assist a corner case described in |IsKeyDownAltRight|. -static bool IsKeyUpAltRight(int action, int virtual_key, bool extended) { -#ifdef WINUWP - return false; -#else - return virtual_key == VK_RMENU && extended && - (action == WM_KEYUP || action == WM_SYSKEYUP); -#endif -} - -// Returns true if this key is a key down event of CtrlLeft. -// -// This is used to assist a corner case described in |IsKeyDownAltRight|. -static bool IsKeyDownCtrlLeft(int action, int virtual_key) { -#ifdef WINUWP - return false; -#else - return virtual_key == VK_LCONTROL && - (action == WM_KEYDOWN || action == WM_SYSKEYDOWN); -#endif -} - } // namespace KeyboardKeyHandler::KeyboardKeyHandlerDelegate::~KeyboardKeyHandlerDelegate() = default; -KeyboardKeyHandler::KeyboardKeyHandler(EventDispatcher dispatch_event) - : dispatch_event_(dispatch_event), - last_sequence_id_(1), - last_key_is_ctrl_left_down(false) {} +KeyboardKeyHandler::KeyboardKeyHandler() : last_sequence_id_(1) {} KeyboardKeyHandler::~KeyboardKeyHandler() = default; @@ -114,112 +32,20 @@ void KeyboardKeyHandler::AddDelegate( delegates_.push_back(std::move(delegate)); } -size_t KeyboardKeyHandler::RedispatchedCount() { - return pending_redispatches_.size(); -} - -void KeyboardKeyHandler::DispatchEvent(const PendingEvent& event) { - // TODO(dkwingsmt) consider adding support for dispatching events for UWP - // in order to support add-to-app. - // https://github.com/flutter/flutter/issues/70202 -#ifdef WINUWP - return; -#else - char32_t character = event.character; - - 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, - .ki = - KEYBDINPUT{ - .wVk = static_cast(event.key), - .wScan = static_cast(event.scancode), - .dwFlags = static_cast( - KEYEVENTF_SCANCODE | - (event.extended ? KEYEVENTF_EXTENDEDKEY : 0x0) | - (event.action == WM_KEYUP ? KEYEVENTF_KEYUP : 0x0)), - }, - }; - - UINT accepted = dispatch_event_(1, &input_event, sizeof(input_event)); - if (accepted != 1) { - std::cerr << "Unable to synthesize event for keyboard event with scancode " - << event.scancode; - if (character != 0) { - std::cerr << " (character " << character << ")"; - } - std::cerr << std::endl; - ; - } -#endif -} - -void KeyboardKeyHandler::RedispatchEvent(std::unique_ptr event) { -#ifdef WINUWP - return; -#else - DispatchEvent(*event); - pending_redispatches_.push_back(std::move(event)); -#endif -} - -bool KeyboardKeyHandler::KeyboardHook(int key, +void KeyboardKeyHandler::KeyboardHook(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) { - std::unique_ptr incoming = - std::make_unique(PendingEvent{ - .key = static_cast(key), - .scancode = static_cast(scancode), - .action = static_cast(action), - .character = character, - .extended = extended, - .was_down = was_down, - }); - incoming->hash = ComputeEventHash(*incoming); - - if (RemoveRedispatchedEvent(*incoming)) { - return false; - } - - if (IsKeyDownAltRight(action, key, extended)) { - if (last_key_is_ctrl_left_down) { - should_synthesize_ctrl_left_up = true; - } - } - if (IsKeyDownCtrlLeft(action, key)) { - last_key_is_ctrl_left_down = true; - ctrl_left_scancode = scancode; - should_synthesize_ctrl_left_up = false; - } else { - last_key_is_ctrl_left_down = false; - } - if (IsKeyUpAltRight(action, key, extended)) { - if (should_synthesize_ctrl_left_up) { - should_synthesize_ctrl_left_up = false; - PendingEvent ctrl_left_up{ - .key = VK_LCONTROL, - .scancode = ctrl_left_scancode, - .action = WM_KEYUP, - .was_down = true, - }; - DispatchEvent(ctrl_left_up); - } - } + bool was_down, + KeyEventCallback callback) { + std::unique_ptr incoming = std::make_unique(); uint64_t sequence_id = ++last_sequence_id_; incoming->sequence_id = sequence_id; incoming->unreplied = delegates_.size(); - // There are a few situations where events must not be redispatched. - // Initializing `any_handled` with true in such cases suffices, since it can - // only be set to true and is used to disable redispatching. - const bool must_not_redispatch = IsKeyDownShiftRight(key, was_down); - incoming->any_handled = must_not_redispatch; + incoming->any_handled = false; + incoming->callback = std::move(callback); if (pending_responds_.size() > kMaxPendingEvents) { std::cerr @@ -241,19 +67,6 @@ bool KeyboardKeyHandler::KeyboardHook(int key, // make events out of order though, because |KeyboardHook| will always // return true at this time, preventing this event from affecting // others. - - return true; -} - -bool KeyboardKeyHandler::RemoveRedispatchedEvent(const PendingEvent& incoming) { - for (auto iter = pending_redispatches_.begin(); - iter != pending_redispatches_.end(); ++iter) { - if ((*iter)->hash == incoming.hash) { - pending_redispatches_.erase(iter); - return true; - } - } - return false; } void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, @@ -266,25 +79,11 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, event.any_handled = event.any_handled || handled; event.unreplied -= 1; assert(event.unreplied >= 0); - // If all delegates have replied, redispatch if no one handled. + // If all delegates have replied, report if any of them handled the event. if (event.unreplied == 0) { std::unique_ptr event_ptr = std::move(*iter); pending_responds_.erase(iter); - // 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. 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)); - } + event.callback(handled); } // Return here; |iter| can't do ++ after erase. return; @@ -294,12 +93,4 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, assert(false); } -uint64_t KeyboardKeyHandler::ComputeEventHash(const PendingEvent& event) { - // Calculate a key event ID based on the scan code of the key pressed, - // and the flags we care about. - return event.scancode | (((event.action == WM_KEYUP ? KEYEVENTF_KEYUP : 0x0) | - (event.extended ? KEYEVENTF_EXTENDEDKEY : 0x0)) - << 16); -} - } // namespace flutter diff --git a/shell/platform/windows/keyboard_key_handler.h b/shell/platform/windows/keyboard_key_handler.h index 0f1217815d754..63a5d99d67cc6 100644 --- a/shell/platform/windows/keyboard_key_handler.h +++ b/shell/platform/windows/keyboard_key_handler.h @@ -45,19 +45,13 @@ class KeyboardKeyHandler : public KeyboardHandlerBase { char32_t character, bool extended, bool was_down, - std::function callback) = 0; + KeyEventCallback callback) = 0; virtual ~KeyboardKeyHandlerDelegate(); }; - using EventDispatcher = - std::function; - - // Create a KeyboardKeyHandler and specify where to redispatch events. - // - // The |redispatch_event| is typically |SendInput|, but can also be nullptr - // (for UWP). - explicit KeyboardKeyHandler(EventDispatcher dispatch_event); + // Create a KeyboardKeyHandler. + explicit KeyboardKeyHandler(); ~KeyboardKeyHandler(); @@ -89,25 +83,19 @@ class KeyboardKeyHandler : public KeyboardHandlerBase { // after which the channel delegate should be removed. // // Inherited from |KeyboardHandlerBase|. - bool KeyboardHook(int key, + void KeyboardHook(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) override; + bool was_down, + KeyEventCallback callback) override; protected: size_t RedispatchedCount(); private: struct PendingEvent { - uint32_t key; - uint8_t scancode; - uint32_t action; - char32_t character; - bool extended; - bool was_down; - // Self-incrementing ID attached to an event sent to the framework. uint64_t sequence_id; // The number of delegates that haven't replied. @@ -115,68 +103,22 @@ class KeyboardKeyHandler : public KeyboardHandlerBase { // Whether any replied delegates reported true (handled). bool any_handled; - // A value calculated out of critical event information that can be used - // to identify redispatched events. - uint64_t hash; + // Where to report the delegates' result to. + // + // Typically a callback provided by KeyboardManager32. + KeyEventCallback callback; }; - void DispatchEvent(const PendingEvent& event); - - // Find an event in the redispatch list that matches the given one. - // - // If an matching event is found, removes the matching event from the - // redispatch list, and returns true. Otherwise, returns false; - bool RemoveRedispatchedEvent(const PendingEvent& incoming); - void RedispatchEvent(std::unique_ptr event); void ResolvePendingEvent(uint64_t sequence_id, bool handled); - std::vector> delegates_; - // The queue of key events that have been sent to the framework but have not // yet received a response. std::deque> pending_responds_; - // The queue of key events that have been redispatched to the system but have - // not yet been received for a second time. - std::deque> pending_redispatches_; - // The sequence_id attached to the last event sent to the framework. uint64_t last_sequence_id_; - // The callback used to dispatch synthesized events. - EventDispatcher dispatch_event_; - - // Whether the last event is a CtrlLeft key down. - // - // This is used to resolve a corner case described in |IsKeyDownAltRight|. - bool last_key_is_ctrl_left_down; - - // The scancode of the last met CtrlLeft down. - // - // This is used to resolve a corner case described in |IsKeyDownAltRight|. - uint8_t ctrl_left_scancode; - - // Whether a CtrlLeft up should be synthesized upon the next AltRight up. - // - // This is used to resolve a corner case described in |IsKeyDownAltRight|. - bool should_synthesize_ctrl_left_up; - - // Calculate a hash based on event data for fast comparison for a redispatched - // event. - // - // This uses event data instead of generating a serial number because - // information can't be attached to the redispatched events, so it has to be - // possible to compute an ID from the identifying data in the event when it is - // received again in order to differentiate between events that are new, and - // events that have been redispatched. - // - // Another alternative would be to compute a checksum from all the data in the - // event (just compute it over the bytes in the struct, probably skipping - // timestamps), but the fields used are enough to differentiate them, and - // since Windows does some processing on the events (coming up with virtual - // key codes, setting timestamps, etc.), it's not clear that the redispatched - // events would have the same checksums. - static uint64_t ComputeEventHash(const PendingEvent& event); + std::vector> delegates_; }; } // namespace flutter diff --git a/shell/platform/windows/keyboard_key_handler_unittests.cc b/shell/platform/windows/keyboard_key_handler_unittests.cc index c41cfbe61bbe2..1689607c67612 100644 --- a/shell/platform/windows/keyboard_key_handler_unittests.cc +++ b/shell/platform/windows/keyboard_key_handler_unittests.cc @@ -94,27 +94,27 @@ class MockKeyHandlerDelegate class TestKeyboardKeyHandler : public KeyboardKeyHandler { public: - explicit TestKeyboardKeyHandler(EventDispatcher redispatch_event) - : KeyboardKeyHandler(redispatch_event) {} + explicit TestKeyboardKeyHandler() : KeyboardKeyHandler() {} +}; - bool HasRedispatched() { return RedispatchedCount() > 0; } +enum KeyEventResponse { + kNoResponse, + kHandled, + kUnhandled, }; +static KeyEventResponse key_event_response = kNoResponse; + +void OnKeyEventResult(bool handled) { + key_event_response = handled ? kHandled : kUnhandled; +} + } // namespace TEST(KeyboardKeyHandlerTest, SingleDelegateWithAsyncResponds) { std::list hook_history; - // Capture the scancode of the last redispatched event - int redispatch_scancode = 0; - bool delegate_handled = false; - TestKeyboardKeyHandler handler([&redispatch_scancode](UINT cInputs, - LPINPUT pInputs, - int cbSize) -> UINT { - EXPECT_TRUE(cbSize > 0); - redispatch_scancode = pInputs->ki.wScan; - return 1; - }); + TestKeyboardKeyHandler handler; // Add one delegate auto delegate = std::make_unique(1, &hook_history); handler.AddDelegate(std::move(delegate)); @@ -122,38 +122,35 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithAsyncResponds) { /// Test 1: One event that is handled by the framework // Dispatch a key event - delegate_handled = - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true, + OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 1); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, true); - EXPECT_EQ(handler.HasRedispatched(), false); + EXPECT_EQ(key_event_response, kNoResponse); hook_history.back().callback(true); - EXPECT_EQ(redispatch_scancode, 0); + EXPECT_EQ(key_event_response, kHandled); - EXPECT_EQ(handler.HasRedispatched(), false); + key_event_response = kNoResponse; hook_history.clear(); /// Test 2: Two events that are unhandled by the framework - delegate_handled = handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, + OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 1); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); // Dispatch another key event - delegate_handled = - handler.KeyboardHook(65, kHandledScanCode2, WM_KEYUP, L'b', false, true); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(65, kHandledScanCode2, WM_KEYUP, L'b', false, true, + OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 2); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode2); @@ -161,37 +158,21 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithAsyncResponds) { // Resolve the second event first to test out-of-order response hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kHandledScanCode2); + EXPECT_EQ(key_event_response, kUnhandled); + key_event_response = kNoResponse; // Resolve the first event then hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kHandledScanCode); - - EXPECT_EQ(handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, - false), - false); - EXPECT_EQ( - handler.KeyboardHook(65, kHandledScanCode2, WM_KEYUP, L'b', false, false), - false); + EXPECT_EQ(key_event_response, kUnhandled); - EXPECT_EQ(handler.HasRedispatched(), false); hook_history.clear(); - redispatch_scancode = 0; + key_event_response = kNoResponse; } TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { std::list hook_history; - // Capture the scancode of the last redispatched event - int redispatch_scancode = 0; - bool delegate_handled = false; - TestKeyboardKeyHandler handler([&redispatch_scancode](UINT cInputs, - LPINPUT pInputs, - int cbSize) -> UINT { - EXPECT_TRUE(cbSize > 0); - redispatch_scancode = pInputs->ki.wScan; - return 1; - }); + TestKeyboardKeyHandler handler; // Add one delegate auto delegate = std::make_unique(1, &hook_history); CallbackHandler& delegate_handler = delegate->callback_handler; @@ -201,55 +182,35 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { // Dispatch a key event delegate_handler = respond_true; - delegate_handled = handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, + OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 1); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); - EXPECT_EQ(handler.HasRedispatched(), false); hook_history.clear(); /// Test 2: An event unhandled by the framework delegate_handler = respond_false; - delegate_handled = handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, kHandledScanCode); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, + OnKeyEventResult); + EXPECT_EQ(key_event_response, kHandled); EXPECT_EQ(hook_history.size(), 1); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); - EXPECT_EQ(handler.HasRedispatched(), true); - - // Resolve the event - EXPECT_EQ(handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, - false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); hook_history.clear(); - redispatch_scancode = 0; + key_event_response = kUnhandled; } TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { std::list hook_history; - // Capture the scancode of the last redispatched event - int redispatch_scancode = 0; - bool delegate_handled = false; - TestKeyboardKeyHandler handler([&redispatch_scancode](UINT cInputs, - LPINPUT pInputs, - int cbSize) -> UINT { - EXPECT_TRUE(cbSize > 0); - redispatch_scancode = pInputs->ki.wScan; - return 1; - }); + TestKeyboardKeyHandler handler; auto delegate1 = std::make_unique(1, &hook_history); CallbackHandler& delegate1_handler = delegate1->callback_handler; @@ -261,10 +222,9 @@ TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { /// Test 1: One delegate responds true, the other false - delegate_handled = handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, + OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 2); EXPECT_EQ(hook_history.front().delegate_id, 1); EXPECT_EQ(hook_history.front().scancode, kHandledScanCode); @@ -273,24 +233,19 @@ TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); - EXPECT_EQ(handler.HasRedispatched(), false); - hook_history.back().callback(true); - EXPECT_EQ(redispatch_scancode, 0); + EXPECT_EQ(key_event_response, kHandled); hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, 0); + EXPECT_EQ(key_event_response, kUnhandled); - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; hook_history.clear(); /// Test 2: All delegates respond false - delegate_handled = handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, + L'a', false, false, OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 2); EXPECT_EQ(hook_history.front().delegate_id, 1); EXPECT_EQ(hook_history.front().scancode, kHandledScanCode); @@ -299,29 +254,20 @@ TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); - EXPECT_EQ(handler.HasRedispatched(), false); - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, 0); + EXPECT_EQ(key_event_response, kNoResponse); hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kHandledScanCode); + EXPECT_EQ(key_event_response, kUnhandled); - EXPECT_EQ(handler.HasRedispatched(), true); - EXPECT_EQ(handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, - false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); hook_history.clear(); - redispatch_scancode = 0; + key_event_response = kNoResponse; /// Test 3: All delegates responds true - delegate_handled = handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, + L'a', false, false, OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 2); EXPECT_EQ(hook_history.front().delegate_id, 1); EXPECT_EQ(hook_history.front().scancode, kHandledScanCode); @@ -330,18 +276,13 @@ TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); - EXPECT_EQ(handler.HasRedispatched(), false); - hook_history.back().callback(true); - EXPECT_EQ(redispatch_scancode, 0); - // Only resolve after everyone has responded - EXPECT_EQ(handler.HasRedispatched(), false); + EXPECT_EQ(key_event_response, kNoResponse); hook_history.front().callback(true); - EXPECT_EQ(redispatch_scancode, 0); + EXPECT_EQ(key_event_response, kHandled); - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; + key_event_response = kNoResponse; hook_history.clear(); } @@ -354,60 +295,32 @@ TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { TEST(KeyboardKeyHandlerTest, WithSlowFrameworkResponse) { std::list hook_history; - // Capture the scancode of the last redispatched event - int redispatch_scancode = 0; - bool delegate_handled = false; - TestKeyboardKeyHandler handler([&redispatch_scancode](UINT cInputs, - LPINPUT pInputs, - int cbSize) -> UINT { - EXPECT_TRUE(cbSize > 0); - redispatch_scancode = pInputs->ki.wScan; - return 1; - }); + TestKeyboardKeyHandler handler; auto delegate1 = std::make_unique(1, &hook_history); CallbackHandler& delegate1_handler = delegate1->callback_handler; handler.AddDelegate(std::move(delegate1)); // The first native event. - EXPECT_EQ( - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true), - true); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true, OnKeyEventResult); // The second identical native event, received between the first and its // framework response. - EXPECT_EQ( - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true), - true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true, OnKeyEventResult); + EXPECT_EQ(key_event_response, kNoResponse); EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(handler.HasRedispatched(), false); - // The first response. hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kHandledScanCode); - EXPECT_EQ(handler.HasRedispatched(), true); + EXPECT_EQ(key_event_response, kUnhandled); - // Redispatch the first event. - EXPECT_EQ(handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, - false), - false); - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; + key_event_response = kNoResponse; // The second response. hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kHandledScanCode); - EXPECT_EQ(handler.HasRedispatched(), true); + EXPECT_EQ(key_event_response, kUnhandled); - // Redispatch the second event. - EXPECT_EQ(handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, - false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; + key_event_response = kNoResponse; hook_history.clear(); } @@ -418,16 +331,7 @@ TEST(KeyboardKeyHandlerTest, WithSlowFrameworkResponse) { TEST(KeyboardKeyHandlerTest, NeverRedispatchShiftRightKeyDown) { std::list hook_history; - // Capture the scancode of the last redispatched event - int redispatch_scancode = 0; - bool delegate_handled = false; - TestKeyboardKeyHandler handler([&redispatch_scancode](UINT cInputs, - LPINPUT pInputs, - int cbSize) -> UINT { - EXPECT_TRUE(cbSize > 0); - redispatch_scancode = pInputs->ki.wScan; - return 1; - }); + TestKeyboardKeyHandler handler; auto delegate = std::make_unique(1, &hook_history); delegate->callback_handler = respond_false; @@ -435,296 +339,11 @@ TEST(KeyboardKeyHandlerTest, NeverRedispatchShiftRightKeyDown) { // Press ShiftRight and the delegate responds false. - delegate_handled = handler.KeyboardHook(VK_RSHIFT, kScanCodeShiftRight, - WM_KEYDOWN, 0, false, false); - EXPECT_EQ(delegate_handled, true); - EXPECT_EQ(redispatch_scancode, 0); - EXPECT_EQ(hook_history.size(), 1); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; - hook_history.clear(); -} - -TEST(KeyboardKeyHandlerTest, AltGr) { - std::list hook_history; - - // Capture the scancode of the last redispatched event - int redispatch_scancode = 0; - TestKeyboardKeyHandler handler([&redispatch_scancode](UINT cInputs, - LPINPUT pInputs, - int cbSize) -> UINT { - EXPECT_TRUE(cbSize > 0); - redispatch_scancode = pInputs->ki.wScan; - return 1; - }); - - auto delegate = std::make_unique(1, &hook_history); - delegate->callback_handler = dont_respond; - handler.AddDelegate(std::move(delegate)); - - // Sequence 1: Tap AltGr. - - // The key down event causes a ControlLeft down and a AltRight (extended - // AltLeft) down. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - true); - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - true); - EXPECT_EQ(redispatch_scancode, 0); - EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(hook_history.front().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.front().was_down, false); - EXPECT_EQ(hook_history.back().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.back().was_down, false); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - false); - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; - hook_history.clear(); - - // The key up event only causes a AltRight (extended AltLeft) up. - EXPECT_EQ( - handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYUP, 0, true, true), - true); - EXPECT_EQ(hook_history.size(), 1); - EXPECT_EQ(hook_history.front().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.front().was_down, true); - - // A ControlLeft key up is synthesized. - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - true); - - EXPECT_EQ(hook_history.back().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.back().was_down, true); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - false); - EXPECT_EQ( - handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYUP, 0, true, true), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; - hook_history.clear(); - - // Sequence 2: Tap CtrlLeft and AltGr. - // This also tests tapping AltGr twice in a row when combined with sequence - // 1 since "tapping CtrlLeft and AltGr" only sends an extra CtrlLeft key up - // than "tapping AltGr". - - // Key down ControlLeft. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - true); - EXPECT_EQ(redispatch_scancode, 0); - EXPECT_EQ(hook_history.size(), 1); - EXPECT_EQ(hook_history.front().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.front().was_down, false); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - hook_history.clear(); - redispatch_scancode = 0; - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - false); - - // Key down AltRight. - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - true); - EXPECT_EQ(redispatch_scancode, 0); - EXPECT_EQ(hook_history.size(), 1); - EXPECT_EQ(hook_history.front().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.front().was_down, false); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - hook_history.clear(); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - false); - - redispatch_scancode = 0; - - // Key up AltRight. - EXPECT_EQ( - handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYUP, 0, true, true), - true); - EXPECT_EQ(hook_history.size(), 1); - EXPECT_EQ(hook_history.front().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.front().was_down, true); - - // A ControlLeft key up is synthesized. - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - true); - - EXPECT_EQ(hook_history.back().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.back().was_down, true); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - false); - EXPECT_EQ( - handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYUP, 0, true, true), - false); - - hook_history.clear(); - redispatch_scancode = 0; - - // Key up ControlLeft should be dispatched to delegates, but will be properly - // handled by delegates' logic. - - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - true); - EXPECT_EQ(redispatch_scancode, 0); + handler.KeyboardHook(VK_RSHIFT, kScanCodeShiftRight, WM_KEYDOWN, 0, false, + false, OnKeyEventResult); + EXPECT_EQ(key_event_response, kHandled); EXPECT_EQ(hook_history.size(), 1); - EXPECT_EQ(hook_history.front().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.front().was_down, true); - - hook_history.front().callback(true); - EXPECT_EQ(redispatch_scancode, 0); - hook_history.clear(); - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; - hook_history.clear(); - - // Sequence 3: Hold AltGr for repeated events. - // Every AltGr key repeat event is also preceded by a ControlLeft down - // (repeat). - - // Key down AltRight. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - true); - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - true); - EXPECT_EQ(redispatch_scancode, 0); - EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(hook_history.front().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.front().was_down, false); - EXPECT_EQ(hook_history.back().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.back().was_down, false); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - false); - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; - hook_history.clear(); - - // Another key down AltRight. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, true), - true); - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, true), - true); - EXPECT_EQ(redispatch_scancode, 0); - EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(hook_history.front().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.front().was_down, true); - EXPECT_EQ(hook_history.back().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.back().was_down, true); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYDOWN, - 0, false, false), - false); - EXPECT_EQ(handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYDOWN, 0, - true, false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; - hook_history.clear(); - - // Key up AltRight. - EXPECT_EQ( - handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYUP, 0, true, true), - true); - EXPECT_EQ(hook_history.size(), 1); - EXPECT_EQ(hook_history.front().scancode, kScanCodeAltLeft); - EXPECT_EQ(hook_history.front().was_down, true); - - // A ControlLeft key up is synthesized. - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - true); - - EXPECT_EQ(hook_history.back().scancode, kScanCodeControlLeft); - EXPECT_EQ(hook_history.back().was_down, true); - - hook_history.front().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeAltLeft); - hook_history.back().callback(false); - EXPECT_EQ(redispatch_scancode, kScanCodeControlLeft); - - // Resolve redispatches. - EXPECT_EQ(handler.KeyboardHook(VK_LCONTROL, kScanCodeControlLeft, WM_KEYUP, 0, - false, true), - false); - EXPECT_EQ( - handler.KeyboardHook(VK_RMENU, kScanCodeAltLeft, WM_KEYUP, 0, true, true), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); - redispatch_scancode = 0; hook_history.clear(); } diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 4bb9e522bdb63..940c93378ac79 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include +#include +#include #include #include "keyboard_manager_win32.h" @@ -11,6 +13,90 @@ namespace flutter { +namespace { + +// The maximum number of pending events to keep before +// emitting a warning on the console about unhandled events. +static constexpr int kMaxPendingEvents = 1000; + +// Returns true if this key is an AltRight key down event. +// +// This is used to resolve an issue where an AltGr press causes CtrlLeft to hang +// when pressed, as reported in https://github.com/flutter/flutter/issues/78005. +// +// When AltGr is pressed (in a supporting layout such as Spanish), Win32 first +// fires a fake CtrlLeft down event, then an AltRight down event. +// This is significant because this fake CtrlLeft down event will not be paired +// with a up event, which is fine until Flutter redispatches the CtrlDown +// event, which Win32 then interprets as a real event, leaving both Win32 and +// the Flutter framework thinking that CtrlLeft is still pressed. +// +// To resolve this, Flutter recognizes this fake CtrlLeft down event using the +// following AltRight down event. Flutter then synthesizes a CtrlLeft key up +// event immediately after the corresponding AltRight key up event. +// +// One catch is that it is impossible to distinguish the fake CtrlLeft down +// from a normal CtrlLeft down (followed by a AltRight down), since they +// contain the exactly same information, including the GetKeyState result. +// Fortunately, this will require the two events to occur *really* close, which +// would be rare, and a misrecognition would only cause a minor consequence +// where the CtrlLeft is released early; the later, real, CtrlLeft up event will +// be ignored. +static bool IsKeyDownAltRight(int action, int virtual_key, bool extended) { +#ifdef WINUWP + return false; +#else + return virtual_key == VK_RMENU && extended && + (action == WM_KEYDOWN || action == WM_SYSKEYDOWN); +#endif +} + +// Returns true if this key is a key up event of AltRight. +// +// This is used to assist a corner case described in |IsKeyDownAltRight|. +static bool IsKeyUpAltRight(int action, int virtual_key, bool extended) { +#ifdef WINUWP + return false; +#else + return virtual_key == VK_RMENU && extended && + (action == WM_KEYUP || action == WM_SYSKEYUP); +#endif +} + +// Returns true if this key is a key down event of CtrlLeft. +// +// This is used to assist a corner case described in |IsKeyDownAltRight|. +static bool IsKeyDownCtrlLeft(int action, int virtual_key) { +#ifdef WINUWP + return false; +#else + return virtual_key == VK_LCONTROL && + (action == WM_KEYDOWN || action == WM_SYSKEYDOWN); +#endif +} + +// Returns true if this key is a key down event of ShiftRight. +// +// This is a temporary solution to +// https://github.com/flutter/flutter/issues/81674, and forces ShiftRight +// KeyDown events to not be redispatched regardless of the framework's response. +// +// If a ShiftRight KeyDown event is not handled by the framework and is +// redispatched, Win32 will not send its following KeyUp event and keeps +// recording ShiftRight as being pressed. +static bool IsKeyDownShiftRight(int virtual_key, bool was_down) { +#ifdef WINUWP + return false; +#else + return virtual_key == VK_RSHIFT && !was_down; +#endif +} + +// Returns if a character sent by Win32 is a dead key. +static bool _IsDeadKey(uint32_t ch) { + return (ch & kDeadKeyCharMask) != 0; +} + static char32_t CodePointFromSurrogatePair(wchar_t high, wchar_t low) { return 0x10000 + ((static_cast(high) & 0x000003FF) << 10) + (low & 0x3FF); @@ -40,8 +126,142 @@ static bool IsPrintable(uint32_t c) { return c >= kMinPrintable && c != kDelete; } +} // namespace + KeyboardManagerWin32::KeyboardManagerWin32(WindowDelegate* delegate) - : window_delegate_(delegate) {} + : window_delegate_(delegate), + last_key_is_ctrl_left_down(false) {} + +void KeyboardManagerWin32::DispatchEvent(const PendingEvent& event) { + char32_t character = event.character; + + 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, + .ki = + KEYBDINPUT{ + .wVk = static_cast(event.key), + .wScan = static_cast(event.scancode), + .dwFlags = static_cast( + KEYEVENTF_SCANCODE | + (event.extended ? KEYEVENTF_EXTENDEDKEY : 0x0) | + (event.action == WM_KEYUP ? KEYEVENTF_KEYUP : 0x0)), + }, + }; + + UINT accepted = window_delegate_->Win32DispatchEvent(1, &input_event, sizeof(input_event)); + if (accepted != 1) { + std::cerr << "Unable to synthesize event for keyboard event with scancode " + << event.scancode; + if (character != 0) { + std::cerr << " (character " << character << ")"; + } + std::cerr << std::endl; + ; + } +} + +void KeyboardManagerWin32::RedispatchEvent(std::unique_ptr event) { +#ifdef WINUWP + return; +#else + DispatchEvent(*event); + if (pending_redispatches_.size() > kMaxPendingEvents) { + std::cerr + << "There are " << pending_redispatches_.size() + << " keyboard events that have not yet received a response from the " + << "framework. Are responses being sent?" << std::endl; + } + pending_redispatches_.push_back(std::move(event)); +#endif +} + +size_t KeyboardManagerWin32::RedispatchedCount() { + return pending_redispatches_.size(); +} + +bool KeyboardManagerWin32::RemoveRedispatchedEvent(const PendingEvent& incoming) { + for (auto iter = pending_redispatches_.begin(); + iter != pending_redispatches_.end(); ++iter) { + if ((*iter)->hash == incoming.hash) { + pending_redispatches_.erase(iter); + return true; + } + } + return false; +} + +bool KeyboardManagerWin32::OnKey(int key, + int scancode, + int action, + char32_t character, + bool extended, + bool was_down) { + std::unique_ptr incoming = + std::make_unique(PendingEvent{ + .key = static_cast(key), + .scancode = static_cast(scancode), + .action = static_cast(action), + .character = character, + .extended = extended, + .was_down = was_down, + }); + incoming->hash = ComputeEventHash(*incoming); + + if (RemoveRedispatchedEvent(*incoming)) { + return false; + } + + if (IsKeyDownAltRight(action, key, extended)) { + if (last_key_is_ctrl_left_down) { + should_synthesize_ctrl_left_up = true; + } + } + if (IsKeyDownCtrlLeft(action, key)) { + last_key_is_ctrl_left_down = true; + ctrl_left_scancode = scancode; + should_synthesize_ctrl_left_up = false; + } else { + last_key_is_ctrl_left_down = false; + } + if (IsKeyUpAltRight(action, key, extended)) { + if (should_synthesize_ctrl_left_up) { + should_synthesize_ctrl_left_up = false; + PendingEvent ctrl_left_up{ + .key = VK_LCONTROL, + .scancode = ctrl_left_scancode, + .action = WM_KEYUP, + .was_down = true, + }; + DispatchEvent(ctrl_left_up); + } + } + + window_delegate_->OnKey( + key, scancode, action, character, extended, was_down, + [this, event = incoming.release()](bool handled) { + // Always consider some key events as handled. + // + // Redispatching dead keys events makes Win32 ignore the dead key state + // and redispatches a normal character without combining it with the + // 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 real_handled = + handled || _IsDeadKey(event->character) || is_syskey + || IsKeyDownShiftRight(event->key, event->was_down); + if (!real_handled) { + RedispatchEvent(std::unique_ptr(event)); + } + }); + return true; +} bool KeyboardManagerWin32::HandleMessage(UINT const message, WPARAM const wparam, @@ -95,33 +315,34 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, } else { event_character = IsPrintable(code_point) ? code_point : 0; } - bool handled = window_delegate_->OnKey( + bool is_new_event = OnKey( keycode_for_char_message_, scancode, message == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, event_character, extended, was_down); + if (!is_new_event) { + break; + } keycode_for_char_message_ = 0; - if (handled) { - // If the OnKey handler handles the message, then return so we don't - // pass it to OnText, because handling the message indicates that - // OnKey either just sent it to the framework to be processed. - // - // This message will be redispatched if not handled by the framework, - // during which the OnText (below) might be reached. However, if the - // original message was preceded by dead chars (such as ^ and e - // yielding ê), then since the redispatched message is no longer - // preceded by the dead char, the text will be wrong. Therefore we - // record the text here for the redispached event to use. - if (message == WM_CHAR) { - text_for_scancode_on_redispatch_[scancode] = text; - } - - // For system characters, always pass them to the default WndProc so - // that system keys like the ALT-TAB are processed correctly. - if (message == WM_SYSCHAR) { - break; - } - return true; + // If the OnKey handler handles the message, then return so we don't + // pass it to OnText, because handling the message indicates that + // OnKey either just sent it to the framework to be processed. + // + // This message will be redispatched if not handled by the framework, + // during which the OnText (below) might be reached. However, if the + // original message was preceded by dead chars (such as ^ and e + // yielding ê), then since the redispatched message is no longer + // preceded by the dead char, the text will be wrong. Therefore we + // record the text here for the redispached event to use. + if (message == WM_CHAR) { + text_for_scancode_on_redispatch_[scancode] = text; } + + // For system characters, always pass them to the default WndProc so + // that system keys like the ALT-TAB are processed correctly. + if (message == WM_SYSCHAR) { + break; + } + return true; } // Of the messages handled here, only WM_CHAR should be treated as @@ -174,16 +395,16 @@ 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; - 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. - if (is_syskey) { - break; - } - return true; + bool is_new_event = OnKey(keyCode, scancode, message, 0, extended, was_down); + if (!is_new_event) { + break; + } + // For system keys, always pass them to the default WndProc so that keys + // like the ALT-TAB or Kanji switches are processed correctly. + if (is_syskey) { + break; } - break; + return true; } default: assert(false); @@ -202,4 +423,12 @@ UINT KeyboardManagerWin32::PeekNextMessageType(UINT wMsgFilterMin, return next_message.message; } +uint64_t KeyboardManagerWin32::ComputeEventHash(const PendingEvent& event) { + // Calculate a key event ID based on the scan code of the key pressed, + // and the flags we care about. + return event.scancode | (((event.action == WM_KEYUP ? KEYEVENTF_KEYUP : 0x0) | + (event.extended ? KEYEVENTF_EXTENDEDKEY : 0x0)) + << 16); +} + } // namespace flutter diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index a542b02910fa5..59d0bf4cc83a9 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -6,6 +6,8 @@ #define FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_MANAGER_H_ #include +#include +#include #include namespace flutter { @@ -32,21 +34,21 @@ class KeyboardManagerWin32 { // Typically implemented by |WindowWin32|. class WindowDelegate { public: + using KeyEventCallback = std::function; + virtual ~WindowDelegate() = default; // Called when text input occurs. virtual void OnText(const std::u16string& text) = 0; // Called when raw keyboard input occurs. - // - // Returns true if the event was handled, indicating that DefWindowProc - // should not be called on the event by the main message loop. - virtual bool OnKey(int key, + virtual void OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) = 0; + bool was_down, + KeyEventCallback callback) = 0; // Win32's PeekMessage. // @@ -60,8 +62,14 @@ class KeyboardManagerWin32 { // // Used to process key messages. virtual uint32_t Win32MapVkToChar(uint32_t virtual_key) = 0; + + virtual UINT Win32DispatchEvent(UINT cInputs, + LPINPUT pInputs, + int cbSize) = 0; }; + using KeyEventCallback = WindowDelegate::KeyEventCallback; + KeyboardManagerWin32(WindowDelegate* delegate); // Processes Win32 messages related to keyboard and text. @@ -80,7 +88,36 @@ class KeyboardManagerWin32 { WPARAM const wparam, LPARAM const lparam); + protected: + size_t RedispatchedCount(); + private: + struct PendingEvent { + uint32_t key; + uint8_t scancode; + uint32_t action; + char32_t character; + bool extended; + bool was_down; + + // The number of delegates that haven't replied. + size_t unreplied; + // Whether any replied delegates reported true (handled). + bool any_handled; + + // A value calculated out of critical event information that can be used + // to identify redispatched events. + uint64_t hash; + }; + + // Returns true if it's a new event, or false if it's a redispatched event. + bool OnKey(int key, + int scancode, + int action, + char32_t character, + bool extended, + bool was_down); + // Returns the type of the next WM message. // // The parameters limits the range of interested messages. See Win32's @@ -89,6 +126,15 @@ class KeyboardManagerWin32 { // If there's no message, returns 0. UINT PeekNextMessageType(UINT wMsgFilterMin, UINT wMsgFilterMax); + void DispatchEvent(const PendingEvent& event); + + // Find an event in the redispatch list that matches the given one. + // + // If an matching event is found, removes the matching event from the + // redispatch list, and returns true. Otherwise, returns false; + bool RemoveRedispatchedEvent(const PendingEvent& incoming); + void RedispatchEvent(std::unique_ptr event); + WindowDelegate* window_delegate_; // Keeps track of the last key code produced by a WM_KEYDOWN or WM_SYSKEYDOWN @@ -96,6 +142,42 @@ class KeyboardManagerWin32 { int keycode_for_char_message_ = 0; std::map text_for_scancode_on_redispatch_; + + // Whether the last event is a CtrlLeft key down. + // + // This is used to resolve a corner case described in |IsKeyDownAltRight|. + bool last_key_is_ctrl_left_down; + + // The scancode of the last met CtrlLeft down. + // + // This is used to resolve a corner case described in |IsKeyDownAltRight|. + uint8_t ctrl_left_scancode; + + // Whether a CtrlLeft up should be synthesized upon the next AltRight up. + // + // This is used to resolve a corner case described in |IsKeyDownAltRight|. + bool should_synthesize_ctrl_left_up; + + // The queue of key events that have been redispatched to the system but have + // not yet been received for a second time. + std::deque> pending_redispatches_; + + // Calculate a hash based on event data for fast comparison for a redispatched + // event. + // + // This uses event data instead of generating a serial number because + // information can't be attached to the redispatched events, so it has to be + // possible to compute an ID from the identifying data in the event when it is + // received again in order to differentiate between events that are new, and + // events that have been redispatched. + // + // Another alternative would be to compute a checksum from all the data in the + // event (just compute it over the bytes in the struct, probably skipping + // timestamps), but the fields used are enough to differentiate them, and + // since Windows does some processing on the events (coming up with virtual + // key codes, setting timestamps, etc.), it's not clear that the redispatched + // events would have the same checksums. + static uint64_t ComputeEventHash(const PendingEvent& event); }; } // namespace flutter diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index 0d505033fb64b..47fad34200bf7 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -70,13 +70,14 @@ class MockKeyboardManagerWin32Delegate virtual ~MockKeyboardManagerWin32Delegate() {} // |WindowWin32| - bool OnKey(int key, + void OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) override { - return view_->OnKey(key, scancode, action, character, extended, was_down); + bool was_down, + KeyEventCallback callback) override { + view_->OnKey(key, scancode, action, character, extended, was_down, callback); } // |WindowWin32| @@ -87,8 +88,35 @@ class MockKeyboardManagerWin32Delegate map_vk_to_char == nullptr ? LayoutDefault : map_vk_to_char; } + int InjectPendingEvents(uint32_t redispatch_char) { + std::vector messages; + int num_pending_responds = pending_responds_.size(); + 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; + const LPARAM lparam = CreateKeyEventLparam( + kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY, is_key_up); + // TODO(dkwingsmt): Don't check the message results for redispatched + // messages for now, because making them work takes non-trivial rework + // to our current structure. + // https://github.com/flutter/flutter/issues/87843 If this is resolved, + // change them to kWmResultDefault. + messages.push_back( + Win32Message{message, kbdinput.wVk, lparam, kWmResultDontCheck}); + if (redispatch_char != 0 && (kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { + messages.push_back( + Win32Message{WM_CHAR, redispatch_char, lparam, kWmResultDontCheck}); + } + } + + pending_responds_.clear(); + InjectMessageList(messages.size(), messages.data()); + return num_pending_responds; + } + protected: - virtual BOOL Win32PeekMessage(LPMSG lpMsg, + BOOL Win32PeekMessage(LPMSG lpMsg, UINT wMsgFilterMin, UINT wMsgFilterMax, UINT wRemoveMsg) override { @@ -96,11 +124,11 @@ class MockKeyboardManagerWin32Delegate wMsgFilterMax, wRemoveMsg); } - virtual uint32_t Win32MapVkToChar(uint32_t virtual_key) override { + uint32_t Win32MapVkToChar(uint32_t virtual_key) override { return map_vk_to_char_(virtual_key); } - virtual LRESULT Win32SendMessage(UINT const message, + LRESULT Win32SendMessage(UINT const message, WPARAM const wparam, LPARAM const lparam) override { return keyboard_manager_->HandleMessage(message, wparam, lparam) @@ -108,12 +136,18 @@ class MockKeyboardManagerWin32Delegate : kWmResultDefault; } + UINT Win32DispatchEvent(UINT cInputs, LPINPUT pInputs, int cbSize) override { + for (UINT input_idx = 0; input_idx < cInputs; input_idx += 1) { + pending_responds_.push_back(pInputs[input_idx].ki); + } + return 1; + } + private: WindowBindingHandlerDelegate* view_; - std::unique_ptr keyboard_manager_; - MapVkToCharHandler map_vk_to_char_; + std::vector pending_responds_; }; class TestKeystate { @@ -148,33 +182,6 @@ class TestFlutterWindowsView : public FlutterWindowsView { 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 KEYBDINPUT& kbdinput : pending_responds_) { - const UINT message = - (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; - const bool is_key_up = kbdinput.dwFlags & KEYEVENTF_KEYUP; - const LPARAM lparam = CreateKeyEventLparam( - kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY, is_key_up); - // TODO(dkwingsmt): Don't check the message results for redispatched - // messages for now, because making them work takes non-trivial rework - // to our current structure. - // https://github.com/flutter/flutter/issues/87843 If this is resolved, - // change them to kWmResultDefault. - messages.push_back( - Win32Message{message, kbdinput.wVk, lparam, kWmResultDontCheck}); - if (redispatch_char != 0 && (kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { - messages.push_back( - Win32Message{WM_CHAR, redispatch_char, lparam, kWmResultDontCheck}); - } - } - - pending_responds_.clear(); - queue->InjectMessageList(messages.size(), messages.data()); - return num_pending_responds; - } - void SetKeyState(uint32_t key, bool pressed, bool toggled_on) { key_state_.Set(key, pressed, toggled_on); } @@ -182,26 +189,14 @@ class TestFlutterWindowsView : public FlutterWindowsView { protected: std::unique_ptr CreateKeyboardKeyHandler( BinaryMessenger* messenger, - KeyboardKeyHandler::EventDispatcher dispatch_event, KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state) override { return FlutterWindowsView::CreateKeyboardKeyHandler( messenger, - [this](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { - return this->SendInput(cInputs, pInputs, cbSize); - }, key_state_.Getter()); } private: - UINT SendInput(UINT cInputs, LPINPUT pInputs, int 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_; TestKeystate key_state_; }; @@ -299,7 +294,7 @@ class KeyboardTester { // If |redispatch_char| is not 0, then WM_KEYDOWN events will // also redispatch a WM_CHAR event with that value as lparam. int InjectPendingEvents(uint32_t redispatch_char = 0) { - return view_->InjectPendingEvents(window_.get(), redispatch_char); + return window_->InjectPendingEvents(redispatch_char); } private: diff --git a/shell/platform/windows/testing/mock_window_win32.h b/shell/platform/windows/testing/mock_window_win32.h index 2d0839bb656de..383b75bd0702e 100644 --- a/shell/platform/windows/testing/mock_window_win32.h +++ b/shell/platform/windows/testing/mock_window_win32.h @@ -46,7 +46,14 @@ class MockWin32Window : public WindowWin32 { MOCK_METHOD2(OnPointerLeave, void(FlutterPointerDeviceKind, int32_t)); MOCK_METHOD0(OnSetCursor, void()); MOCK_METHOD1(OnText, void(const std::u16string&)); - MOCK_METHOD6(OnKey, bool(int, int, int, char32_t, bool, bool)); + MOCK_METHOD7(OnKey, + void(int, + int, + int, + char32_t, + bool, + bool, + KeyboardManagerWin32::KeyEventCallback)); MOCK_METHOD1(OnUpdateSemanticsEnabled, void(bool)); MOCK_METHOD0(GetNativeViewAccessible, gfx::NativeViewAccessible()); MOCK_METHOD4(OnScroll, diff --git a/shell/platform/windows/window_binding_handler_delegate.h b/shell/platform/windows/window_binding_handler_delegate.h index 177724e596168..54768de4f358d 100644 --- a/shell/platform/windows/window_binding_handler_delegate.h +++ b/shell/platform/windows/window_binding_handler_delegate.h @@ -5,6 +5,8 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_WINDOW_BINDING_HANDLER_DELEGATE_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_WINDOW_BINDING_HANDLER_DELEGATE_H_ +#include + #include "flutter/shell/platform/common/geometry.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/third_party/accessibility/gfx/native_widget_types.h" @@ -13,6 +15,8 @@ namespace flutter { class WindowBindingHandlerDelegate { public: + using KeyEventCallback = std::function; + // Notifies delegate that backing window size has changed. // Typically called by currently configured WindowBindingHandler, this is // called on the platform thread. @@ -55,12 +59,13 @@ class WindowBindingHandlerDelegate { // delegate that backing window size has received key press. Should return // true if the event was handled and should not be propagated. Typically // called by currently configured WindowBindingHandler. - virtual bool OnKey(int key, + virtual void OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down) = 0; + bool was_down, + KeyEventCallback callback) = 0; // Notifies the delegate that IME composing mode has begun. // diff --git a/shell/platform/windows/window_win32.cc b/shell/platform/windows/window_win32.cc index 177d0043fc557..f5035cb178cc9 100644 --- a/shell/platform/windows/window_win32.cc +++ b/shell/platform/windows/window_win32.cc @@ -551,4 +551,10 @@ uint32_t WindowWin32::Win32MapVkToChar(uint32_t virtual_key) { return ::MapVirtualKey(virtual_key, MAPVK_VK_TO_CHAR); } +UINT WindowWin32::Win32DispatchEvent(UINT cInputs, + LPINPUT pInputs, + int cbSize) { + return ::SendInput(cInputs, pInputs, cbSize); +} + } // namespace flutter diff --git a/shell/platform/windows/window_win32.h b/shell/platform/windows/window_win32.h index 6b8d7220cc2a5..7e3088c0062cb 100644 --- a/shell/platform/windows/window_win32.h +++ b/shell/platform/windows/window_win32.h @@ -48,6 +48,11 @@ class WindowWin32 : public KeyboardManagerWin32::WindowDelegate { // |KeyboardManagerWin32::WindowDelegate| virtual uint32_t Win32MapVkToChar(uint32_t virtual_key) override; + // |KeyboardManagerWin32::WindowDelegate| + virtual UINT Win32DispatchEvent(UINT cInputs, + LPINPUT pInputs, + int cbSize) override; + protected: // Converts a c string to a wide unicode string. std::wstring NarrowToWide(const char* source); diff --git a/shell/platform/windows/window_win32_unittests.cc b/shell/platform/windows/window_win32_unittests.cc index d494a0ff16e43..eee99879013c9 100644 --- a/shell/platform/windows/window_win32_unittests.cc +++ b/shell/platform/windows/window_win32_unittests.cc @@ -128,7 +128,7 @@ TEST(MockWin32Window, HorizontalScroll) { TEST(MockWin32Window, KeyDown) { MockWin32Window window; - EXPECT_CALL(window, OnKey(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(window, OnKey(_, _, _, _, _, _, _)).Times(1); LPARAM lparam = CreateKeyEventLparam(42, false, false); // send a "Shift" key down event. window.InjectWindowMessage(WM_KEYDOWN, 16, lparam); @@ -136,7 +136,7 @@ TEST(MockWin32Window, KeyDown) { TEST(MockWin32Window, KeyUp) { MockWin32Window window; - EXPECT_CALL(window, OnKey(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(window, OnKey(_, _, _, _, _, _, _)).Times(1); LPARAM lparam = CreateKeyEventLparam(42, false, true); // send a "Shift" key up event. window.InjectWindowMessage(WM_KEYUP, 16, lparam); @@ -144,7 +144,7 @@ TEST(MockWin32Window, KeyUp) { TEST(MockWin32Window, SysKeyDown) { MockWin32Window window; - EXPECT_CALL(window, OnKey(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(window, OnKey(_, _, _, _, _, _, _)).Times(1); LPARAM lparam = CreateKeyEventLparam(42, false, false); // send a "Shift" key down event. window.InjectWindowMessage(WM_SYSKEYDOWN, 16, lparam); @@ -152,7 +152,7 @@ TEST(MockWin32Window, SysKeyDown) { TEST(MockWin32Window, SysKeyUp) { MockWin32Window window; - EXPECT_CALL(window, OnKey(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(window, OnKey(_, _, _, _, _, _, _)).Times(1); LPARAM lparam = CreateKeyEventLparam(42, false, true); // send a "Shift" key up event. window.InjectWindowMessage(WM_SYSKEYUP, 16, lparam); @@ -162,7 +162,7 @@ TEST(MockWin32Window, KeyDownPrintable) { MockWin32Window window; LPARAM lparam = CreateKeyEventLparam(30, false, false); - EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 0, false, false)).Times(1); + EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 0, false, false, _)).Times(1); EXPECT_CALL(window, OnText(_)).Times(1); Win32Message messages[] = {{WM_KEYDOWN, 65, lparam, kWmResultDontCheck}, {WM_CHAR, 65, lparam, kWmResultDontCheck}}; @@ -182,7 +182,7 @@ TEST(MockWin32Window, KeyDownWithCtrl) { // Expect OnKey, but not OnText, because Control + Key is not followed by // WM_CHAR - EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 0, false, false)).Times(1); + EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 0, false, false, _)).Times(1); EXPECT_CALL(window, OnText(_)).Times(0); window.InjectWindowMessage(WM_KEYDOWN, 65, lparam); @@ -202,7 +202,7 @@ TEST(MockWin32Window, KeyDownWithCtrlToggled) { LPARAM lparam = CreateKeyEventLparam(30, false, false); - EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 0, false, false)).Times(1); + EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 0, false, false, _)).Times(1); EXPECT_CALL(window, OnText(_)).Times(1); // send a "A" key down event. From eb57adcd3ebe14faddc14205a56c3ef33a460f2c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 11 Jan 2022 01:54:12 -0800 Subject: [PATCH 2/9] Rewrite char processing. Fix most of keyboard test --- .../windows/flutter_window_win32_unittests.cc | 8 +- .../platform/windows/flutter_windows_view.cc | 5 +- .../platform/windows/keyboard_key_handler.cc | 2 +- .../windows/keyboard_manager_win32.cc | 97 ++++++++++++------- .../platform/windows/keyboard_manager_win32.h | 10 +- .../windows/keyboard_win32_unittests.cc | 50 +++++----- .../windows/testing/mock_window_win32.h | 8 +- 7 files changed, 99 insertions(+), 81 deletions(-) diff --git a/shell/platform/windows/flutter_window_win32_unittests.cc b/shell/platform/windows/flutter_window_win32_unittests.cc index 505e9afc7f245..51a970233623d 100644 --- a/shell/platform/windows/flutter_window_win32_unittests.cc +++ b/shell/platform/windows/flutter_window_win32_unittests.cc @@ -246,13 +246,7 @@ class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate { MOCK_METHOD2(OnPointerLeave, void(FlutterPointerDeviceKind, int32_t)); MOCK_METHOD1(OnText, void(const std::u16string&)); MOCK_METHOD7(OnKey, - void(int, - int, - int, - char32_t, - bool, - bool, - KeyboardManagerWin32::KeyEventCallback)); + void(int, int, int, char32_t, bool, bool, KeyEventCallback)); MOCK_METHOD0(OnComposeBegin, void()); MOCK_METHOD0(OnComposeCommit, void()); MOCK_METHOD0(OnComposeEnd, void()); diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 72be7f43bb534..2ffa51f1c6eb1 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -380,12 +380,13 @@ void FlutterWindowsView::SendKey(int key, bool was_down, KeyEventCallback callback) { keyboard_key_handler_->KeyboardHook( - key, scancode, action, character, extended, was_down, [&](bool handled) { + key, scancode, action, character, extended, was_down, + [&, callback = std::move(callback)](bool handled) { if (!handled) { text_input_plugin_->KeyboardHook(key, scancode, action, character, extended, was_down); } - return handled; + callback(handled); }); } diff --git a/shell/platform/windows/keyboard_key_handler.cc b/shell/platform/windows/keyboard_key_handler.cc index 4439af700736c..3306ae534e0cc 100644 --- a/shell/platform/windows/keyboard_key_handler.cc +++ b/shell/platform/windows/keyboard_key_handler.cc @@ -83,7 +83,7 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, if (event.unreplied == 0) { std::unique_ptr event_ptr = std::move(*iter); pending_responds_.erase(iter); - event.callback(handled); + event.callback(event.any_handled); } // Return here; |iter| can't do ++ after erase. return; diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 940c93378ac79..6e7dee6c4ca21 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -199,7 +199,8 @@ bool KeyboardManagerWin32::OnKey(int key, int action, char32_t character, bool extended, - bool was_down) { + bool was_down, + OnKeyCallback callback) { std::unique_ptr incoming = std::make_unique(PendingEvent{ .key = static_cast(key), @@ -242,27 +243,51 @@ bool KeyboardManagerWin32::OnKey(int key, window_delegate_->OnKey( key, scancode, action, character, extended, was_down, - [this, event = incoming.release()](bool handled) { - // Always consider some key events as handled. - // - // Redispatching dead keys events makes Win32 ignore the dead key state - // and redispatches a normal character without combining it with the - // 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 real_handled = - handled || _IsDeadKey(event->character) || is_syskey - || IsKeyDownShiftRight(event->key, event->was_down); - if (!real_handled) { - RedispatchEvent(std::unique_ptr(event)); - } + [this, event = incoming.release(), callback = std::move(callback)](bool handled) { + callback(std::unique_ptr(event), handled); }); return true; } +void KeyboardManagerWin32::HandleOnKeyResult( + std::unique_ptr event, + bool handled, + int char_action, + std::u16string text) { + // First, patch |handled|, because some key events must always be treated as + // handled. + // + // Redispatching dead keys events makes Win32 ignore the dead key state + // and redispatches a normal character without combining it with the + // 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 real_handled = + handled || _IsDeadKey(event->character) || is_syskey + || IsKeyDownShiftRight(event->key, event->was_down); + + // For handled events, that's all. + if (real_handled) { + return; + } + + // For unhandled events, dispatch them to OnText. + + // Of the messages handled here, only WM_CHAR should be treated as + // characters. WM_SYS*CHAR are not part of text input, and WM_DEADCHAR + // will be incorporated into a later WM_CHAR with the full character. + // Non-printable event characters have been filtered out before being passed + // to OnKey. + if (char_action == WM_CHAR && event->character != 0) { + window_delegate_->OnText(text); + } + + RedispatchEvent(std::move(event)); +} + bool KeyboardManagerWin32::HandleMessage(UINT const message, WPARAM const wparam, LPARAM const lparam) { @@ -274,17 +299,22 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, static wchar_t s_pending_high_surrogate = 0; wchar_t character = static_cast(wparam); - std::u16string text({character}); - char32_t code_point = character; + std::u16string text; + char32_t code_point; if (IS_HIGH_SURROGATE(character)) { // Save to send later with the trailing surrogate. s_pending_high_surrogate = character; + return true; } else if (IS_LOW_SURROGATE(character) && s_pending_high_surrogate != 0) { - text.insert(text.begin(), s_pending_high_surrogate); + text.push_back(s_pending_high_surrogate); + text.push_back(character); // Merge the surrogate pairs for the key event. code_point = CodePointFromSurrogatePair(s_pending_high_surrogate, character); s_pending_high_surrogate = 0; + } else { + text.push_back(character); + code_point = character; } const unsigned int scancode = (lparam >> 16) & 0xff; @@ -318,24 +348,14 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, bool is_new_event = OnKey( keycode_for_char_message_, scancode, message == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, event_character, - extended, was_down); + extended, was_down, + [this, message, text](std::unique_ptr event, bool handled) { + HandleOnKeyResult(std::move(event), handled, message, text); + }); if (!is_new_event) { break; } keycode_for_char_message_ = 0; - // If the OnKey handler handles the message, then return so we don't - // pass it to OnText, because handling the message indicates that - // OnKey either just sent it to the framework to be processed. - // - // This message will be redispatched if not handled by the framework, - // during which the OnText (below) might be reached. However, if the - // original message was preceded by dead chars (such as ^ and e - // yielding ê), then since the redispatched message is no longer - // preceded by the dead char, the text will be wrong. Therefore we - // record the text here for the redispached event to use. - if (message == WM_CHAR) { - text_for_scancode_on_redispatch_[scancode] = text; - } // For system characters, always pass them to the default WndProc so // that system keys like the ALT-TAB are processed correctly. @@ -395,7 +415,12 @@ 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; - bool is_new_event = OnKey(keyCode, scancode, message, 0, extended, was_down); + bool is_new_event = + OnKey(keyCode, scancode, message, 0, extended, was_down, + [this](std::unique_ptr event, + bool handled) { + HandleOnKeyResult(std::move(event), handled, 0, std::u16string()); + }); if (!is_new_event) { break; } diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index 59d0bf4cc83a9..2ba615968f6bf 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -110,13 +110,21 @@ class KeyboardManagerWin32 { uint64_t hash; }; + using OnKeyCallback = std::function, bool)>; + // Returns true if it's a new event, or false if it's a redispatched event. bool OnKey(int key, int scancode, int action, char32_t character, bool extended, - bool was_down); + bool was_down, + OnKeyCallback callback); + + void HandleOnKeyResult(std::unique_ptr event, + bool handled, + int char_action, + std::u16string text); // Returns the type of the next WM message. // diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index 47fad34200bf7..e247aa192d942 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -425,14 +425,14 @@ TEST(KeyboardTest, LowerCaseAUnhandled) { WmCharInfo{'a', kScanCodeKeyA, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyA, kLogicalKeyA, "a", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"a"); clear_key_calls(); tester.InjectPendingEvents('a'); - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release A @@ -482,14 +482,14 @@ TEST(KeyboardTest, ShiftLeftKeyA) { WmCharInfo{'A', kScanCodeKeyA, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyA, kLogicalKeyA, "A", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"A"); clear_key_calls(); tester.InjectPendingEvents('A'); - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"A"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release ShiftLeft @@ -677,14 +677,14 @@ TEST(KeyboardTest, Digit1OnFrenchLayout) { WmCharInfo{'&', kScanCodeDigit1, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalDigit1, kLogicalDigit1, "&", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"&"); clear_key_calls(); tester.InjectPendingEvents('&'); - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"&"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release 1 @@ -738,14 +738,14 @@ TEST(KeyboardTest, AltGrModifiedKey) { WmCharInfo{'@', kScanCodeKeyQ, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyQ, kLogicalKeyQ, "@", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"@"); clear_key_calls(); EXPECT_EQ(tester.InjectPendingEvents('@'), 1); - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"@"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release Q @@ -950,15 +950,15 @@ TEST(KeyboardTest, DeadKeyThatCombines) { WmCharInfo{0xEA, kScanCodeKeyE, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyE, kLogicalKeyE, "ê", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"ê"); clear_key_calls(); tester.InjectPendingEvents( 0xEA); // The redispatched event uses unmodified 'e' - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"ê"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release E @@ -1054,15 +1054,15 @@ TEST(KeyboardTest, DeadKeyWithoutDeadMaskThatCombines) { WmCharInfo{0xEA, kScanCodeKeyE, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyE, kLogicalKeyE, "ê", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"ê"); clear_key_calls(); tester.InjectPendingEvents( 0xEA); // The redispatched event uses unmodified 'e' - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"ê"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release E @@ -1130,18 +1130,14 @@ TEST(KeyboardTest, DeadKeyThatDoesNotCombine) { WmCharInfo{'&', kScanCodeDigit1, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 2); + EXPECT_EQ(key_calls.size(), 3); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalDigit1, kLogicalDigit1, "^", kNotSynthesized); EXPECT_CALL_IS_TEXT(key_calls[1], u"^"); + EXPECT_CALL_IS_TEXT(key_calls[2], u"&"); clear_key_calls(); tester.InjectPendingEvents('&'); - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"&"); - clear_key_calls(); - - tester.InjectPendingEvents(); EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); @@ -1179,9 +1175,10 @@ TEST(KeyboardTest, MultibyteCharacter) { const char* st = key_calls[0].key_event.character; - EXPECT_EQ(key_calls.size(), 1); + EXPECT_EQ(key_calls.size(), 2); EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyW, kLogicalKeyW, "𐍅", kNotSynthesized); + EXPECT_CALL_IS_TEXT(key_calls[1], u"𐍅"); clear_key_calls(); // Inject the redispatched high surrogate. @@ -1191,8 +1188,7 @@ TEST(KeyboardTest, MultibyteCharacter) { 1, WmCharInfo{0xdf45, kScanCodeKeyW, kNotExtended, kWasUp}.Build( kWmResultZero)); - EXPECT_EQ(key_calls.size(), 1); - EXPECT_CALL_IS_TEXT(key_calls[0], u"𐍅"); + EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); // Release W diff --git a/shell/platform/windows/testing/mock_window_win32.h b/shell/platform/windows/testing/mock_window_win32.h index 383b75bd0702e..2bc1991f2f369 100644 --- a/shell/platform/windows/testing/mock_window_win32.h +++ b/shell/platform/windows/testing/mock_window_win32.h @@ -47,13 +47,7 @@ class MockWin32Window : public WindowWin32 { MOCK_METHOD0(OnSetCursor, void()); MOCK_METHOD1(OnText, void(const std::u16string&)); MOCK_METHOD7(OnKey, - void(int, - int, - int, - char32_t, - bool, - bool, - KeyboardManagerWin32::KeyEventCallback)); + void(int, int, int, char32_t, bool, bool, KeyEventCallback)); MOCK_METHOD1(OnUpdateSemanticsEnabled, void(bool)); MOCK_METHOD0(GetNativeViewAccessible, gfx::NativeViewAccessible()); MOCK_METHOD4(OnScroll, From c0ff96de25715812ff5a2b821420d007ea5ffea1 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 11 Jan 2022 02:43:19 -0800 Subject: [PATCH 3/9] keyboardkeyhandler, keyboardmanager tests --- .../windows/keyboard_key_handler_unittests.cc | 144 +----------------- .../windows/keyboard_win32_unittests.cc | 19 ++- .../platform/windows/testing/test_keyboard.cc | 22 ++- .../platform/windows/testing/test_keyboard.h | 1 + 4 files changed, 35 insertions(+), 151 deletions(-) diff --git a/shell/platform/windows/keyboard_key_handler_unittests.cc b/shell/platform/windows/keyboard_key_handler_unittests.cc index 1689607c67612..4f95efc63cd72 100644 --- a/shell/platform/windows/keyboard_key_handler_unittests.cc +++ b/shell/platform/windows/keyboard_key_handler_unittests.cc @@ -184,7 +184,7 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { delegate_handler = respond_true; handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, OnKeyEventResult); - EXPECT_EQ(key_event_response, kNoResponse); + EXPECT_EQ(key_event_response, kHandled); EXPECT_EQ(hook_history.size(), 1); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); @@ -197,7 +197,7 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { delegate_handler = respond_false; handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, OnKeyEventResult); - EXPECT_EQ(key_event_response, kHandled); + EXPECT_EQ(key_event_response, kUnhandled); EXPECT_EQ(hook_history.size(), 1); EXPECT_EQ(hook_history.back().delegate_id, 1); EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); @@ -207,145 +207,5 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { key_event_response = kUnhandled; } -TEST(KeyboardKeyHandlerTest, WithTwoAsyncDelegates) { - std::list hook_history; - - TestKeyboardKeyHandler handler; - - auto delegate1 = std::make_unique(1, &hook_history); - CallbackHandler& delegate1_handler = delegate1->callback_handler; - handler.AddDelegate(std::move(delegate1)); - - auto delegate2 = std::make_unique(2, &hook_history); - CallbackHandler& delegate2_handler = delegate2->callback_handler; - handler.AddDelegate(std::move(delegate2)); - - /// Test 1: One delegate responds true, the other false - - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, false, - OnKeyEventResult); - EXPECT_EQ(key_event_response, kNoResponse); - EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(hook_history.front().delegate_id, 1); - EXPECT_EQ(hook_history.front().scancode, kHandledScanCode); - EXPECT_EQ(hook_history.front().was_down, false); - EXPECT_EQ(hook_history.back().delegate_id, 2); - EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); - EXPECT_EQ(hook_history.back().was_down, false); - - hook_history.back().callback(true); - EXPECT_EQ(key_event_response, kHandled); - - hook_history.front().callback(false); - EXPECT_EQ(key_event_response, kUnhandled); - - hook_history.clear(); - - /// Test 2: All delegates respond false - - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false, OnKeyEventResult); - EXPECT_EQ(key_event_response, kNoResponse); - EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(hook_history.front().delegate_id, 1); - EXPECT_EQ(hook_history.front().scancode, kHandledScanCode); - EXPECT_EQ(hook_history.front().was_down, false); - EXPECT_EQ(hook_history.back().delegate_id, 2); - EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); - EXPECT_EQ(hook_history.back().was_down, false); - - hook_history.front().callback(false); - EXPECT_EQ(key_event_response, kNoResponse); - - hook_history.back().callback(false); - EXPECT_EQ(key_event_response, kUnhandled); - - hook_history.clear(); - key_event_response = kNoResponse; - - /// Test 3: All delegates responds true - - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, - L'a', false, false, OnKeyEventResult); - EXPECT_EQ(key_event_response, kNoResponse); - EXPECT_EQ(hook_history.size(), 2); - EXPECT_EQ(hook_history.front().delegate_id, 1); - EXPECT_EQ(hook_history.front().scancode, kHandledScanCode); - EXPECT_EQ(hook_history.front().was_down, false); - EXPECT_EQ(hook_history.back().delegate_id, 2); - EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); - EXPECT_EQ(hook_history.back().was_down, false); - - hook_history.back().callback(true); - EXPECT_EQ(key_event_response, kNoResponse); - - hook_history.front().callback(true); - EXPECT_EQ(key_event_response, kHandled); - - key_event_response = kNoResponse; - hook_history.clear(); -} - -// 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 is identical -// to the one waiting for response, an earlier implementation will crash upon -// the response. -TEST(KeyboardKeyHandlerTest, WithSlowFrameworkResponse) { - std::list hook_history; - - TestKeyboardKeyHandler handler; - - auto delegate1 = std::make_unique(1, &hook_history); - CallbackHandler& delegate1_handler = delegate1->callback_handler; - handler.AddDelegate(std::move(delegate1)); - - // The first native event. - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true, OnKeyEventResult); - - // The second identical native event, received between the first and its - // framework response. - handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, true, OnKeyEventResult); - EXPECT_EQ(key_event_response, kNoResponse); - EXPECT_EQ(hook_history.size(), 2); - - // The first response. - hook_history.front().callback(false); - EXPECT_EQ(key_event_response, kUnhandled); - - key_event_response = kNoResponse; - - // The second response. - hook_history.back().callback(false); - EXPECT_EQ(key_event_response, kUnhandled); - - key_event_response = kNoResponse; - hook_history.clear(); -} - -// 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(KeyboardKeyHandlerTest, NeverRedispatchShiftRightKeyDown) { - std::list hook_history; - - TestKeyboardKeyHandler handler; - - auto delegate = std::make_unique(1, &hook_history); - delegate->callback_handler = respond_false; - handler.AddDelegate(std::move(delegate)); - - // Press ShiftRight and the delegate responds false. - - handler.KeyboardHook(VK_RSHIFT, kScanCodeShiftRight, WM_KEYDOWN, 0, false, - false, OnKeyEventResult); - EXPECT_EQ(key_event_response, kHandled); - EXPECT_EQ(hook_history.size(), 1); - - hook_history.clear(); -} - } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index e247aa192d942..e26986e8c26ca 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -105,6 +105,7 @@ class MockKeyboardManagerWin32Delegate messages.push_back( Win32Message{message, kbdinput.wVk, lparam, kWmResultDontCheck}); if (redispatch_char != 0 && (kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { + num_pending_responds += 1; messages.push_back( Win32Message{WM_CHAR, redispatch_char, lparam, kWmResultDontCheck}); } @@ -744,7 +745,7 @@ TEST(KeyboardTest, AltGrModifiedKey) { EXPECT_CALL_IS_TEXT(key_calls[1], u"@"); clear_key_calls(); - EXPECT_EQ(tester.InjectPendingEvents('@'), 1); + EXPECT_EQ(tester.InjectPendingEvents('@'), 2); EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); @@ -1182,11 +1183,15 @@ TEST(KeyboardTest, MultibyteCharacter) { clear_key_calls(); // Inject the redispatched high surrogate. - tester.InjectPendingEvents(0xd800); + EXPECT_EQ(tester.InjectPendingEvents(0xd800), 2); // Manually inject the redispatched low surrogate. + // + // TODO(dkwingsmt): The following message should return kWmResultZero. + // For now this is impossible since KeyboardManagerWin32 isn't passing the + // high surrogate messages to redispatching logic. tester.InjectMessages( 1, WmCharInfo{0xdf45, kScanCodeKeyW, kNotExtended, kWasUp}.Build( - kWmResultZero)); + kWmResultDontCheck)); EXPECT_EQ(key_calls.size(), 0); clear_key_calls(); @@ -1264,7 +1269,7 @@ TEST(KeyboardTest, DisorderlyRespondedEvents) { // Resolve the second event first to test disordered responses. recorded_callbacks.back()(false); - EXPECT_EQ(tester.InjectPendingEvents('b'), 1); + EXPECT_EQ(tester.InjectPendingEvents('b'), 2); EXPECT_EQ(key_calls.size(), 1); EXPECT_CALL_IS_TEXT(key_calls[0], u"b"); clear_key_calls(); @@ -1272,7 +1277,7 @@ TEST(KeyboardTest, DisorderlyRespondedEvents) { // Resolve the first event. recorded_callbacks.front()(false); - EXPECT_EQ(tester.InjectPendingEvents('a'), 1); + EXPECT_EQ(tester.InjectPendingEvents('a'), 2); EXPECT_EQ(key_calls.size(), 1); EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); clear_key_calls(); @@ -1320,7 +1325,7 @@ TEST(KeyboardTest, SlowFrameworkResponse) { // The first response. recorded_callbacks.front()(false); - EXPECT_EQ(tester.InjectPendingEvents('a'), 1); + EXPECT_EQ(tester.InjectPendingEvents('a'), 2); EXPECT_EQ(key_calls.size(), 1); EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); clear_key_calls(); @@ -1328,7 +1333,7 @@ TEST(KeyboardTest, SlowFrameworkResponse) { // The second response. recorded_callbacks.back()(false); - EXPECT_EQ(tester.InjectPendingEvents('a'), 1); + EXPECT_EQ(tester.InjectPendingEvents('a'), 2); EXPECT_EQ(key_calls.size(), 1); EXPECT_CALL_IS_TEXT(key_calls[0], u"a"); clear_key_calls(); diff --git a/shell/platform/windows/testing/test_keyboard.cc b/shell/platform/windows/testing/test_keyboard.cc index b93308d7cc92d..a712c293d5f15 100644 --- a/shell/platform/windows/testing/test_keyboard.cc +++ b/shell/platform/windows/testing/test_keyboard.cc @@ -38,6 +38,19 @@ std::unique_ptr> _keyHandlingResponse(bool handled) { return flutter::JsonMessageCodec::GetInstance().EncodeMessage(document); } +static std::string ordinal(int num) { + switch (num) { + case 1: + return "st"; + case 2: + return "nd"; + case 3: + return "rd"; + default: + return "th"; + } +} + // A struct to use as a FlutterPlatformMessageResponseHandle so it can keep the // callbacks and user data passed to the engine's // PlatformMessageCreateResponseHandle for use in the SendPlatformMessage @@ -184,10 +197,15 @@ void MockMessageQueue::InjectMessageList(int count, while (!_pending_messages.empty()) { Win32Message message = _pending_messages.front(); _pending_messages.pop_front(); + _sent_messages.push_back(message); LRESULT result = Win32SendMessage(message.message, message.wParam, message.lParam); if (message.expected_result != kWmResultDontCheck) { - EXPECT_EQ(result, message.expected_result); + EXPECT_EQ(result, message.expected_result) + << " This is the " << _sent_messages.size() + << ordinal(_sent_messages.size()) << " event, with\n " << std::hex + << "Message 0x" << message.message << " LParam 0x" << message.lParam + << " WParam 0x" << message.wParam; } } } @@ -213,5 +231,5 @@ BOOL MockMessageQueue::Win32PeekMessage(LPMSG lpMsg, return FALSE; } -} // namespace testing + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/testing/test_keyboard.h b/shell/platform/windows/testing/test_keyboard.h index 1d0b8f02c5763..f36b4904605f4 100644 --- a/shell/platform/windows/testing/test_keyboard.h +++ b/shell/platform/windows/testing/test_keyboard.h @@ -112,6 +112,7 @@ class MockMessageQueue { LPARAM const lparam) = 0; std::list _pending_messages; + std::list _sent_messages; }; } // namespace testing From 4561306fea6155f7047abd19dc06eed9b82e4840 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 12 Jan 2022 15:29:23 -0800 Subject: [PATCH 4/9] Cleanup --- shell/platform/windows/flutter_window_win32_unittests.cc | 5 ----- shell/platform/windows/keyboard_key_handler_unittests.cc | 8 -------- shell/platform/windows/keyboard_manager_win32.cc | 5 ----- shell/platform/windows/keyboard_manager_win32.h | 2 ++ 4 files changed, 2 insertions(+), 18 deletions(-) diff --git a/shell/platform/windows/flutter_window_win32_unittests.cc b/shell/platform/windows/flutter_window_win32_unittests.cc index eeebc21e14c7f..b93d5b85807a9 100644 --- a/shell/platform/windows/flutter_window_win32_unittests.cc +++ b/shell/platform/windows/flutter_window_win32_unittests.cc @@ -51,11 +51,6 @@ class SpyKeyboardKeyHandler : public KeyboardHandlerBase { bool extended, bool was_down, KeyEventCallback callback)); - MOCK_METHOD0(ComposeBeginHook, void()); - MOCK_METHOD0(ComposeCommitHook, void()); - MOCK_METHOD0(ComposeEndHook, void()); - MOCK_METHOD2(ComposeChangeHook, - void(const std::u16string& text, int cursor_pos)); private: std::unique_ptr real_implementation_; diff --git a/shell/platform/windows/keyboard_key_handler_unittests.cc b/shell/platform/windows/keyboard_key_handler_unittests.cc index b7da32cd7c4c3..eeb275cf309bb 100644 --- a/shell/platform/windows/keyboard_key_handler_unittests.cc +++ b/shell/platform/windows/keyboard_key_handler_unittests.cc @@ -203,14 +203,6 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { EXPECT_EQ(hook_history.back().scancode, kHandledScanCode); EXPECT_EQ(hook_history.back().was_down, false); - EXPECT_EQ(handler.HasRedispatched(), true); - - // Resolve the event - EXPECT_EQ(handler.KeyboardHook(64, kHandledScanCode, WM_KEYDOWN, L'a', false, - false), - false); - - EXPECT_EQ(handler.HasRedispatched(), false); hook_history.clear(); key_event_response = kNoResponse; } diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index e5e574e6700a5..1a18eee483838 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -161,14 +161,10 @@ void KeyboardManagerWin32::DispatchEvent(const PendingEvent& event) { std::cerr << " (character " << character << ")"; } std::cerr << std::endl; - ; } } void KeyboardManagerWin32::RedispatchEvent(std::unique_ptr event) { -#ifdef WINUWP - return; -#else DispatchEvent(*event); if (pending_redispatches_.size() > kMaxPendingEvents) { std::cerr @@ -177,7 +173,6 @@ void KeyboardManagerWin32::RedispatchEvent(std::unique_ptr event) << "framework. Are responses being sent?" << std::endl; } pending_redispatches_.push_back(std::move(event)); -#endif } size_t KeyboardManagerWin32::RedispatchedCount() { diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index 2ba615968f6bf..78b87355791b4 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -28,6 +28,8 @@ namespace flutter { // |FlutterWindowsView|'s. class KeyboardManagerWin32 { public: + using KeyEventCallback = std::function; + // Define how the keyboard manager accesses Win32 system calls (to allow // mocking) and sends the results of |OnKey| and |OnText|. // From 61d45e552ea32fa8a9f63916a8177265eff8b71b Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 12 Jan 2022 15:47:29 -0800 Subject: [PATCH 5/9] More cleanup --- shell/platform/windows/keyboard_key_handler.h | 7 ++----- .../windows/keyboard_key_handler_unittests.cc | 9 ++------- shell/platform/windows/keyboard_manager_win32.cc | 8 ++------ shell/platform/windows/keyboard_manager_win32.h | 13 +++---------- shell/platform/windows/testing/test_keyboard.cc | 2 +- 5 files changed, 10 insertions(+), 29 deletions(-) diff --git a/shell/platform/windows/keyboard_key_handler.h b/shell/platform/windows/keyboard_key_handler.h index 63a5d99d67cc6..a0a67fad69998 100644 --- a/shell/platform/windows/keyboard_key_handler.h +++ b/shell/platform/windows/keyboard_key_handler.h @@ -91,9 +91,6 @@ class KeyboardKeyHandler : public KeyboardHandlerBase { bool was_down, KeyEventCallback callback) override; - protected: - size_t RedispatchedCount(); - private: struct PendingEvent { // Self-incrementing ID attached to an event sent to the framework. @@ -111,14 +108,14 @@ class KeyboardKeyHandler : public KeyboardHandlerBase { void ResolvePendingEvent(uint64_t sequence_id, bool handled); + std::vector> delegates_; + // The queue of key events that have been sent to the framework but have not // yet received a response. std::deque> pending_responds_; // The sequence_id attached to the last event sent to the framework. uint64_t last_sequence_id_; - - std::vector> delegates_; }; } // namespace flutter diff --git a/shell/platform/windows/keyboard_key_handler_unittests.cc b/shell/platform/windows/keyboard_key_handler_unittests.cc index eeb275cf309bb..5e58739fda5f8 100644 --- a/shell/platform/windows/keyboard_key_handler_unittests.cc +++ b/shell/platform/windows/keyboard_key_handler_unittests.cc @@ -92,11 +92,6 @@ class MockKeyHandlerDelegate std::list* hook_history; }; -class TestKeyboardKeyHandler : public KeyboardKeyHandler { - public: - explicit TestKeyboardKeyHandler() : KeyboardKeyHandler() {} -}; - enum KeyEventResponse { kNoResponse, kHandled, @@ -114,7 +109,7 @@ void OnKeyEventResult(bool handled) { TEST(KeyboardKeyHandlerTest, SingleDelegateWithAsyncResponds) { std::list hook_history; - TestKeyboardKeyHandler handler; + KeyboardKeyHandler handler; // Add one delegate auto delegate = std::make_unique(1, &hook_history); handler.AddDelegate(std::move(delegate)); @@ -172,7 +167,7 @@ TEST(KeyboardKeyHandlerTest, SingleDelegateWithAsyncResponds) { TEST(KeyboardKeyHandlerTest, SingleDelegateWithSyncResponds) { std::list hook_history; - TestKeyboardKeyHandler handler; + KeyboardKeyHandler handler; // Add one delegate auto delegate = std::make_unique(1, &hook_history); CallbackHandler& delegate_handler = delegate->callback_handler; diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 1a18eee483838..ca85bdc07fb79 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -134,12 +134,12 @@ KeyboardManagerWin32::KeyboardManagerWin32(WindowDelegate* delegate) should_synthesize_ctrl_left_up(false) {} void KeyboardManagerWin32::DispatchEvent(const PendingEvent& event) { - char32_t character = event.character; - 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."); + char32_t character = event.character; + INPUT input_event{ .type = INPUT_KEYBOARD, .ki = @@ -175,10 +175,6 @@ void KeyboardManagerWin32::RedispatchEvent(std::unique_ptr event) pending_redispatches_.push_back(std::move(event)); } -size_t KeyboardManagerWin32::RedispatchedCount() { - return pending_redispatches_.size(); -} - bool KeyboardManagerWin32::RemoveRedispatchedEvent(const PendingEvent& incoming) { for (auto iter = pending_redispatches_.begin(); iter != pending_redispatches_.end(); ++iter) { diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index 78b87355791b4..d43b7751d6e6d 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -28,8 +28,6 @@ namespace flutter { // |FlutterWindowsView|'s. class KeyboardManagerWin32 { public: - using KeyEventCallback = std::function; - // Define how the keyboard manager accesses Win32 system calls (to allow // mocking) and sends the results of |OnKey| and |OnText|. // @@ -65,6 +63,9 @@ class KeyboardManagerWin32 { // Used to process key messages. virtual uint32_t Win32MapVkToChar(uint32_t virtual_key) = 0; + // Win32's |SendInput|. + // + // Used to synthesize key events. virtual UINT Win32DispatchEvent(UINT cInputs, LPINPUT pInputs, int cbSize) = 0; @@ -90,9 +91,6 @@ class KeyboardManagerWin32 { WPARAM const wparam, LPARAM const lparam); - protected: - size_t RedispatchedCount(); - private: struct PendingEvent { uint32_t key; @@ -102,11 +100,6 @@ class KeyboardManagerWin32 { bool extended; bool was_down; - // The number of delegates that haven't replied. - size_t unreplied; - // Whether any replied delegates reported true (handled). - bool any_handled; - // A value calculated out of critical event information that can be used // to identify redispatched events. uint64_t hash; diff --git a/shell/platform/windows/testing/test_keyboard.cc b/shell/platform/windows/testing/test_keyboard.cc index a712c293d5f15..eca499a26c84d 100644 --- a/shell/platform/windows/testing/test_keyboard.cc +++ b/shell/platform/windows/testing/test_keyboard.cc @@ -231,5 +231,5 @@ BOOL MockMessageQueue::Win32PeekMessage(LPMSG lpMsg, return FALSE; } - } // namespace testing +} // namespace testing } // namespace flutter From de01fb103c51684ad613f2995de641782e3111d4 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 12 Jan 2022 15:49:38 -0800 Subject: [PATCH 6/9] format --- .../platform/windows/flutter_windows_view.cc | 4 +- .../windows/keyboard_manager_win32.cc | 66 ++++++++++--------- .../platform/windows/keyboard_manager_win32.h | 3 +- .../windows/keyboard_win32_unittests.cc | 18 ++--- shell/platform/windows/window_win32.cc | 4 +- 5 files changed, 50 insertions(+), 45 deletions(-) diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 2ffa51f1c6eb1..538353344f22e 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -262,8 +262,8 @@ void FlutterWindowsView::InitializeKeyboard() { #else KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state = GetKeyState; #endif - keyboard_key_handler_ = std::move(CreateKeyboardKeyHandler( - internal_plugin_messenger, get_key_state)); + keyboard_key_handler_ = std::move( + CreateKeyboardKeyHandler(internal_plugin_messenger, get_key_state)); text_input_plugin_ = std::move(CreateTextInputPlugin(internal_plugin_messenger)); } diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index ca85bdc07fb79..0b535dafa08d3 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -3,8 +3,8 @@ // found in the LICENSE file. #include -#include #include +#include #include #include "keyboard_manager_win32.h" @@ -153,7 +153,8 @@ void KeyboardManagerWin32::DispatchEvent(const PendingEvent& event) { }, }; - UINT accepted = window_delegate_->Win32DispatchEvent(1, &input_event, sizeof(input_event)); + UINT accepted = window_delegate_->Win32DispatchEvent(1, &input_event, + sizeof(input_event)); if (accepted != 1) { std::cerr << "Unable to synthesize event for keyboard event with scancode " << event.scancode; @@ -164,7 +165,8 @@ void KeyboardManagerWin32::DispatchEvent(const PendingEvent& event) { } } -void KeyboardManagerWin32::RedispatchEvent(std::unique_ptr event) { +void KeyboardManagerWin32::RedispatchEvent( + std::unique_ptr event) { DispatchEvent(*event); if (pending_redispatches_.size() > kMaxPendingEvents) { std::cerr @@ -175,7 +177,8 @@ void KeyboardManagerWin32::RedispatchEvent(std::unique_ptr event) pending_redispatches_.push_back(std::move(event)); } -bool KeyboardManagerWin32::RemoveRedispatchedEvent(const PendingEvent& incoming) { +bool KeyboardManagerWin32::RemoveRedispatchedEvent( + const PendingEvent& incoming) { for (auto iter = pending_redispatches_.begin(); iter != pending_redispatches_.end(); ++iter) { if ((*iter)->hash == incoming.hash) { @@ -187,12 +190,12 @@ bool KeyboardManagerWin32::RemoveRedispatchedEvent(const PendingEvent& incoming) } bool KeyboardManagerWin32::OnKey(int key, - int scancode, - int action, - char32_t character, - bool extended, - bool was_down, - OnKeyCallback callback) { + int scancode, + int action, + char32_t character, + bool extended, + bool was_down, + OnKeyCallback callback) { std::unique_ptr incoming = std::make_unique(PendingEvent{ .key = static_cast(key), @@ -233,11 +236,12 @@ bool KeyboardManagerWin32::OnKey(int key, } } - window_delegate_->OnKey( - key, scancode, action, character, extended, was_down, - [this, event = incoming.release(), callback = std::move(callback)](bool handled) { - callback(std::unique_ptr(event), handled); - }); + window_delegate_->OnKey(key, scancode, action, character, extended, was_down, + [this, event = incoming.release(), + callback = std::move(callback)](bool handled) { + callback(std::unique_ptr(event), + handled); + }); return true; } @@ -257,9 +261,9 @@ void KeyboardManagerWin32::HandleOnKeyResult( // |SendInput|. const bool is_syskey = event->action == WM_SYSKEYDOWN || event->action == WM_SYSKEYUP; - const bool real_handled = - handled || _IsDeadKey(event->character) || is_syskey - || IsKeyDownShiftRight(event->key, event->was_down); + const bool real_handled = handled || _IsDeadKey(event->character) || + is_syskey || + IsKeyDownShiftRight(event->key, event->was_down); // For handled events, that's all. if (real_handled) { @@ -337,13 +341,14 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, } else { event_character = IsPrintable(code_point) ? code_point : 0; } - bool is_new_event = OnKey( - keycode_for_char_message_, scancode, - message == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, event_character, - extended, was_down, - [this, message, text](std::unique_ptr event, bool handled) { - HandleOnKeyResult(std::move(event), handled, message, text); - }); + bool is_new_event = + OnKey(keycode_for_char_message_, scancode, + message == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, + event_character, extended, was_down, + [this, message, text](std::unique_ptr event, + bool handled) { + HandleOnKeyResult(std::move(event), handled, message, text); + }); if (!is_new_event) { break; } @@ -407,12 +412,11 @@ 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; - bool is_new_event = - OnKey(keyCode, scancode, message, 0, extended, was_down, - [this](std::unique_ptr event, - bool handled) { - HandleOnKeyResult(std::move(event), handled, 0, std::u16string()); - }); + bool is_new_event = OnKey( + keyCode, scancode, message, 0, extended, was_down, + [this](std::unique_ptr event, bool handled) { + HandleOnKeyResult(std::move(event), handled, 0, std::u16string()); + }); if (!is_new_event) { break; } diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index d43b7751d6e6d..a1c3ec1ad7688 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -105,7 +105,8 @@ class KeyboardManagerWin32 { uint64_t hash; }; - using OnKeyCallback = std::function, bool)>; + using OnKeyCallback = + std::function, bool)>; // Returns true if it's a new event, or false if it's a redispatched event. bool OnKey(int key, diff --git a/shell/platform/windows/keyboard_win32_unittests.cc b/shell/platform/windows/keyboard_win32_unittests.cc index 44a922ec02c90..bfdda3da0915d 100644 --- a/shell/platform/windows/keyboard_win32_unittests.cc +++ b/shell/platform/windows/keyboard_win32_unittests.cc @@ -77,7 +77,8 @@ class MockKeyboardManagerWin32Delegate bool extended, bool was_down, KeyEventCallback callback) override { - view_->OnKey(key, scancode, action, character, extended, was_down, callback); + view_->OnKey(key, scancode, action, character, extended, was_down, + callback); } // |WindowWin32| @@ -118,9 +119,9 @@ class MockKeyboardManagerWin32Delegate protected: BOOL Win32PeekMessage(LPMSG lpMsg, - UINT wMsgFilterMin, - UINT wMsgFilterMax, - UINT wRemoveMsg) override { + UINT wMsgFilterMin, + UINT wMsgFilterMax, + UINT wRemoveMsg) override { return MockMessageQueue::Win32PeekMessage(lpMsg, wMsgFilterMin, wMsgFilterMax, wRemoveMsg); } @@ -130,8 +131,8 @@ class MockKeyboardManagerWin32Delegate } LRESULT Win32SendMessage(UINT const message, - WPARAM const wparam, - LPARAM const lparam) override { + WPARAM const wparam, + LPARAM const lparam) override { return keyboard_manager_->HandleMessage(message, wparam, lparam) ? 0 : kWmResultDefault; @@ -191,9 +192,8 @@ class TestFlutterWindowsView : public FlutterWindowsView { std::unique_ptr CreateKeyboardKeyHandler( BinaryMessenger* messenger, KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state) override { - return FlutterWindowsView::CreateKeyboardKeyHandler( - messenger, - key_state_.Getter()); + return FlutterWindowsView::CreateKeyboardKeyHandler(messenger, + key_state_.Getter()); } private: diff --git a/shell/platform/windows/window_win32.cc b/shell/platform/windows/window_win32.cc index f5035cb178cc9..965ef01bef73a 100644 --- a/shell/platform/windows/window_win32.cc +++ b/shell/platform/windows/window_win32.cc @@ -552,8 +552,8 @@ uint32_t WindowWin32::Win32MapVkToChar(uint32_t virtual_key) { } UINT WindowWin32::Win32DispatchEvent(UINT cInputs, - LPINPUT pInputs, - int cbSize) { + LPINPUT pInputs, + int cbSize) { return ::SendInput(cInputs, pInputs, cbSize); } From 9d3308444190dfa80ffee742017435e548e6e913 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 12 Jan 2022 16:06:20 -0800 Subject: [PATCH 7/9] Fix UWP compile --- .../platform/windows/flutter_window_winuwp.cc | 10 ++++++++-- .../windows/game_pad_cursor_winuwp.cc | 20 +++++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/shell/platform/windows/flutter_window_winuwp.cc b/shell/platform/windows/flutter_window_winuwp.cc index 7274e1af1d4c9..b879d7682eefe 100644 --- a/shell/platform/windows/flutter_window_winuwp.cc +++ b/shell/platform/windows/flutter_window_winuwp.cc @@ -67,6 +67,10 @@ winrt::Windows::UI::Core::CoreCursor GetCursorByName( } } +// UWP does not care if key events are handled, since it can not redispatch them +// anyway. +static void IgnoreKeyEventResult(bool handled) {} + } // namespace FlutterWindowWinUWP::FlutterWindowWinUWP( @@ -349,7 +353,8 @@ void FlutterWindowWinUWP::OnKeyUp( int action = 0x0101; binding_handler_delegate_->OnKey(key, scancode, action, 0, status.IsExtendedKey /* extended */, - status.WasKeyDown /* was_down */); + status.WasKeyDown /* was_down */, + IgnoreKeyEventResult); } void FlutterWindowWinUWP::OnKeyDown( @@ -365,7 +370,8 @@ void FlutterWindowWinUWP::OnKeyDown( int action = 0x0100; binding_handler_delegate_->OnKey(key, scancode, action, 0, status.IsExtendedKey /* extended */, - status.WasKeyDown /* was_down */); + status.WasKeyDown /* was_down */, + IgnoreKeyEventResult); } void FlutterWindowWinUWP::OnCharacterReceived( diff --git a/shell/platform/windows/game_pad_cursor_winuwp.cc b/shell/platform/windows/game_pad_cursor_winuwp.cc index f4edb195a6af1..ba764588aa96b 100644 --- a/shell/platform/windows/game_pad_cursor_winuwp.cc +++ b/shell/platform/windows/game_pad_cursor_winuwp.cc @@ -9,6 +9,10 @@ namespace flutter { static constexpr int32_t kDefaultPointerDeviceId = 0; +// UWP does not care if key events are handled, since it can not redispatch them +// anyway. +static void IgnoreKeyEventResult(bool handled) {} + GamepadCursorWinUWP::GamepadCursorWinUWP( WindowBindingHandlerDelegate* view, DisplayHelperWinUWP* displayhelper, @@ -201,25 +205,25 @@ void GamepadCursorWinUWP::OnGamepadButtonPressed( winrt::Windows::Gaming::Input::GamepadButtons::DPadLeft) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Left), kScanLeft, - 0x0100, 0, true, true); + 0x0100, 0, true, true, IgnoreKeyEventResult); } else if ((buttons & winrt::Windows::Gaming::Input::GamepadButtons::DPadRight) == winrt::Windows::Gaming::Input::GamepadButtons::DPadRight) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Right), kScanRight, - 0x0100, 0, true, true); + 0x0100, 0, true, true, IgnoreKeyEventResult); } else if ((buttons & winrt::Windows::Gaming::Input::GamepadButtons::DPadUp) == winrt::Windows::Gaming::Input::GamepadButtons::DPadUp) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Up), kScanUp, - 0x0100, 0, true, true); + 0x0100, 0, true, true, IgnoreKeyEventResult); } else if ((buttons & winrt::Windows::Gaming::Input::GamepadButtons::DPadDown) == winrt::Windows::Gaming::Input::GamepadButtons::DPadDown) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Down), kScanDown, - 0x0100, 0, true, true); + 0x0100, 0, true, true, IgnoreKeyEventResult); } } @@ -246,25 +250,25 @@ void GamepadCursorWinUWP::OnGamepadButtonReleased( winrt::Windows::Gaming::Input::GamepadButtons::DPadLeft) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Left), kScanLeft, - 0x0100, 0, true, false); + 0x0100, 0, true, false, IgnoreKeyEventResult); } else if ((buttons & winrt::Windows::Gaming::Input::GamepadButtons::DPadRight) == winrt::Windows::Gaming::Input::GamepadButtons::DPadRight) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Right), kScanRight, - 0x0100, 0, true, false); + 0x0100, 0, true, false, IgnoreKeyEventResult); } else if ((buttons & winrt::Windows::Gaming::Input::GamepadButtons::DPadUp) == winrt::Windows::Gaming::Input::GamepadButtons::DPadUp) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Up), kScanUp, - 0x0100, 0, true, false); + 0x0100, 0, true, false, IgnoreKeyEventResult); } else if ((buttons & winrt::Windows::Gaming::Input::GamepadButtons::DPadDown) == winrt::Windows::Gaming::Input::GamepadButtons::DPadDown) { binding_handler_delegate_->OnKey( static_cast(winrt::Windows::System::VirtualKey::Down), kScanDown, - 0x0100, 0, true, false); + 0x0100, 0, true, false, IgnoreKeyEventResult); } } From 2a96e3c017d7d8bf6e0ae24d5a86d3a7fd4cfd2a Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 12 Jan 2022 18:11:13 -0800 Subject: [PATCH 8/9] format --- shell/platform/windows/flutter_window_winuwp.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/shell/platform/windows/flutter_window_winuwp.cc b/shell/platform/windows/flutter_window_winuwp.cc index b879d7682eefe..9a2dc80092351 100644 --- a/shell/platform/windows/flutter_window_winuwp.cc +++ b/shell/platform/windows/flutter_window_winuwp.cc @@ -351,10 +351,9 @@ void FlutterWindowWinUWP::OnKeyUp( unsigned int scancode = status.ScanCode; int key = static_cast(args.VirtualKey()); int action = 0x0101; - binding_handler_delegate_->OnKey(key, scancode, action, 0, - status.IsExtendedKey /* extended */, - status.WasKeyDown /* was_down */, - IgnoreKeyEventResult); + binding_handler_delegate_->OnKey( + key, scancode, action, 0, status.IsExtendedKey /* extended */, + status.WasKeyDown /* was_down */, IgnoreKeyEventResult); } void FlutterWindowWinUWP::OnKeyDown( @@ -368,10 +367,9 @@ void FlutterWindowWinUWP::OnKeyDown( unsigned int scancode = status.ScanCode; int key = static_cast(args.VirtualKey()); int action = 0x0100; - binding_handler_delegate_->OnKey(key, scancode, action, 0, - status.IsExtendedKey /* extended */, - status.WasKeyDown /* was_down */, - IgnoreKeyEventResult); + binding_handler_delegate_->OnKey( + key, scancode, action, 0, status.IsExtendedKey /* extended */, + status.WasKeyDown /* was_down */, IgnoreKeyEventResult); } void FlutterWindowWinUWP::OnCharacterReceived( From 37c8bdb55e4008eb7d61fc4aad4af0595057e497 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 13 Jan 2022 13:40:27 -0800 Subject: [PATCH 9/9] Remove text_for_scancode_on_redispatch_ --- shell/platform/windows/keyboard_manager_win32.cc | 5 ----- shell/platform/windows/window_win32.h | 2 -- 2 files changed, 7 deletions(-) diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 0b535dafa08d3..365e75084e102 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -371,11 +371,6 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, // control key shortcuts. if (message == WM_CHAR && s_pending_high_surrogate == 0 && IsPrintable(character)) { - auto found_text_iter = text_for_scancode_on_redispatch_.find(scancode); - if (found_text_iter != text_for_scancode_on_redispatch_.end()) { - text = found_text_iter->second; - text_for_scancode_on_redispatch_.erase(found_text_iter); - } window_delegate_->OnText(text); } return true; diff --git a/shell/platform/windows/window_win32.h b/shell/platform/windows/window_win32.h index 7e3088c0062cb..e5d292fcb7556 100644 --- a/shell/platform/windows/window_win32.h +++ b/shell/platform/windows/window_win32.h @@ -243,8 +243,6 @@ class WindowWin32 : public KeyboardManagerWin32::WindowDelegate { // message. int keycode_for_char_message_ = 0; - std::map text_for_scancode_on_redispatch_; - // Manages IME state. std::unique_ptr text_input_manager_;