From 15639b88203382fa52c7e4a873a31558a05cfe3f Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 25 Aug 2020 17:18:39 -0700 Subject: [PATCH 1/7] Implement delayed key event synthesis for Windows --- shell/platform/windows/BUILD.gn | 1 + .../flutter_windows_engine_unittests.cc | 2 +- .../platform/windows/flutter_windows_view.cc | 19 ++- shell/platform/windows/flutter_windows_view.h | 12 +- shell/platform/windows/key_event_handler.cc | 150 +++++++++++++++--- shell/platform/windows/key_event_handler.h | 31 +++- .../platform/windows/keyboard_hook_handler.h | 8 +- .../windows/testing/mock_win32_window.h | 2 +- shell/platform/windows/text_input_plugin.cc | 8 +- shell/platform/windows/text_input_plugin.h | 5 +- .../platform/windows/win32_flutter_window.cc | 8 +- shell/platform/windows/win32_flutter_window.h | 6 +- shell/platform/windows/win32_window.cc | 33 ++-- shell/platform/windows/win32_window.h | 9 +- .../windows/window_binding_handler_delegate.h | 11 +- 15 files changed, 245 insertions(+), 60 deletions(-) diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index e9e33d6efa518..bcd6b27a97733 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -167,6 +167,7 @@ if (target_os == "winuwp") { "flutter_project_bundle_unittests.cc", "flutter_windows_engine_unittests.cc", "flutter_windows_texture_registrar_unittests.cc", + "key_event_handler_unittests.cc", "string_conversion_unittests.cc", "system_utils_unittests.cc", "testing/engine_embedder_api_modifier.h", diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 5837e32330200..6d48818af3db2 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -102,7 +102,7 @@ TEST(FlutterWindowsEngine, SendPlatformMessageWithoutResponse) { const char* channel = "test"; const std::vector test_message = {1, 2, 3, 4}; - // Without a respones, SendPlatformMessage should be a simple passthrough. + // Without a responses, SendPlatformMessage should be a simple pass-through. bool called = false; modifier.embedder_api().SendPlatformMessage = MOCK_ENGINE_PROC( SendPlatformMessage, ([&called, test_message](auto engine, auto message) { diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index a9315d6f58cda..6f735aea449ff 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -120,11 +120,12 @@ void FlutterWindowsView::OnText(const std::u16string& text) { SendText(text); } -void FlutterWindowsView::OnKey(int key, +bool FlutterWindowsView::OnKey(int key, int scancode, int action, - char32_t character) { - SendKey(key, scancode, action, character); + char32_t character, + bool extended) { + return SendKey(key, scancode, action, character, extended); } void FlutterWindowsView::OnScroll(double x, @@ -215,13 +216,19 @@ void FlutterWindowsView::SendText(const std::u16string& text) { } } -void FlutterWindowsView::SendKey(int key, +bool FlutterWindowsView::SendKey(int key, int scancode, int action, - char32_t character) { + char32_t character, + bool extended) { for (const auto& handler : keyboard_hook_handlers_) { - handler->KeyboardHook(this, key, scancode, action, character); + if (handler->KeyboardHook(this, key, scancode, action, character, + extended)) { + // key event was handled, so don't send to other handlers. + return true; + } } + return false; } void FlutterWindowsView::SendScroll(double x, diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index d6373016c505e..25d4f59402ab1 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -100,7 +100,11 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, void OnText(const std::u16string&) override; // |WindowBindingHandlerDelegate| - void OnKey(int key, int scancode, int action, char32_t character) override; + bool OnKey(int key, + int scancode, + int action, + char32_t character, + bool extended) override; // |WindowBindingHandlerDelegate| void OnScroll(double x, @@ -166,7 +170,11 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, void SendText(const std::u16string&); // Reports a raw keyboard message to Flutter engine. - void SendKey(int key, int scancode, int action, char32_t character); + bool SendKey(int key, + int scancode, + int action, + char32_t character, + bool extended); // Reports scroll wheel events to Flutter engine. void SendScroll(double x, diff --git a/shell/platform/windows/key_event_handler.cc b/shell/platform/windows/key_event_handler.cc index 9b562f8f0944e..e57f4e6bc26e2 100644 --- a/shell/platform/windows/key_event_handler.cc +++ b/shell/platform/windows/key_event_handler.cc @@ -10,6 +10,10 @@ #include "flutter/shell/platform/common/cpp/json_message_codec.h" +namespace flutter { + +namespace { + static constexpr char kChannelName[] = "flutter/keyevent"; static constexpr char kKeyCodeKey[] = "keyCode"; @@ -18,33 +22,35 @@ static constexpr char kCharacterCodePointKey[] = "characterCodePoint"; static constexpr char kModifiersKey[] = "modifiers"; static constexpr char kKeyMapKey[] = "keymap"; static constexpr char kTypeKey[] = "type"; +static constexpr char kHandledKey[] = "handled"; static constexpr char kWindowsKeyMap[] = "windows"; static constexpr char kKeyUp[] = "keyup"; static constexpr char kKeyDown[] = "keydown"; -namespace flutter { +// The maximum number of pending events to keep before +// emitting a warning on the console about unhandled events. +static constexpr int kMaxPendingEvents = 1000; // Re-definition of the modifiers for compatibility with the Flutter framework. // These have to be in sync with the framework's RawKeyEventDataWindows // modifiers definition. // https://github.com/flutter/flutter/blob/19ff596979e407c484a32f4071420fca4f4c885f/packages/flutter/lib/src/services/raw_keyboard_windows.dart#L203 -const int kShift = 1 << 0; -const int kShiftLeft = 1 << 1; -const int kShiftRight = 1 << 2; -const int kControl = 1 << 3; -const int kControlLeft = 1 << 4; -const int kControlRight = 1 << 5; -const int kAlt = 1 << 6; -const int kAltLeft = 1 << 7; -const int kAltRight = 1 << 8; -const int kWinLeft = 1 << 9; -const int kWinRight = 1 << 10; -const int kCapsLock = 1 << 11; -const int kNumLock = 1 << 12; -const int kScrollLock = 1 << 13; +static constexpr int kShift = 1 << 0; +static constexpr int kShiftLeft = 1 << 1; +static constexpr int kShiftRight = 1 << 2; +static constexpr int kControl = 1 << 3; +static constexpr int kControlLeft = 1 << 4; +static constexpr int kControlRight = 1 << 5; +static constexpr int kAlt = 1 << 6; +static constexpr int kAltLeft = 1 << 7; +static constexpr int kAltRight = 1 << 8; +static constexpr int kWinLeft = 1 << 9; +static constexpr int kWinRight = 1 << 10; +static constexpr int kCapsLock = 1 << 11; +static constexpr int kNumLock = 1 << 12; +static constexpr int kScrollLock = 1 << 13; -namespace { /// Calls GetKeyState() an all modifier keys and packs the result in an int, /// with the re-defined values declared above for compatibility with the Flutter /// framework. @@ -81,25 +87,117 @@ int GetModsForKeyState() { mods |= kScrollLock; return mods; } + +uint64_t CalculateEventId(int scancode, int action, bool extended) { + // Calculate a key event ID based on the scan code of the key pressed, + // and the flags we care about. + return scancode | (((action == WM_KEYUP ? KEYEVENTF_KEYUP : 0x0) | + (extended ? KEYEVENTF_EXTENDEDKEY : 0x0)) + << 16); +} } // namespace -KeyEventHandler::KeyEventHandler(flutter::BinaryMessenger* messenger) +KeyEventHandler::KeyEventHandler(flutter::BinaryMessenger* messenger, + KeyEventHandler::SendInputDelegate send_input) : channel_( std::make_unique>( messenger, kChannelName, - &flutter::JsonMessageCodec::GetInstance())) {} + &flutter::JsonMessageCodec::GetInstance())), + send_input_(send_input) { + assert(send_input != nullptr); +} KeyEventHandler::~KeyEventHandler() = default; void KeyEventHandler::TextHook(FlutterWindowsView* view, const std::u16string& code_point) {} -void KeyEventHandler::KeyboardHook(FlutterWindowsView* view, +KEYBDINPUT* KeyEventHandler::FindPendingEvent(uint64_t id) { + if (pending_events_.empty()) { + return nullptr; + } + for (auto iter = pending_events_.begin(); iter != pending_events_.end(); + ++iter) { + if (iter->first == id) { + return &iter->second; + } + } + return nullptr; +} + +void KeyEventHandler::RemovePendingEvent(uint64_t id) { + for (auto iter = pending_events_.begin(); iter != pending_events_.end(); + ++iter) { + if (iter->first == id) { + pending_events_.erase(iter); + return; + } + } + std::cerr << "Tried to remove pending event with id " << id + << ", but the event was not found." << std::endl; +} + +void KeyEventHandler::AddPendingEvent(uint64_t id, + int scancode, + int action, + bool extended) { + if (pending_events_.size() > kMaxPendingEvents) { + std::cerr + << "There are " << pending_events_.size() + << " keyboard events that have not yet received a response from the " + << "framework. Are responses being sent?" << std::endl; + } + KEYBDINPUT key_event = KEYBDINPUT{0}; + key_event.wScan = scancode; + key_event.dwFlags = KEYEVENTF_SCANCODE | + (extended ? KEYEVENTF_EXTENDEDKEY : 0x0) | + (action == WM_KEYUP ? KEYEVENTF_KEYUP : 0x0); + pending_events_.push_back(std::make_pair(id, key_event)); +} + +void KeyEventHandler::HandleResponse(bool handled, + uint64_t id, + int action, + bool extended, + int scancode, + int character) { + if (handled) { + this->RemovePendingEvent(id); + } else { + // Since the framework didn't handle the event, we inject a newly + // synthesized one. We let Windows figure out the virtual key and + // character for the given scancode, as well as a new timestamp. + const KEYBDINPUT* key_event = this->FindPendingEvent(id); + if (key_event == nullptr) { + std::cerr << "Unable to find event " << id << " in pending events queue."; + return; + } + INPUT input_event; + input_event.type = INPUT_KEYBOARD; + input_event.ki = *key_event; + UINT accepted = send_input_(1, &input_event, sizeof(input_event)); + if (accepted != 1) { + std::cerr << "Unable to synthesize event for unhandled keyboard event " + "with scancode " + << scancode << " (character " << character << ")" << std::endl; + } + } +} + +bool KeyEventHandler::KeyboardHook(FlutterWindowsView* view, int key, int scancode, int action, - char32_t character) { + char32_t character, + bool extended) { + const uint64_t id = CalculateEventId(scancode, action, extended); + if (FindPendingEvent(id) != nullptr) { + // Don't pass messages that we synthesized to the framework again. + RemovePendingEvent(id); + return false; + } + // TODO: Translate to a cross-platform key code system rather than passing // the native key code. rapidjson::Document event(rapidjson::kObjectType); @@ -119,9 +217,17 @@ void KeyEventHandler::KeyboardHook(FlutterWindowsView* view, break; default: std::cerr << "Unknown key event action: " << action << std::endl; - return; + return false; } - channel_->Send(event); + AddPendingEvent(id, scancode, action, extended); + channel_->Send(event, [this, id, action, extended, scancode, character]( + const uint8_t* reply, size_t reply_size) { + auto decoded = flutter::JsonMessageCodec::GetInstance().DecodeMessage( + reply, reply_size); + bool handled = (*decoded)[kHandledKey].GetBool(); + this->HandleResponse(handled, id, action, extended, scancode, character); + }); + return true; } } // namespace flutter diff --git a/shell/platform/windows/key_event_handler.h b/shell/platform/windows/key_event_handler.h index 9a1b2752288e9..86433e6f6322a 100644 --- a/shell/platform/windows/key_event_handler.h +++ b/shell/platform/windows/key_event_handler.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_KEY_EVENT_HANDLER_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_KEY_EVENT_HANDLER_H_ +#include #include #include @@ -23,24 +24,48 @@ class FlutterWindowsView; // Handles key events and forwards them to the Flutter engine. class KeyEventHandler : public KeyboardHookHandler { public: - explicit KeyEventHandler(flutter::BinaryMessenger* messenger); + using SendInputDelegate = + std::function; + + explicit KeyEventHandler(flutter::BinaryMessenger* messenger, + SendInputDelegate delegate = SendInput); virtual ~KeyEventHandler(); // |KeyboardHookHandler| - void KeyboardHook(FlutterWindowsView* window, + bool KeyboardHook(FlutterWindowsView* window, int key, int scancode, int action, - char32_t character) override; + char32_t character, + bool extended) override; // |KeyboardHookHandler| void TextHook(FlutterWindowsView* window, const std::u16string& text) override; private: + KEYBDINPUT* FindPendingEvent(uint64_t id); + void RemovePendingEvent(uint64_t id); + void AddPendingEvent(uint64_t id, int scancode, int action, bool extended); + void HandleResponse(bool handled, + uint64_t id, + int action, + bool extended, + int scancode, + int character); + // The Flutter system channel for key event messages. std::unique_ptr> channel_; + + // The queue of key events that have been sent to the framework but have not + // yet received a response. + std::deque> pending_events_; + + // A function used to dispatch synthesized events. Used in testing to inject a + // test function to collect events. Defaults to the Windows function + // SendInput. + SendInputDelegate send_input_; }; } // namespace flutter diff --git a/shell/platform/windows/keyboard_hook_handler.h b/shell/platform/windows/keyboard_hook_handler.h index b36ad3a6d7d82..37bcbafbeb2b3 100644 --- a/shell/platform/windows/keyboard_hook_handler.h +++ b/shell/platform/windows/keyboard_hook_handler.h @@ -19,11 +19,15 @@ class KeyboardHookHandler { virtual ~KeyboardHookHandler() = default; // A function for hooking into keyboard input. - virtual void KeyboardHook(FlutterWindowsView* view, + // + // Returns true if the key event has been handled, to indicate that other + // handlers should not be called for this event. + virtual bool KeyboardHook(FlutterWindowsView* view, int key, int scancode, int action, - char32_t character) = 0; + char32_t character, + bool extended) = 0; // A function for hooking into Unicode text input. virtual void TextHook(FlutterWindowsView* view, diff --git a/shell/platform/windows/testing/mock_win32_window.h b/shell/platform/windows/testing/mock_win32_window.h index 224aaf9090510..ed7320a96f3f0 100644 --- a/shell/platform/windows/testing/mock_win32_window.h +++ b/shell/platform/windows/testing/mock_win32_window.h @@ -36,7 +36,7 @@ class MockWin32Window : public Win32Window { MOCK_METHOD0(OnPointerLeave, void()); MOCK_METHOD0(OnSetCursor, void()); MOCK_METHOD1(OnText, void(const std::u16string&)); - MOCK_METHOD4(OnKey, void(int, int, int, char32_t)); + MOCK_METHOD5(OnKey, bool(int, int, int, char32_t, bool)); MOCK_METHOD2(OnScroll, void(double, double)); }; diff --git a/shell/platform/windows/text_input_plugin.cc b/shell/platform/windows/text_input_plugin.cc index 43fbd40689a53..69c66de50ba97 100644 --- a/shell/platform/windows/text_input_plugin.cc +++ b/shell/platform/windows/text_input_plugin.cc @@ -61,13 +61,14 @@ void TextInputPlugin::TextHook(FlutterWindowsView* view, SendStateUpdate(*active_model_); } -void TextInputPlugin::KeyboardHook(FlutterWindowsView* view, +bool TextInputPlugin::KeyboardHook(FlutterWindowsView* view, int key, int scancode, int action, - char32_t character) { + char32_t character, + bool extended) { if (active_model_ == nullptr) { - return; + return false; } if (action == WM_KEYDOWN) { // Most editing keys (arrow keys, backspace, delete, etc.) are handled in @@ -80,6 +81,7 @@ void TextInputPlugin::KeyboardHook(FlutterWindowsView* view, break; } } + return false; } TextInputPlugin::TextInputPlugin(flutter::BinaryMessenger* messenger, diff --git a/shell/platform/windows/text_input_plugin.h b/shell/platform/windows/text_input_plugin.h index acf0241d3d98c..47415365507bc 100644 --- a/shell/platform/windows/text_input_plugin.h +++ b/shell/platform/windows/text_input_plugin.h @@ -33,11 +33,12 @@ class TextInputPlugin : public KeyboardHookHandler { virtual ~TextInputPlugin(); // |KeyboardHookHandler| - void KeyboardHook(FlutterWindowsView* view, + bool KeyboardHook(FlutterWindowsView* view, int key, int scancode, int action, - char32_t character) override; + char32_t character, + bool extended) override; // |KeyboardHookHandler| void TextHook(FlutterWindowsView* view, const std::u16string& text) override; diff --git a/shell/platform/windows/win32_flutter_window.cc b/shell/platform/windows/win32_flutter_window.cc index 0ec6f416549a9..5915cd03b7a32 100644 --- a/shell/platform/windows/win32_flutter_window.cc +++ b/shell/platform/windows/win32_flutter_window.cc @@ -162,11 +162,13 @@ void Win32FlutterWindow::OnText(const std::u16string& text) { binding_handler_delegate_->OnText(text); } -void Win32FlutterWindow::OnKey(int key, +bool Win32FlutterWindow::OnKey(int key, int scancode, int action, - char32_t character) { - binding_handler_delegate_->OnKey(key, scancode, action, character); + char32_t character, + bool extended) { + return binding_handler_delegate_->OnKey(key, scancode, action, character, + extended); } void Win32FlutterWindow::OnScroll(double delta_x, double delta_y) { diff --git a/shell/platform/windows/win32_flutter_window.h b/shell/platform/windows/win32_flutter_window.h index e3f02ea22d43d..3e86d9da7c1de 100644 --- a/shell/platform/windows/win32_flutter_window.h +++ b/shell/platform/windows/win32_flutter_window.h @@ -55,7 +55,11 @@ class Win32FlutterWindow : public Win32Window, public WindowBindingHandler { void OnText(const std::u16string& text) override; // |Win32Window| - void OnKey(int key, int scancode, int action, char32_t character) override; + bool OnKey(int key, + int scancode, + int action, + char32_t character, + bool extended) override; // |Win32Window| void OnScroll(double delta_x, double delta_y) override; diff --git a/shell/platform/windows/win32_window.cc b/shell/platform/windows/win32_window.cc index 7ed1f7c0ccd6f..8b9d9ab70f276 100644 --- a/shell/platform/windows/win32_window.cc +++ b/shell/platform/windows/win32_window.cc @@ -225,6 +225,25 @@ Win32Window::HandleMessage(UINT const message, s_pending_high_surrogate = 0; } + // All key presses that generate a character should be sent from + // WM_CHAR. In order to send the full key press information, the keycode + // is persisted in keycode_for_char_message_ obtained from WM_KEYDOWN. + if (keycode_for_char_message_ != 0) { + const unsigned int scancode = (lparam >> 16) & 0xff; + const bool extended = ((lparam >> 24) & 0x01) == 0x01; + bool handled = OnKey(keycode_for_char_message_, scancode, WM_KEYDOWN, + code_point, extended); + 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, or the + // framework handled the key in its response, so it shouldn't also be + // added as text. + return 0; + } + } + // 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. @@ -236,15 +255,6 @@ Win32Window::HandleMessage(UINT const message, character >= u' ') { OnText(text); } - - // All key presses that generate a character should be sent from - // WM_CHAR. In order to send the full key press information, the keycode - // is persisted in keycode_for_char_message_ obtained from WM_KEYDOWN. - if (keycode_for_char_message_ != 0) { - const unsigned int scancode = (lparam >> 16) & 0xff; - OnKey(keycode_for_char_message_, scancode, WM_KEYDOWN, code_point); - keycode_for_char_message_ = 0; - } break; } case WM_KEYDOWN: @@ -263,12 +273,15 @@ Win32Window::HandleMessage(UINT const message, } unsigned int keyCode(wparam); const unsigned int scancode = (lparam >> 16) & 0xff; + const bool extended = ((lparam >> 24) & 0x01) == 0x01; // If the key is a modifier, get its side. if (keyCode == VK_SHIFT || keyCode == VK_MENU || keyCode == VK_CONTROL) { keyCode = MapVirtualKey(scancode, MAPVK_VSC_TO_VK_EX); } const int action = is_keydown_message ? WM_KEYDOWN : WM_KEYUP; - OnKey(keyCode, scancode, action, 0); + if (OnKey(keyCode, scancode, action, 0, extended)) { + return 0; + } break; } diff --git a/shell/platform/windows/win32_window.h b/shell/platform/windows/win32_window.h index 453bc0e90480f..2d964bb62540f 100644 --- a/shell/platform/windows/win32_window.h +++ b/shell/platform/windows/win32_window.h @@ -92,7 +92,14 @@ class Win32Window { virtual void OnText(const std::u16string& text) = 0; // Called when raw keyboard input occurs. - virtual void OnKey(int key, int scancode, int action, char32_t character) = 0; + // + // 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, + int scancode, + int action, + char32_t character, + bool extended) = 0; // Called when mouse scrollwheel input occurs. virtual void OnScroll(double delta_x, double delta_y) = 0; diff --git a/shell/platform/windows/window_binding_handler_delegate.h b/shell/platform/windows/window_binding_handler_delegate.h index 61218822ca7ce..8ea914bde3e9e 100644 --- a/shell/platform/windows/window_binding_handler_delegate.h +++ b/shell/platform/windows/window_binding_handler_delegate.h @@ -41,9 +41,14 @@ class WindowBindingHandlerDelegate { // Typically called by currently configured WindowBindingHandler virtual void OnText(const std::u16string&) = 0; - // Notifies delegate that backing window size has received key press. - // Typically called by currently configured WindowBindingHandler - virtual void OnKey(int key, int scancode, int action, char32_t character) = 0; + // Notifies 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, + int scancode, + int action, + char32_t character, + bool extended) = 0; // Notifies delegate that backing window size has recevied scroll. // Typically called by currently configured WindowBindingHandler From 25c6337ce51120870710e297005a482910b9159d Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 14 Jan 2021 19:46:54 -0800 Subject: [PATCH 2/7] Adding some tests --- shell/platform/windows/BUILD.gn | 2 + .../platform/windows/flutter_windows_view.cc | 14 +- shell/platform/windows/flutter_windows_view.h | 8 + .../windows/testing/mock_win32_window.cc | 8 +- .../windows/testing/mock_win32_window.h | 11 +- .../testing/mock_window_binding_handler.cc | 15 ++ .../testing/mock_window_binding_handler.h | 36 ++++ .../testing/win32_flutter_window_test.h | 9 +- .../windows/win32_flutter_window_unittests.cc | 173 ++++++++++++++++++ shell/platform/windows/win32_window.cc | 7 + .../windows/win32_window_unittests.cc | 43 +++++ 11 files changed, 312 insertions(+), 14 deletions(-) create mode 100644 shell/platform/windows/testing/mock_window_binding_handler.cc create mode 100644 shell/platform/windows/testing/mock_window_binding_handler.h diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index bcd6b27a97733..a1ce9f169c8b8 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -173,6 +173,8 @@ if (target_os == "winuwp") { "testing/engine_embedder_api_modifier.h", "testing/mock_win32_window.cc", "testing/mock_win32_window.h", + "testing/mock_window_binding_handler.cc", + "testing/mock_window_binding_handler.h", "testing/win32_flutter_window_test.cc", "testing/win32_flutter_window_test.h", "win32_dpi_utils_unittests.cc", diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 6f735aea449ff..b2867edae6dfc 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -35,10 +35,7 @@ void FlutterWindowsView::SetEngine( // Set up the system channel handlers. auto internal_plugin_messenger = internal_plugin_registrar_->messenger(); - keyboard_hook_handlers_.push_back( - std::make_unique(internal_plugin_messenger)); - keyboard_hook_handlers_.push_back(std::make_unique( - internal_plugin_messenger, this)); + RegisterKeyboardHookHandlers(internal_plugin_messenger, this); platform_handler_ = PlatformHandler::Create(internal_plugin_messenger, this); cursor_handler_ = std::make_unique( internal_plugin_messenger, binding_handler_.get()); @@ -49,6 +46,15 @@ void FlutterWindowsView::SetEngine( binding_handler_->GetDpiScale()); } +void FlutterWindowsView::RegisterKeyboardHookHandlers(flutter::BinaryMessenger* messenger, FlutterWindowsView* view) { + AddKeyboardHookHandler(std::make_unique(messenger)); + AddKeyboardHookHandler(std::make_unique(messenger, view)); +} + +void FlutterWindowsView::AddKeyboardHookHandler(std::unique_ptr handler) { + keyboard_hook_handlers_.push_back(std::move(handler)); +} + uint32_t FlutterWindowsView::GetFrameBufferId(size_t width, size_t height) { // Called on an engine-controlled (non-platform) thread. std::unique_lock lock(resize_mutex_); diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index 25d4f59402ab1..d7e6bca8d74ee 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -116,6 +116,14 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, // |TextInputPluginDelegate| void OnCursorRectUpdated(const Rect& rect) override; + protected: + + // Called to create the keyboard hook handlers. + virtual void RegisterKeyboardHookHandlers(flutter::BinaryMessenger* messenger); + + // Used by RegisterKeyboardHookHandlers to add a new keyboard hook handler. + void AddKeyboardHookHandler(std::unique_ptr handler); + private: // Struct holding the mouse state. The engine doesn't keep track of which // mouse buttons have been pressed, so it's the embedding's responsibility. diff --git a/shell/platform/windows/testing/mock_win32_window.cc b/shell/platform/windows/testing/mock_win32_window.cc index 68e8e3f516f06..f17964aa3fc3c 100644 --- a/shell/platform/windows/testing/mock_win32_window.cc +++ b/shell/platform/windows/testing/mock_win32_window.cc @@ -15,10 +15,10 @@ UINT MockWin32Window::GetDpi() { return GetCurrentDPI(); } -void MockWin32Window::InjectWindowMessage(UINT const message, - WPARAM const wparam, - LPARAM const lparam) { - HandleMessage(message, wparam, lparam); +LRESULT MockWin32Window::InjectWindowMessage(UINT const message, + WPARAM const wparam, + LPARAM const lparam) { + return HandleMessage(message, wparam, lparam); } } // namespace testing diff --git a/shell/platform/windows/testing/mock_win32_window.h b/shell/platform/windows/testing/mock_win32_window.h index ed7320a96f3f0..14839cba9852a 100644 --- a/shell/platform/windows/testing/mock_win32_window.h +++ b/shell/platform/windows/testing/mock_win32_window.h @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WIN32_WINDOW_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WIN32_WINDOW_H_ + #include #include "flutter/shell/platform/windows/win32_window.h" @@ -24,9 +27,9 @@ class MockWin32Window : public Win32Window { UINT GetDpi(); // Simulates a WindowProc message from the OS. - void InjectWindowMessage(UINT const message, - WPARAM const wparam, - LPARAM const lparam); + LRESULT InjectWindowMessage(UINT const message, + WPARAM const wparam, + LPARAM const lparam); MOCK_METHOD1(OnDpiScale, void(unsigned int)); MOCK_METHOD2(OnResize, void(unsigned int, unsigned int)); @@ -42,3 +45,5 @@ class MockWin32Window : public Win32Window { } // namespace testing } // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WIN32_WINDOW_H_ diff --git a/shell/platform/windows/testing/mock_window_binding_handler.cc b/shell/platform/windows/testing/mock_window_binding_handler.cc new file mode 100644 index 0000000000000..7b7f0e17fdae7 --- /dev/null +++ b/shell/platform/windows/testing/mock_window_binding_handler.cc @@ -0,0 +1,15 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h" + +namespace flutter { +namespace testing { + +MockWindowBindingHandler::MockWindowBindingHandler() : WindowBindingHandler(){}; + +MockWindowBindingHandler::~MockWindowBindingHandler() = default; + +} // namespace testing +} // namespace flutter diff --git a/shell/platform/windows/testing/mock_window_binding_handler.h b/shell/platform/windows/testing/mock_window_binding_handler.h new file mode 100644 index 0000000000000..52915a12a17c0 --- /dev/null +++ b/shell/platform/windows/testing/mock_window_binding_handler.h @@ -0,0 +1,36 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WINDOW_BINDING_HANDLER_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WINDOW_BINDING_HANDLER_H_ + +#include + +#include "flutter/shell/platform/windows/window_binding_handler.h" +#include "gmock/gmock.h" + +namespace flutter { +namespace testing { + +/// Mock for the Win32Window base class. +class MockWindowBindingHandler : public WindowBindingHandler { + public: + MockWindowBindingHandler(); + virtual ~MockWindowBindingHandler(); + + // Prevent copying. + MockWindowBindingHandler(MockWindowBindingHandler const&) = delete; + MockWindowBindingHandler& operator=(MockWindowBindingHandler const&) = delete; + + MOCK_METHOD1(SetView, void(WindowBindingHandlerDelegate* view)); + MOCK_METHOD0(GetRenderTarget, WindowsRenderTarget()); + MOCK_METHOD0(GetDpiScale, float()); + MOCK_METHOD0(GetPhysicalWindowBounds, PhysicalWindowBounds()); + MOCK_METHOD1(UpdateFlutterCursor, void(const std::string& cursor_name)); +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WINDOW_BINDING_HANDLER_H_ diff --git a/shell/platform/windows/testing/win32_flutter_window_test.h b/shell/platform/windows/testing/win32_flutter_window_test.h index c48bda6311a8b..4833b4d961844 100644 --- a/shell/platform/windows/testing/win32_flutter_window_test.h +++ b/shell/platform/windows/testing/win32_flutter_window_test.h @@ -2,9 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ + #include #include "flutter/shell/platform/windows/win32_flutter_window.h" +#include "gmock/gmock.h" namespace flutter { namespace testing { @@ -18,10 +22,9 @@ class Win32FlutterWindowTest : public Win32FlutterWindow { // Prevent copying. Win32FlutterWindowTest(Win32FlutterWindowTest const&) = delete; Win32FlutterWindowTest& operator=(Win32FlutterWindowTest const&) = delete; - - private: - bool on_font_change_called_ = false; }; } // namespace testing } // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ diff --git a/shell/platform/windows/win32_flutter_window_unittests.cc b/shell/platform/windows/win32_flutter_window_unittests.cc index e03470dcd34ef..86cd944b745c2 100644 --- a/shell/platform/windows/win32_flutter_window_unittests.cc +++ b/shell/platform/windows/win32_flutter_window_unittests.cc @@ -2,16 +2,189 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "flutter/shell/platform/common/cpp/json_message_codec.h" +#include "flutter/shell/platform/embedder/embedder.h" +#include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" +#include "flutter/shell/platform/windows/flutter_windows_engine.h" +#include "flutter/shell/platform/windows/testing/engine_embedder_api_modifier.h" +#include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h" #include "flutter/shell/platform/windows/testing/win32_flutter_window_test.h" + +#include "gmock/gmock.h" #include "gtest/gtest.h" +#include + +using testing::_; +using testing::Invoke; + namespace flutter { namespace testing { +namespace { + +class SpyKeyEventHandler : public KeyEventHandler { + public: + SpyKeyEventHandler(flutter::BinaryMessenger* messenger, + SendInputDelegate delegate = SendInput) + : KeyEventHandler(messenger, delegate) { + KeyEventHandler* super = reinterpret_cast(this); + ON_CALL(*this, KeyboardHook(_, _, _, _, _, _)) + .WillByDefault(Invoke(super, &KeyEventHandler::KeyboardHook)); + ON_CALL(*this, TextHook(_, _)) + .WillByDefault(Invoke(super, &KeyEventHandler::TextHook)); + } + + MOCK_METHOD6(KeyboardHook, + bool(FlutterWindowsView* window, + int key, + int scancode, + int action, + char32_t character, + bool extended)); + MOCK_METHOD2(TextHook, + void(FlutterWindowsView* window, const std::u16string& text)); +}; + +class SpyTextInputPlugin : public TextInputPlugin { + public: + SpyTextInputPlugin(flutter::BinaryMessenger* messenger) + : TextInputPlugin(messenger) { + TextInputPlugin* super = reinterpret_cast(this); + ON_CALL(*this, KeyboardHook(_, _, _, _, _, _)) + .WillByDefault(Invoke(super, &TextInputPlugin::KeyboardHook)); + ON_CALL(*this, TextHook(_, _)) + .WillByDefault(Invoke(super, &TextInputPlugin::TextHook)); + } + + MOCK_METHOD6(KeyboardHook, + bool(FlutterWindowsView* window, + int key, + int scancode, + int action, + char32_t character, + bool extended)); + MOCK_METHOD2(TextHook, + void(FlutterWindowsView* window, const std::u16string& text)); +}; + +class TestFlutterWindowsView : public FlutterWindowsView { + public: + TestFlutterWindowsView(std::unique_ptr window_binding) + : FlutterWindowsView(std::move(window_binding)) {} + + SpyKeyEventHandler* key_event_handler; + SpyTextInputPlugin* text_input_plugin; + + protected: + void RegisterKeyboardHookHandlers( + flutter::BinaryMessenger* messenger) override { + auto spy_key_event_handler = + std::make_unique(messenger); + auto spy_text_input_plugin = + std::make_unique(messenger); + key_event_handler = spy_key_event_handler.get(); + text_input_plugin = spy_text_input_plugin.get(); + AddKeyboardHookHandler(std::move(spy_key_event_handler)); + AddKeyboardHookHandler(std::move(spy_text_input_plugin)); + } +}; + +struct TestResponseHandle { + FlutterDesktopBinaryReply callback; + void* user_data; +}; + +using TestResponseCallback = bool(); + +// Returns an engine instance configured with dummy project path values. +std::unique_ptr GetTestEngine( + TestResponseCallback test_response) { + FlutterDesktopEngineProperties properties = {}; + properties.assets_path = L"C:\\foo\\flutter_assets"; + properties.icu_data_path = L"C:\\foo\\icudtl.dat"; + properties.aot_library_path = L"C:\\foo\\aot.so"; + FlutterProjectBundle project(properties); + auto engine = std::make_unique(project); + + EngineEmbedderApiModifier modifier(engine.get()); + // Force the non-AOT path unless overridden by the test. + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + + modifier.embedder_api().PlatformMessageCreateResponseHandle = + MOCK_ENGINE_PROC( + PlatformMessageCreateResponseHandle, + [](auto engine, auto data_callback, auto user_data, + auto response_out) { + TestResponseHandle* response_handle = new TestResponseHandle(); + response_handle->user_data = user_data; + response_handle->callback = data_callback; + *response_out = + reinterpret_cast( + response_handle); + return kSuccess; + }); + + modifier.embedder_api().SendPlatformMessage = MOCK_ENGINE_PROC( + SendPlatformMessage, + ([test_response](FLUTTER_API_SYMBOL(FlutterEngine) engine, + const FlutterPlatformMessage* message) { + rapidjson::Document document; + auto& allocator = document.GetAllocator(); + document.SetObject(); + document.AddMember("handled", test_response(), allocator); + auto encoded = + flutter::JsonMessageCodec::GetInstance().EncodeMessage(document); + const TestResponseHandle* response_handle = + reinterpret_cast( + message->response_handle); + if (response_handle->callback != nullptr) { + response_handle->callback(encoded->data(), encoded->size(), + response_handle->user_data); + } + return kSuccess; + })); + + modifier.embedder_api().PlatformMessageReleaseResponseHandle = + MOCK_ENGINE_PROC( + PlatformMessageReleaseResponseHandle, + [](FLUTTER_API_SYMBOL(FlutterEngine) engine, + FlutterPlatformMessageResponseHandle* response) { + const TestResponseHandle* response_handle = + reinterpret_cast(response); + delete response_handle; + return kSuccess; + }); + + return engine; +} +} // namespace + TEST(Win32FlutterWindowTest, CreateDestroy) { Win32FlutterWindowTest window(800, 600); ASSERT_TRUE(TRUE); } +TEST(Win32FlutterWindowTest, KeyEventPropagation) { + Win32FlutterWindowTest window(800, 600); + auto window_binding_handler = std::make_unique(); + EXPECT_CALL(*window_binding_handler, SetView(_)).Times(1); + EXPECT_CALL(*window_binding_handler, GetRenderTarget()).Times(1); + EXPECT_CALL(*window_binding_handler, GetPhysicalWindowBounds()).Times(1); + EXPECT_CALL(*window_binding_handler, GetDpiScale()).Times(1); + TestFlutterWindowsView flutter_windows_view( + std::move(window_binding_handler)); + window.SetView(&flutter_windows_view); + flutter_windows_view.SetEngine( + std::move(GetTestEngine([]() { return true; }))); + EXPECT_CALL(*flutter_windows_view.key_event_handler, + KeyboardHook(_, 65, 30, WM_KEYDOWN, 65, false)) + .Times(1); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, 65, 30, WM_KEYDOWN, 65, false)) + .Times(0); + EXPECT_EQ(flutter_windows_view.OnKey(65, 30, WM_KEYDOWN, 65, false), true); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/win32_window.cc b/shell/platform/windows/win32_window.cc index 8b9d9ab70f276..ed4d1088f1088 100644 --- a/shell/platform/windows/win32_window.cc +++ b/shell/platform/windows/win32_window.cc @@ -5,6 +5,7 @@ #include "flutter/shell/platform/windows/win32_window.h" #include +#include #include "win32_dpi_utils.h" @@ -210,6 +211,7 @@ Win32Window::HandleMessage(UINT const message, case WM_CHAR: case WM_SYSCHAR: { static wchar_t s_pending_high_surrogate = 0; + std::cerr << "Handling text message " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; wchar_t character = static_cast(wparam); std::u16string text({character}); @@ -231,6 +233,8 @@ Win32Window::HandleMessage(UINT const message, if (keycode_for_char_message_ != 0) { const unsigned int scancode = (lparam >> 16) & 0xff; const bool extended = ((lparam >> 24) & 0x01) == 0x01; + std::cerr << "Sending key message to OnKey from WM_CHAR " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; + bool handled = OnKey(keycode_for_char_message_, scancode, WM_KEYDOWN, code_point, extended); keycode_for_char_message_ = 0; @@ -261,12 +265,14 @@ Win32Window::HandleMessage(UINT const message, case WM_SYSKEYDOWN: case WM_KEYUP: case WM_SYSKEYUP: + std::cerr << "Handling key message " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; const bool is_keydown_message = (message == WM_KEYDOWN || message == WM_SYSKEYDOWN); // Check if this key produces a character. If so, the key press should // be sent with the character produced at WM_CHAR. Store the produced // keycode (it's not accessible from WM_CHAR) to be used in WM_CHAR. const unsigned int character = MapVirtualKey(wparam, MAPVK_VK_TO_CHAR); + std::cerr << "char is " << character << std::endl; if (character > 0 && is_keydown_message) { keycode_for_char_message_ = wparam; break; @@ -279,6 +285,7 @@ Win32Window::HandleMessage(UINT const message, keyCode = MapVirtualKey(scancode, MAPVK_VSC_TO_VK_EX); } const int action = is_keydown_message ? WM_KEYDOWN : WM_KEYUP; + std::cerr << "Sending key message to OnKey " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; if (OnKey(keyCode, scancode, action, 0, extended)) { return 0; } diff --git a/shell/platform/windows/win32_window_unittests.cc b/shell/platform/windows/win32_window_unittests.cc index ea860114e53ed..14d60441533d9 100644 --- a/shell/platform/windows/win32_window_unittests.cc +++ b/shell/platform/windows/win32_window_unittests.cc @@ -5,6 +5,8 @@ #include "flutter/shell/platform/windows/testing/mock_win32_window.h" #include "gtest/gtest.h" +using testing::_; + namespace flutter { namespace testing { @@ -37,5 +39,46 @@ TEST(MockWin32Window, HorizontalScroll) { window.InjectWindowMessage(WM_MOUSEHWHEEL, MAKEWPARAM(0, scroll_amount), 0); } +LPARAM CreateKeyEventLparam(USHORT RepeatCount, USHORT ScanCode, bool extended, bool ContextCode, bool PreviousKeyState,bool TransitionState) +{ + return ( + (LPARAM(TransitionState) << 31) | + (LPARAM(PreviousKeyState) << 30) | + (LPARAM(ContextCode) << 29) | + (LPARAM(extended ? 0x1 : 0x0) << 24) | + (LPARAM(ScanCode) << 16) | + LPARAM(RepeatCount) + ); +} + +TEST(MockWin32Window, KeyDown) { + MockWin32Window window; + EXPECT_CALL(window, OnKey(_, _, _, _, _)).Times(1); + LPARAM lparam = CreateKeyEventLparam(1, 42, false, 0, 1, 1); + // send a "Shift" key down event. + window.InjectWindowMessage(WM_KEYDOWN, 16, lparam); +} + +TEST(MockWin32Window, KeyUp) { + MockWin32Window window; + EXPECT_CALL(window, OnKey(_, _, _, _, _)).Times(1); + LPARAM lparam = CreateKeyEventLparam(1, 42, false, 0, 1, 1); + // send a "Shift" key up event. + window.InjectWindowMessage(WM_KEYUP, 16, lparam); +} + +TEST(MockWin32Window, KeyDownPrintable) { + MockWin32Window window; + LPARAM lparam = CreateKeyEventLparam(1, 30, false, 0, 1, 1); + // OnKey shouldn't be called until the WM_CHAR message. + EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 65, false)).Times(0); + // send a "A" key down event. + window.InjectWindowMessage(WM_KEYDOWN, 65, lparam); + + EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 65, false)).Times(1); + EXPECT_CALL(window, OnText(_)).Times(1); + window.InjectWindowMessage(WM_CHAR, 65, lparam); +} + } // namespace testing } // namespace flutter From 995c65ff42272e644cec224a9d020151bfcfbb60 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 15 Jan 2021 16:45:05 -0800 Subject: [PATCH 3/7] Tests working --- .../platform/windows/flutter_windows_view.cc | 9 +- shell/platform/windows/flutter_windows_view.h | 7 +- .../windows/testing/mock_win32_window.h | 2 +- .../testing/mock_window_binding_handler.h | 1 + .../testing/win32_flutter_window_test.h | 2 +- .../windows/win32_flutter_window_unittests.cc | 396 +++++++++++++++--- shell/platform/windows/win32_window.cc | 5 - .../windows/win32_window_unittests.cc | 19 +- 8 files changed, 355 insertions(+), 86 deletions(-) diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index b2867edae6dfc..88e1417da731a 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -35,7 +35,7 @@ void FlutterWindowsView::SetEngine( // Set up the system channel handlers. auto internal_plugin_messenger = internal_plugin_registrar_->messenger(); - RegisterKeyboardHookHandlers(internal_plugin_messenger, this); + RegisterKeyboardHookHandlers(internal_plugin_messenger); platform_handler_ = PlatformHandler::Create(internal_plugin_messenger, this); cursor_handler_ = std::make_unique( internal_plugin_messenger, binding_handler_.get()); @@ -46,12 +46,13 @@ void FlutterWindowsView::SetEngine( binding_handler_->GetDpiScale()); } -void FlutterWindowsView::RegisterKeyboardHookHandlers(flutter::BinaryMessenger* messenger, FlutterWindowsView* view) { +void FlutterWindowsView::RegisterKeyboardHookHandlers(flutter::BinaryMessenger* messenger) { AddKeyboardHookHandler(std::make_unique(messenger)); - AddKeyboardHookHandler(std::make_unique(messenger, view)); + AddKeyboardHookHandler(std::make_unique(messenger, this)); } -void FlutterWindowsView::AddKeyboardHookHandler(std::unique_ptr handler) { +void FlutterWindowsView::AddKeyboardHookHandler( + std::unique_ptr handler) { keyboard_hook_handlers_.push_back(std::move(handler)); } diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index d7e6bca8d74ee..c7423acff6731 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -117,12 +117,13 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, void OnCursorRectUpdated(const Rect& rect) override; protected: - // Called to create the keyboard hook handlers. - virtual void RegisterKeyboardHookHandlers(flutter::BinaryMessenger* messenger); + virtual void RegisterKeyboardHookHandlers( + flutter::BinaryMessenger* messenger); // Used by RegisterKeyboardHookHandlers to add a new keyboard hook handler. - void AddKeyboardHookHandler(std::unique_ptr handler); + void AddKeyboardHookHandler( + std::unique_ptr handler); private: // Struct holding the mouse state. The engine doesn't keep track of which diff --git a/shell/platform/windows/testing/mock_win32_window.h b/shell/platform/windows/testing/mock_win32_window.h index 14839cba9852a..dc7765e95959a 100644 --- a/shell/platform/windows/testing/mock_win32_window.h +++ b/shell/platform/windows/testing/mock_win32_window.h @@ -46,4 +46,4 @@ class MockWin32Window : public Win32Window { } // namespace testing } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WIN32_WINDOW_H_ +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WIN32_WINDOW_H_ diff --git a/shell/platform/windows/testing/mock_window_binding_handler.h b/shell/platform/windows/testing/mock_window_binding_handler.h index 52915a12a17c0..e8b18229f2123 100644 --- a/shell/platform/windows/testing/mock_window_binding_handler.h +++ b/shell/platform/windows/testing/mock_window_binding_handler.h @@ -26,6 +26,7 @@ class MockWindowBindingHandler : public WindowBindingHandler { MOCK_METHOD1(SetView, void(WindowBindingHandlerDelegate* view)); MOCK_METHOD0(GetRenderTarget, WindowsRenderTarget()); MOCK_METHOD0(GetDpiScale, float()); + MOCK_METHOD0(OnWindowResized, void()); MOCK_METHOD0(GetPhysicalWindowBounds, PhysicalWindowBounds()); MOCK_METHOD1(UpdateFlutterCursor, void(const std::string& cursor_name)); }; diff --git a/shell/platform/windows/testing/win32_flutter_window_test.h b/shell/platform/windows/testing/win32_flutter_window_test.h index 4833b4d961844..04433ddf26e24 100644 --- a/shell/platform/windows/testing/win32_flutter_window_test.h +++ b/shell/platform/windows/testing/win32_flutter_window_test.h @@ -27,4 +27,4 @@ class Win32FlutterWindowTest : public Win32FlutterWindow { } // namespace testing } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ diff --git a/shell/platform/windows/win32_flutter_window_unittests.cc b/shell/platform/windows/win32_flutter_window_unittests.cc index 86cd944b745c2..52ed904ae4d4c 100644 --- a/shell/platform/windows/win32_flutter_window_unittests.cc +++ b/shell/platform/windows/win32_flutter_window_unittests.cc @@ -23,16 +23,20 @@ namespace testing { namespace { -class SpyKeyEventHandler : public KeyEventHandler { +// A key event handler that can be spied on while it forwards calls to the real +// key event handler. +class SpyKeyEventHandler : public KeyboardHookHandler { public: SpyKeyEventHandler(flutter::BinaryMessenger* messenger, - SendInputDelegate delegate = SendInput) - : KeyEventHandler(messenger, delegate) { - KeyEventHandler* super = reinterpret_cast(this); + KeyEventHandler::SendInputDelegate delegate) { + real_implementation_ = + std::make_unique(messenger, delegate); ON_CALL(*this, KeyboardHook(_, _, _, _, _, _)) - .WillByDefault(Invoke(super, &KeyEventHandler::KeyboardHook)); + .WillByDefault( + Invoke(real_implementation_.get(), &KeyEventHandler::KeyboardHook)); ON_CALL(*this, TextHook(_, _)) - .WillByDefault(Invoke(super, &KeyEventHandler::TextHook)); + .WillByDefault( + Invoke(real_implementation_.get(), &KeyEventHandler::TextHook)); } MOCK_METHOD6(KeyboardHook, @@ -44,17 +48,23 @@ class SpyKeyEventHandler : public KeyEventHandler { bool extended)); MOCK_METHOD2(TextHook, void(FlutterWindowsView* window, const std::u16string& text)); + + private: + std::unique_ptr real_implementation_; }; -class SpyTextInputPlugin : public TextInputPlugin { +// A text input plugin that can be spied on while it forwards calls to the real +// text input plugin. +class SpyTextInputPlugin : public KeyboardHookHandler { public: - SpyTextInputPlugin(flutter::BinaryMessenger* messenger) - : TextInputPlugin(messenger) { - TextInputPlugin* super = reinterpret_cast(this); + SpyTextInputPlugin(flutter::BinaryMessenger* messenger) { + real_implementation_ = std::make_unique(messenger); ON_CALL(*this, KeyboardHook(_, _, _, _, _, _)) - .WillByDefault(Invoke(super, &TextInputPlugin::KeyboardHook)); + .WillByDefault( + Invoke(real_implementation_.get(), &TextInputPlugin::KeyboardHook)); ON_CALL(*this, TextHook(_, _)) - .WillByDefault(Invoke(super, &TextInputPlugin::TextHook)); + .WillByDefault( + Invoke(real_implementation_.get(), &TextInputPlugin::TextHook)); } MOCK_METHOD6(KeyboardHook, @@ -66,12 +76,19 @@ class SpyTextInputPlugin : public TextInputPlugin { bool extended)); MOCK_METHOD2(TextHook, void(FlutterWindowsView* window, const std::u16string& text)); + + private: + std::unique_ptr real_implementation_; }; +// A FlutterWindowsView that overrides the RegisterKeyboardHookHandlers function +// to register the keyboard hook handlers that can be spied upon. class TestFlutterWindowsView : public FlutterWindowsView { public: - TestFlutterWindowsView(std::unique_ptr window_binding) - : FlutterWindowsView(std::move(window_binding)) {} + TestFlutterWindowsView(std::unique_ptr window_binding, + KeyEventHandler::SendInputDelegate send_input) + : FlutterWindowsView(std::move(window_binding)), + send_input_(send_input) {} SpyKeyEventHandler* key_event_handler; SpyTextInputPlugin* text_input_plugin; @@ -80,7 +97,7 @@ class TestFlutterWindowsView : public FlutterWindowsView { void RegisterKeyboardHookHandlers( flutter::BinaryMessenger* messenger) override { auto spy_key_event_handler = - std::make_unique(messenger); + std::make_unique(messenger, send_input_); auto spy_text_input_plugin = std::make_unique(messenger); key_event_handler = spy_key_event_handler.get(); @@ -88,18 +105,58 @@ class TestFlutterWindowsView : public FlutterWindowsView { AddKeyboardHookHandler(std::move(spy_key_event_handler)); AddKeyboardHookHandler(std::move(spy_text_input_plugin)); } + + private: + KeyEventHandler::SendInputDelegate send_input_; }; +// 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 +// overridden function. struct TestResponseHandle { FlutterDesktopBinaryReply callback; void* user_data; }; -using TestResponseCallback = bool(); +class MockWin32FlutterWindow : public Win32FlutterWindow { + public: + MockWin32FlutterWindow() : Win32FlutterWindow(800, 600) {} + virtual ~MockWin32FlutterWindow() {} -// Returns an engine instance configured with dummy project path values. -std::unique_ptr GetTestEngine( - TestResponseCallback test_response) { + // Prevent copying. + MockWin32FlutterWindow(MockWin32FlutterWindow const&) = delete; + MockWin32FlutterWindow& operator=(MockWin32FlutterWindow const&) = delete; + + // Wrapper for GetCurrentDPI() which is a protected method. + UINT GetDpi() { return GetCurrentDPI(); } + + // Simulates a WindowProc message from the OS. + LRESULT InjectWindowMessage(UINT const message, + WPARAM const wparam, + LPARAM const lparam) { + return HandleMessage(message, wparam, lparam); + } + + MOCK_METHOD1(OnDpiScale, void(unsigned int)); + MOCK_METHOD2(OnResize, void(unsigned int, unsigned int)); + MOCK_METHOD2(OnPointerMove, void(double, double)); + MOCK_METHOD3(OnPointerDown, void(double, double, UINT)); + MOCK_METHOD3(OnPointerUp, void(double, double, UINT)); + MOCK_METHOD0(OnPointerLeave, void()); + MOCK_METHOD0(OnSetCursor, void()); + MOCK_METHOD2(OnScroll, void(double, double)); +}; + +// The static value to return as the "handled" value from the framework for key +// events. Individual tests set this to change the framework response that the +// engine simulates. +static bool test_response = false; + +// Returns an engine instance configured with dummy project path values, and +// overridden methods for sending platform messages, so that the engine can +// respond as if the framework were connected. +std::unique_ptr GetTestEngine() { FlutterDesktopEngineProperties properties = {}; properties.assets_path = L"C:\\foo\\flutter_assets"; properties.icu_data_path = L"C:\\foo\\icudtl.dat"; @@ -112,27 +169,22 @@ std::unique_ptr GetTestEngine( modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; modifier.embedder_api().PlatformMessageCreateResponseHandle = - MOCK_ENGINE_PROC( - PlatformMessageCreateResponseHandle, - [](auto engine, auto data_callback, auto user_data, - auto response_out) { - TestResponseHandle* response_handle = new TestResponseHandle(); - response_handle->user_data = user_data; - response_handle->callback = data_callback; - *response_out = - reinterpret_cast( - response_handle); - return kSuccess; - }); - - modifier.embedder_api().SendPlatformMessage = MOCK_ENGINE_PROC( - SendPlatformMessage, - ([test_response](FLUTTER_API_SYMBOL(FlutterEngine) engine, - const FlutterPlatformMessage* message) { + [](auto engine, auto data_callback, auto user_data, auto response_out) { + TestResponseHandle* response_handle = new TestResponseHandle(); + response_handle->user_data = user_data; + response_handle->callback = data_callback; + *response_out = reinterpret_cast( + response_handle); + return kSuccess; + }; + + modifier.embedder_api().SendPlatformMessage = + [](FLUTTER_API_SYMBOL(FlutterEngine) engine, + const FlutterPlatformMessage* message) { rapidjson::Document document; auto& allocator = document.GetAllocator(); document.SetObject(); - document.AddMember("handled", test_response(), allocator); + document.AddMember("handled", test_response, allocator); auto encoded = flutter::JsonMessageCodec::GetInstance().EncodeMessage(document); const TestResponseHandle* response_handle = @@ -143,21 +195,41 @@ std::unique_ptr GetTestEngine( response_handle->user_data); } return kSuccess; - })); + }; modifier.embedder_api().PlatformMessageReleaseResponseHandle = - MOCK_ENGINE_PROC( - PlatformMessageReleaseResponseHandle, - [](FLUTTER_API_SYMBOL(FlutterEngine) engine, - FlutterPlatformMessageResponseHandle* response) { - const TestResponseHandle* response_handle = - reinterpret_cast(response); - delete response_handle; - return kSuccess; - }); + [](FLUTTER_API_SYMBOL(FlutterEngine) engine, + FlutterPlatformMessageResponseHandle* response) { + const TestResponseHandle* response_handle = + reinterpret_cast(response); + delete response_handle; + return kSuccess; + }; return engine; } + +// Creates a valid Windows LPARAM for WM_KEYDOWN and WM_CHAR from parameters +// given. +static LPARAM CreateKeyEventLparam(USHORT ScanCode, + bool extended = false, + USHORT RepeatCount = 1, + bool ContextCode = 0, + bool PreviousKeyState = 1, + bool TransitionState = 1) { + return ((LPARAM(TransitionState) << 31) | (LPARAM(PreviousKeyState) << 30) | + (LPARAM(ContextCode) << 29) | (LPARAM(extended ? 0x1 : 0x0) << 24) | + (LPARAM(ScanCode) << 16) | LPARAM(RepeatCount)); +} + +// A struc to hold simulated events that will be delivered after the framework +// response is handled. +struct SimulatedEvent { + UINT message; + WPARAM wparam; + LPARAM lparam; +}; + } // namespace TEST(Win32FlutterWindowTest, CreateDestroy) { @@ -165,25 +237,225 @@ TEST(Win32FlutterWindowTest, CreateDestroy) { ASSERT_TRUE(TRUE); } -TEST(Win32FlutterWindowTest, KeyEventPropagation) { - Win32FlutterWindowTest window(800, 600); - auto window_binding_handler = std::make_unique(); - EXPECT_CALL(*window_binding_handler, SetView(_)).Times(1); - EXPECT_CALL(*window_binding_handler, GetRenderTarget()).Times(1); - EXPECT_CALL(*window_binding_handler, GetPhysicalWindowBounds()).Times(1); - EXPECT_CALL(*window_binding_handler, GetDpiScale()).Times(1); +// Tests key event propagation of printable character key down events. +TEST(Win32FlutterWindowTest, CharKeyDownPropagation) { + constexpr WPARAM virtual_key = 65; // The "A" key, which produces a character + constexpr WPARAM scan_code = 30; + constexpr char32_t character = 65; + + MockWin32FlutterWindow win32window; + std::deque pending_events; + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler)); - window.SetView(&flutter_windows_view); - flutter_windows_view.SetEngine( - std::move(GetTestEngine([]() { return true; }))); - EXPECT_CALL(*flutter_windows_view.key_event_handler, - KeyboardHook(_, 65, 30, WM_KEYDOWN, 65, false)) + std::move(window_binding_handler), + [&pending_events](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { + // Simulate the event loop by just sending the event sent to + // "SendInput" directly to the window. + const KEYBDINPUT kbdinput = pInputs->ki; + const UINT message = + (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; + const LPARAM lparam = CreateKeyEventLparam( + kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); + // 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). + pending_events.push_back(SimulatedEvent{message, virtual_key, lparam}); + if ((kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { + pending_events.push_back( + SimulatedEvent{WM_CHAR, virtual_key, lparam}); + } + return 1; + }); + win32window.SetView(&flutter_windows_view); + + // Test an event not handled by the framework + test_response = false; + flutter_windows_view.SetEngine(std::move(GetTestEngine())); + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(2); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) .Times(1); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(1); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(1); + LPARAM lparam = CreateKeyEventLparam(scan_code); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_CHAR, virtual_key, lparam), 0); + while (pending_events.size() > 0) { + SimulatedEvent event = pending_events.front(); + win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); + pending_events.pop_front(); + } + pending_events.clear(); + + // Test an event handled by the framework + test_response = true; + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(1); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(0); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(0); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_CHAR, virtual_key, lparam), 0); + while (pending_events.size() > 0) { + SimulatedEvent event = pending_events.front(); + win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); + pending_events.pop_front(); + } +} + +// Tests key event propagation of modifier key down events. +TEST(Win32FlutterWindowTest, ModifierKeyDownPropagation) { + constexpr WPARAM virtual_key = VK_LSHIFT; + constexpr WPARAM scan_code = 20; + constexpr char32_t character = 0; + MockWin32FlutterWindow win32window; + std::deque pending_events; + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + TestFlutterWindowsView flutter_windows_view( + std::move(window_binding_handler), + [&pending_events](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { + // Simulate the event loop by just sending the event sent to + // "SendInput" directly to the window. + const KEYBDINPUT kbdinput = pInputs->ki; + const UINT message = + (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; + const LPARAM lparam = CreateKeyEventLparam( + kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); + // SendEvent will typically add the events to a queue, so we do too. + // 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). + pending_events.push_back(SimulatedEvent{message, virtual_key, lparam}); + return 1; + }); + win32window.SetView(&flutter_windows_view); + + // Test an event not handled by the framework + test_response = false; + flutter_windows_view.SetEngine(std::move(GetTestEngine())); + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(2) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(0); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(0); + LPARAM lparam = CreateKeyEventLparam(scan_code); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + while (pending_events.size() > 0) { + SimulatedEvent event = pending_events.front(); + win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); + pending_events.pop_front(); + } + pending_events.clear(); + + // Test an event handled by the framework + test_response = true; + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + while (pending_events.size() > 0) { + SimulatedEvent event = pending_events.front(); + win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); + pending_events.pop_front(); + } +} + +// Tests key event propagation of non-printable, non-modifier key down events. +TEST(Win32FlutterWindowTest, NonPrintableKeyDownPropagation) { + constexpr WPARAM virtual_key = VK_LEFT; + constexpr WPARAM scan_code = 10; + constexpr char32_t character = 0; + MockWin32FlutterWindow win32window; + std::deque pending_events; + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + TestFlutterWindowsView flutter_windows_view( + std::move(window_binding_handler), + [&pending_events](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { + // Simulate the event loop by just sending the event sent to + // "SendInput" directly to the window. + const KEYBDINPUT kbdinput = pInputs->ki; + const UINT message = + (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; + const LPARAM lparam = CreateKeyEventLparam( + kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); + // SendEvent will typically add the events to a queue, so we do too. + // 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). + pending_events.push_back(SimulatedEvent{message, virtual_key, lparam}); + return 1; + }); + win32window.SetView(&flutter_windows_view); + + // Test an event not handled by the framework + test_response = false; + flutter_windows_view.SetEngine(std::move(GetTestEngine())); + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(2) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(0); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(0); + LPARAM lparam = CreateKeyEventLparam(scan_code); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + while (pending_events.size() > 0) { + SimulatedEvent event = pending_events.front(); + win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); + pending_events.pop_front(); + } + pending_events.clear(); + + // Test an event handled by the framework + test_response = true; + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(1) + .RetiresOnSaturation(); EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, 65, 30, WM_KEYDOWN, 65, false)) + KeyboardHook(_, _, _, _, _, _)) .Times(0); - EXPECT_EQ(flutter_windows_view.OnKey(65, 30, WM_KEYDOWN, 65, false), true); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + while (pending_events.size() > 0) { + SimulatedEvent event = pending_events.front(); + win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); + pending_events.pop_front(); + } } } // namespace testing diff --git a/shell/platform/windows/win32_window.cc b/shell/platform/windows/win32_window.cc index ed4d1088f1088..2c75618a2ea83 100644 --- a/shell/platform/windows/win32_window.cc +++ b/shell/platform/windows/win32_window.cc @@ -211,7 +211,6 @@ Win32Window::HandleMessage(UINT const message, case WM_CHAR: case WM_SYSCHAR: { static wchar_t s_pending_high_surrogate = 0; - std::cerr << "Handling text message " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; wchar_t character = static_cast(wparam); std::u16string text({character}); @@ -233,7 +232,6 @@ Win32Window::HandleMessage(UINT const message, if (keycode_for_char_message_ != 0) { const unsigned int scancode = (lparam >> 16) & 0xff; const bool extended = ((lparam >> 24) & 0x01) == 0x01; - std::cerr << "Sending key message to OnKey from WM_CHAR " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; bool handled = OnKey(keycode_for_char_message_, scancode, WM_KEYDOWN, code_point, extended); @@ -265,14 +263,12 @@ Win32Window::HandleMessage(UINT const message, case WM_SYSKEYDOWN: case WM_KEYUP: case WM_SYSKEYUP: - std::cerr << "Handling key message " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; const bool is_keydown_message = (message == WM_KEYDOWN || message == WM_SYSKEYDOWN); // Check if this key produces a character. If so, the key press should // be sent with the character produced at WM_CHAR. Store the produced // keycode (it's not accessible from WM_CHAR) to be used in WM_CHAR. const unsigned int character = MapVirtualKey(wparam, MAPVK_VK_TO_CHAR); - std::cerr << "char is " << character << std::endl; if (character > 0 && is_keydown_message) { keycode_for_char_message_ = wparam; break; @@ -285,7 +281,6 @@ Win32Window::HandleMessage(UINT const message, keyCode = MapVirtualKey(scancode, MAPVK_VSC_TO_VK_EX); } const int action = is_keydown_message ? WM_KEYDOWN : WM_KEYUP; - std::cerr << "Sending key message to OnKey " << message << " with wparam: " << wparam << " and lparam:" << lparam << std::endl; if (OnKey(keyCode, scancode, action, 0, extended)) { return 0; } diff --git a/shell/platform/windows/win32_window_unittests.cc b/shell/platform/windows/win32_window_unittests.cc index 14d60441533d9..d1b61dc7b9dec 100644 --- a/shell/platform/windows/win32_window_unittests.cc +++ b/shell/platform/windows/win32_window_unittests.cc @@ -39,16 +39,15 @@ TEST(MockWin32Window, HorizontalScroll) { window.InjectWindowMessage(WM_MOUSEHWHEEL, MAKEWPARAM(0, scroll_amount), 0); } -LPARAM CreateKeyEventLparam(USHORT RepeatCount, USHORT ScanCode, bool extended, bool ContextCode, bool PreviousKeyState,bool TransitionState) -{ - return ( - (LPARAM(TransitionState) << 31) | - (LPARAM(PreviousKeyState) << 30) | - (LPARAM(ContextCode) << 29) | - (LPARAM(extended ? 0x1 : 0x0) << 24) | - (LPARAM(ScanCode) << 16) | - LPARAM(RepeatCount) - ); +static LPARAM CreateKeyEventLparam(USHORT RepeatCount, + USHORT ScanCode, + bool extended, + bool ContextCode, + bool PreviousKeyState, + bool TransitionState) { + return ((LPARAM(TransitionState) << 31) | (LPARAM(PreviousKeyState) << 30) | + (LPARAM(ContextCode) << 29) | (LPARAM(extended ? 0x1 : 0x0) << 24) | + (LPARAM(ScanCode) << 16) | LPARAM(RepeatCount)); } TEST(MockWin32Window, KeyDown) { From bbb0253d7c4e68b43d01758bba41be4d66575da0 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 19 Jan 2021 14:26:30 -0800 Subject: [PATCH 4/7] Add missing file --- ci/licenses_golden/licenses_flutter | 1 + .../windows/key_event_handler_unittests.cc | 118 ++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 shell/platform/windows/key_event_handler_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 41ed21d3bff57..40a5df92483d1 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1475,6 +1475,7 @@ FILE: ../../../flutter/shell/platform/windows/flutter_windows_win32.cc FILE: ../../../flutter/shell/platform/windows/flutter_windows_winuwp.cc FILE: ../../../flutter/shell/platform/windows/key_event_handler.cc FILE: ../../../flutter/shell/platform/windows/key_event_handler.h +FILE: ../../../flutter/shell/platform/windows/key_event_handler_unittests.cc FILE: ../../../flutter/shell/platform/windows/keyboard_hook_handler.h FILE: ../../../flutter/shell/platform/windows/platform_handler.cc FILE: ../../../flutter/shell/platform/windows/platform_handler.h diff --git a/shell/platform/windows/key_event_handler_unittests.cc b/shell/platform/windows/key_event_handler_unittests.cc new file mode 100644 index 0000000000000..398afc5efc68a --- /dev/null +++ b/shell/platform/windows/key_event_handler_unittests.cc @@ -0,0 +1,118 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#include "flutter/shell/platform/windows/key_event_handler.h" + +#include +#include + +#include "flutter/shell/platform/common/cpp/json_message_codec.h" +#include "flutter/shell/platform/windows/flutter_windows_view.h" +#include "flutter/shell/platform/windows/testing/test_binary_messenger.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace flutter { +namespace testing { + +static constexpr char kScanCodeKey[] = "scanCode"; +static constexpr int kHandledScanCode = 20; +static constexpr int kUnhandledScanCode = 21; + +std::unique_ptr> CreateResponse(bool handled) { + auto response_doc = + std::make_unique(rapidjson::kObjectType); + auto& allocator = response_doc->GetAllocator(); + response_doc->AddMember("handled", handled, allocator); + return JsonMessageCodec::GetInstance().EncodeMessage(*response_doc); +} + +TEST(KeyEventHandlerTest, KeyboardHookHandling) { + auto handled_message = CreateResponse(true); + auto unhandled_message = CreateResponse(false); + int received_scancode = 0; + + TestBinaryMessenger messenger( + [&received_scancode, &handled_message, &unhandled_message]( + const std::string& channel, const uint8_t* message, + size_t message_size, BinaryReply reply) { + if (channel == "flutter/keyevent") { + auto message_doc = JsonMessageCodec::GetInstance().DecodeMessage( + message, message_size); + received_scancode = (*message_doc)[kScanCodeKey].GetInt(); + if (received_scancode == kHandledScanCode) { + reply(handled_message->data(), handled_message->size()); + } else { + reply(unhandled_message->data(), unhandled_message->size()); + } + } + }); + + int redispatch_scancode = 0; + KeyEventHandler handler(&messenger, + [&redispatch_scancode](UINT cInputs, LPINPUT pInputs, + int cbSize) -> UINT { + EXPECT_TRUE(cbSize > 0); + redispatch_scancode = pInputs->ki.wScan; + return 1; + }); + + handler.KeyboardHook(nullptr, 64, kHandledScanCode, WM_KEYDOWN, L'a', false); + EXPECT_EQ(received_scancode, kHandledScanCode); + EXPECT_EQ(redispatch_scancode, 0); + received_scancode = 0; + handler.KeyboardHook(nullptr, 64, kUnhandledScanCode, WM_KEYDOWN, L'b', + false); + EXPECT_EQ(received_scancode, kUnhandledScanCode); + EXPECT_EQ(redispatch_scancode, kUnhandledScanCode); +} + +TEST(KeyEventHandlerTest, ExtendedKeysAreSentToRedispatch) { + auto handled_message = CreateResponse(true); + auto unhandled_message = CreateResponse(false); + int received_scancode = 0; + bool is_extended_key = false; + + TestBinaryMessenger messenger( + [&received_scancode, &handled_message, &unhandled_message]( + const std::string& channel, const uint8_t* message, + size_t message_size, BinaryReply reply) { + if (channel == "flutter/keyevent") { + auto message_doc = JsonMessageCodec::GetInstance().DecodeMessage( + message, message_size); + received_scancode = (*message_doc)[kScanCodeKey].GetInt(); + if (received_scancode == kHandledScanCode) { + reply(handled_message->data(), handled_message->size()); + } else { + reply(unhandled_message->data(), unhandled_message->size()); + } + } + }); + + int redispatch_scancode = 0; + KeyEventHandler handler( + &messenger, + [&redispatch_scancode, &is_extended_key](UINT cInputs, LPINPUT pInputs, + int cbSize) -> UINT { + EXPECT_TRUE(cbSize > 0); + redispatch_scancode = pInputs->ki.wScan; + is_extended_key = (pInputs->ki.dwFlags & KEYEVENTF_EXTENDEDKEY) != 0; + return 1; + }); + + // Extended key flag is passed to redispatched events if set. + handler.KeyboardHook(nullptr, 64, kUnhandledScanCode, WM_KEYDOWN, L'b', true); + EXPECT_EQ(received_scancode, kUnhandledScanCode); + EXPECT_EQ(redispatch_scancode, kUnhandledScanCode); + EXPECT_EQ(is_extended_key, true); + + // Extended key flag is not passed to redispatched events if not set. + handler.KeyboardHook(nullptr, 64, kUnhandledScanCode, WM_KEYDOWN, L'b', + false); + EXPECT_EQ(received_scancode, kUnhandledScanCode); + EXPECT_EQ(redispatch_scancode, kUnhandledScanCode); + EXPECT_EQ(is_extended_key, false); +} + +} // namespace testing +} // namespace flutter From b22d1894eba1189456be0a3f2590800c82ddbdb2 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 19 Jan 2021 16:26:59 -0800 Subject: [PATCH 5/7] Some cleanup --- .../flutter_windows_engine_unittests.cc | 2 +- shell/platform/windows/flutter_windows_view.h | 4 +-- .../testing/win32_flutter_window_test.h | 3 -- shell/platform/windows/win32_window.cc | 1 - .../windows/win32_window_unittests.cc | 33 +++++++++++-------- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 6d48818af3db2..fa6d87e72b186 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -102,7 +102,7 @@ TEST(FlutterWindowsEngine, SendPlatformMessageWithoutResponse) { const char* channel = "test"; const std::vector test_message = {1, 2, 3, 4}; - // Without a responses, SendPlatformMessage should be a simple pass-through. + // Without a response, SendPlatformMessage should be a simple pass-through. bool called = false; modifier.embedder_api().SendPlatformMessage = MOCK_ENGINE_PROC( SendPlatformMessage, ([&called, test_message](auto engine, auto message) { diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index c7423acff6731..0998f04912685 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -38,14 +38,14 @@ inline constexpr uint32_t kWindowFrameBufferID = 0; class FlutterWindowsView : public WindowBindingHandlerDelegate, public TextInputPluginDelegate { public: - // Creates a FlutterWindowsView with the given implementator of + // Creates a FlutterWindowsView with the given implementor of // WindowBindingHandler. // // In order for object to render Flutter content the SetEngine method must be // called with a valid FlutterWindowsEngine instance. FlutterWindowsView(std::unique_ptr window_binding); - ~FlutterWindowsView(); + virtual ~FlutterWindowsView(); // Configures the window instance with an instance of a running Flutter // engine. diff --git a/shell/platform/windows/testing/win32_flutter_window_test.h b/shell/platform/windows/testing/win32_flutter_window_test.h index 04433ddf26e24..bf42b6c92a0a8 100644 --- a/shell/platform/windows/testing/win32_flutter_window_test.h +++ b/shell/platform/windows/testing/win32_flutter_window_test.h @@ -5,10 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_WIN32_FLUTTER_WINDOW_TEST_H_ -#include - #include "flutter/shell/platform/windows/win32_flutter_window.h" -#include "gmock/gmock.h" namespace flutter { namespace testing { diff --git a/shell/platform/windows/win32_window.cc b/shell/platform/windows/win32_window.cc index 2c75618a2ea83..01432cfb0f6fe 100644 --- a/shell/platform/windows/win32_window.cc +++ b/shell/platform/windows/win32_window.cc @@ -5,7 +5,6 @@ #include "flutter/shell/platform/windows/win32_window.h" #include -#include #include "win32_dpi_utils.h" diff --git a/shell/platform/windows/win32_window_unittests.cc b/shell/platform/windows/win32_window_unittests.cc index d1b61dc7b9dec..325d0ca81315e 100644 --- a/shell/platform/windows/win32_window_unittests.cc +++ b/shell/platform/windows/win32_window_unittests.cc @@ -9,6 +9,22 @@ using testing::_; namespace flutter { namespace testing { +namespace { + +// Creates a valid Windows LPARAM for WM_KEYDOWN and WM_KEYUP from parameters +// given. +static LPARAM CreateKeyEventLparam(USHORT ScanCode, + bool extended = false, + USHORT RepeatCount = 1, + bool ContextCode = 0, + bool PreviousKeyState = 1, + bool TransitionState = 1) { + return ((LPARAM(TransitionState) << 31) | (LPARAM(PreviousKeyState) << 30) | + (LPARAM(ContextCode) << 29) | (LPARAM(extended ? 0x1 : 0x0) << 24) | + (LPARAM(ScanCode) << 16) | LPARAM(RepeatCount)); +} + +} // namespace TEST(MockWin32Window, CreateDestroy) { MockWin32Window window; @@ -39,21 +55,10 @@ TEST(MockWin32Window, HorizontalScroll) { window.InjectWindowMessage(WM_MOUSEHWHEEL, MAKEWPARAM(0, scroll_amount), 0); } -static LPARAM CreateKeyEventLparam(USHORT RepeatCount, - USHORT ScanCode, - bool extended, - bool ContextCode, - bool PreviousKeyState, - bool TransitionState) { - return ((LPARAM(TransitionState) << 31) | (LPARAM(PreviousKeyState) << 30) | - (LPARAM(ContextCode) << 29) | (LPARAM(extended ? 0x1 : 0x0) << 24) | - (LPARAM(ScanCode) << 16) | LPARAM(RepeatCount)); -} - TEST(MockWin32Window, KeyDown) { MockWin32Window window; EXPECT_CALL(window, OnKey(_, _, _, _, _)).Times(1); - LPARAM lparam = CreateKeyEventLparam(1, 42, false, 0, 1, 1); + LPARAM lparam = CreateKeyEventLparam(42); // send a "Shift" key down event. window.InjectWindowMessage(WM_KEYDOWN, 16, lparam); } @@ -61,14 +66,14 @@ TEST(MockWin32Window, KeyDown) { TEST(MockWin32Window, KeyUp) { MockWin32Window window; EXPECT_CALL(window, OnKey(_, _, _, _, _)).Times(1); - LPARAM lparam = CreateKeyEventLparam(1, 42, false, 0, 1, 1); + LPARAM lparam = CreateKeyEventLparam(42); // send a "Shift" key up event. window.InjectWindowMessage(WM_KEYUP, 16, lparam); } TEST(MockWin32Window, KeyDownPrintable) { MockWin32Window window; - LPARAM lparam = CreateKeyEventLparam(1, 30, false, 0, 1, 1); + LPARAM lparam = CreateKeyEventLparam(30); // OnKey shouldn't be called until the WM_CHAR message. EXPECT_CALL(window, OnKey(65, 30, WM_KEYDOWN, 65, false)).Times(0); // send a "A" key down event. From ee4ab0422561252f5959c9fc52061cad72ec1085 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 21 Jan 2021 17:21:30 -0800 Subject: [PATCH 6/7] Review Changes --- .../windows/win32_flutter_window_unittests.cc | 453 +++++++++--------- 1 file changed, 218 insertions(+), 235 deletions(-) diff --git a/shell/platform/windows/win32_flutter_window_unittests.cc b/shell/platform/windows/win32_flutter_window_unittests.cc index 52ed904ae4d4c..a7ad7305367bc 100644 --- a/shell/platform/windows/win32_flutter_window_unittests.cc +++ b/shell/platform/windows/win32_flutter_window_unittests.cc @@ -22,6 +22,26 @@ namespace flutter { namespace testing { namespace { +// Creates a valid Windows LPARAM for WM_KEYDOWN and WM_CHAR from parameters +// given. +static LPARAM CreateKeyEventLparam(USHORT ScanCode, + bool extended = false, + USHORT RepeatCount = 1, + bool ContextCode = 0, + bool PreviousKeyState = 1, + bool TransitionState = 1) { + return ((LPARAM(TransitionState) << 31) | (LPARAM(PreviousKeyState) << 30) | + (LPARAM(ContextCode) << 29) | (LPARAM(extended ? 0x1 : 0x0) << 24) | + (LPARAM(ScanCode) << 16) | LPARAM(RepeatCount)); +} + +// A struc to hold simulated events that will be delivered after the framework +// response is handled. +struct SimulatedEvent { + UINT message; + WPARAM wparam; + LPARAM lparam; +}; // A key event handler that can be spied on while it forwards calls to the real // key event handler. @@ -81,23 +101,65 @@ class SpyTextInputPlugin : public KeyboardHookHandler { std::unique_ptr real_implementation_; }; +class MockWin32FlutterWindow : public Win32FlutterWindow { + public: + MockWin32FlutterWindow() : Win32FlutterWindow(800, 600) {} + virtual ~MockWin32FlutterWindow() {} + + // Prevent copying. + MockWin32FlutterWindow(MockWin32FlutterWindow const&) = delete; + MockWin32FlutterWindow& operator=(MockWin32FlutterWindow const&) = delete; + + // Wrapper for GetCurrentDPI() which is a protected method. + UINT GetDpi() { return GetCurrentDPI(); } + + // Simulates a WindowProc message from the OS. + LRESULT InjectWindowMessage(UINT const message, + WPARAM const wparam, + LPARAM const lparam) { + return HandleMessage(message, wparam, lparam); + } + + MOCK_METHOD1(OnDpiScale, void(unsigned int)); + MOCK_METHOD2(OnResize, void(unsigned int, unsigned int)); + MOCK_METHOD2(OnPointerMove, void(double, double)); + MOCK_METHOD3(OnPointerDown, void(double, double, UINT)); + MOCK_METHOD3(OnPointerUp, void(double, double, UINT)); + MOCK_METHOD0(OnPointerLeave, void()); + MOCK_METHOD0(OnSetCursor, void()); + MOCK_METHOD2(OnScroll, void(double, double)); +}; + // A FlutterWindowsView that overrides the RegisterKeyboardHookHandlers function // to register the keyboard hook handlers that can be spied upon. class TestFlutterWindowsView : public FlutterWindowsView { public: TestFlutterWindowsView(std::unique_ptr window_binding, - KeyEventHandler::SendInputDelegate send_input) + WPARAM virtual_key, + bool is_printable = true) : FlutterWindowsView(std::move(window_binding)), - send_input_(send_input) {} + virtual_key_(virtual_key), + is_printable_(is_printable) {} SpyKeyEventHandler* key_event_handler; SpyTextInputPlugin* text_input_plugin; + void InjectPendingEvents(MockWin32FlutterWindow* win32window) { + while (pending_events_.size() > 0) { + SimulatedEvent event = pending_events_.front(); + win32window->InjectWindowMessage(event.message, event.wparam, + event.lparam); + pending_events_.pop_front(); + } + } + protected: void RegisterKeyboardHookHandlers( flutter::BinaryMessenger* messenger) override { - auto spy_key_event_handler = - std::make_unique(messenger, send_input_); + 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_text_input_plugin = std::make_unique(messenger); key_event_handler = spy_key_event_handler.get(); @@ -107,7 +169,28 @@ class TestFlutterWindowsView : public FlutterWindowsView { } private: - KeyEventHandler::SendInputDelegate send_input_; + 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 UINT message = + (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; + const LPARAM lparam = CreateKeyEventLparam( + kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); + // 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). + pending_events_.push_back(SimulatedEvent{message, virtual_key_, lparam}); + if (is_printable_ && (kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { + pending_events_.push_back(SimulatedEvent{WM_CHAR, virtual_key_, lparam}); + } + return 1; + } + + std::deque pending_events_; + WPARAM virtual_key_; + bool is_printable_; }; // A struct to use as a FlutterPlatformMessageResponseHandle so it can keep the @@ -119,38 +202,9 @@ struct TestResponseHandle { void* user_data; }; -class MockWin32FlutterWindow : public Win32FlutterWindow { - public: - MockWin32FlutterWindow() : Win32FlutterWindow(800, 600) {} - virtual ~MockWin32FlutterWindow() {} - - // Prevent copying. - MockWin32FlutterWindow(MockWin32FlutterWindow const&) = delete; - MockWin32FlutterWindow& operator=(MockWin32FlutterWindow const&) = delete; - - // Wrapper for GetCurrentDPI() which is a protected method. - UINT GetDpi() { return GetCurrentDPI(); } - - // Simulates a WindowProc message from the OS. - LRESULT InjectWindowMessage(UINT const message, - WPARAM const wparam, - LPARAM const lparam) { - return HandleMessage(message, wparam, lparam); - } - - MOCK_METHOD1(OnDpiScale, void(unsigned int)); - MOCK_METHOD2(OnResize, void(unsigned int, unsigned int)); - MOCK_METHOD2(OnPointerMove, void(double, double)); - MOCK_METHOD3(OnPointerDown, void(double, double, UINT)); - MOCK_METHOD3(OnPointerUp, void(double, double, UINT)); - MOCK_METHOD0(OnPointerLeave, void()); - MOCK_METHOD0(OnSetCursor, void()); - MOCK_METHOD2(OnScroll, void(double, double)); -}; - // The static value to return as the "handled" value from the framework for key // events. Individual tests set this to change the framework response that the -// engine simulates. +// test engine simulates. static bool test_response = false; // Returns an engine instance configured with dummy project path values, and @@ -209,27 +263,6 @@ std::unique_ptr GetTestEngine() { return engine; } -// Creates a valid Windows LPARAM for WM_KEYDOWN and WM_CHAR from parameters -// given. -static LPARAM CreateKeyEventLparam(USHORT ScanCode, - bool extended = false, - USHORT RepeatCount = 1, - bool ContextCode = 0, - bool PreviousKeyState = 1, - bool TransitionState = 1) { - return ((LPARAM(TransitionState) << 31) | (LPARAM(PreviousKeyState) << 30) | - (LPARAM(ContextCode) << 29) | (LPARAM(extended ? 0x1 : 0x0) << 24) | - (LPARAM(ScanCode) << 16) | LPARAM(RepeatCount)); -} - -// A struc to hold simulated events that will be delivered after the framework -// response is handled. -struct SimulatedEvent { - UINT message; - WPARAM wparam; - LPARAM lparam; -}; - } // namespace TEST(Win32FlutterWindowTest, CreateDestroy) { @@ -237,224 +270,174 @@ TEST(Win32FlutterWindowTest, CreateDestroy) { ASSERT_TRUE(TRUE); } -// Tests key event propagation of printable character key down events. -TEST(Win32FlutterWindowTest, CharKeyDownPropagation) { - constexpr WPARAM virtual_key = 65; // The "A" key, which produces a character - constexpr WPARAM scan_code = 30; - constexpr char32_t character = 65; - +// Tests key event propagation of non-printable, non-modifier key down events. +TEST(Win32FlutterWindowTest, NonPrintableKeyDownPropagation) { + constexpr WPARAM virtual_key = VK_LEFT; + constexpr WPARAM scan_code = 10; + constexpr char32_t character = 0; MockWin32FlutterWindow win32window; std::deque pending_events; auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), - [&pending_events](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { - // Simulate the event loop by just sending the event sent to - // "SendInput" directly to the window. - const KEYBDINPUT kbdinput = pInputs->ki; - const UINT message = - (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; - const LPARAM lparam = CreateKeyEventLparam( - kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); - // 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). - pending_events.push_back(SimulatedEvent{message, virtual_key, lparam}); - if ((kbdinput.dwFlags & KEYEVENTF_KEYUP) == 0) { - pending_events.push_back( - SimulatedEvent{WM_CHAR, virtual_key, lparam}); - } - return 1; - }); + std::move(window_binding_handler), virtual_key, false /* is_printable */); win32window.SetView(&flutter_windows_view); + LPARAM lparam = CreateKeyEventLparam(scan_code); // Test an event not handled by the framework - test_response = false; - flutter_windows_view.SetEngine(std::move(GetTestEngine())); - EXPECT_CALL( - *flutter_windows_view.key_event_handler, - KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) - .Times(2); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, _, _, _, _, _)) - .Times(1); - EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(1); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(1); - LPARAM lparam = CreateKeyEventLparam(scan_code); - EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), - 0); - EXPECT_EQ(win32window.InjectWindowMessage(WM_CHAR, virtual_key, lparam), 0); - while (pending_events.size() > 0) { - SimulatedEvent event = pending_events.front(); - win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); - pending_events.pop_front(); + { + test_response = false; + 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 */)) + .Times(2) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)) + .Times(0); + 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); } - pending_events.clear(); // Test an event handled by the framework - test_response = true; - EXPECT_CALL( - *flutter_windows_view.key_event_handler, - KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) - .Times(1); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, _, _, _, _, _)) - .Times(0); - EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(0); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(0); - EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), - 0); - EXPECT_EQ(win32window.InjectWindowMessage(WM_CHAR, virtual_key, lparam), 0); - while (pending_events.size() > 0) { - SimulatedEvent event = pending_events.front(); - win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); - pending_events.pop_front(); + { + test_response = true; + EXPECT_CALL(*flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, + false /* extended */)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + flutter_windows_view.InjectPendingEvents(&win32window); } } -// Tests key event propagation of modifier key down events. -TEST(Win32FlutterWindowTest, ModifierKeyDownPropagation) { - constexpr WPARAM virtual_key = VK_LSHIFT; - constexpr WPARAM scan_code = 20; - constexpr char32_t character = 0; +// Tests key event propagation of printable character key down events. These +// differ from non-printable characters in that they follow a different code +// path in the WndProc (HandleMessage), producing a follow-on WM_CHAR event. +TEST(Win32FlutterWindowTest, CharKeyDownPropagation) { + constexpr WPARAM virtual_key = 65; // The "A" key, which produces a character + constexpr WPARAM scan_code = 30; + constexpr char32_t character = 65; + MockWin32FlutterWindow win32window; - std::deque pending_events; auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), - [&pending_events](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { - // Simulate the event loop by just sending the event sent to - // "SendInput" directly to the window. - const KEYBDINPUT kbdinput = pInputs->ki; - const UINT message = - (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; - const LPARAM lparam = CreateKeyEventLparam( - kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); - // SendEvent will typically add the events to a queue, so we do too. - // 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). - pending_events.push_back(SimulatedEvent{message, virtual_key, lparam}); - return 1; - }); + std::move(window_binding_handler), virtual_key, true /* is_printable */); win32window.SetView(&flutter_windows_view); + LPARAM lparam = CreateKeyEventLparam(scan_code); // Test an event not handled by the framework - test_response = false; - flutter_windows_view.SetEngine(std::move(GetTestEngine())); - EXPECT_CALL( - *flutter_windows_view.key_event_handler, - KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) - .Times(2) - .RetiresOnSaturation(); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, _, _, _, _, _)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(0); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(0); - LPARAM lparam = CreateKeyEventLparam(scan_code); - EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), - 0); - while (pending_events.size() > 0) { - SimulatedEvent event = pending_events.front(); - win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); - pending_events.pop_front(); + { + test_response = false; + flutter_windows_view.SetEngine(std::move(GetTestEngine())); + EXPECT_CALL( + *flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) + .Times(2) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_CHAR, virtual_key, lparam), 0); + flutter_windows_view.InjectPendingEvents(&win32window); } - pending_events.clear(); // Test an event handled by the framework - test_response = true; - EXPECT_CALL( - *flutter_windows_view.key_event_handler, - KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, _, _, _, _, _)) - .Times(0); - EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), - 0); - while (pending_events.size() > 0) { - SimulatedEvent event = pending_events.front(); - win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); - pending_events.pop_front(); + { + test_response = true; + EXPECT_CALL(*flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, + false /* is_printable */)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(0); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)) + .Times(0); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)) + .Times(0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_CHAR, virtual_key, lparam), 0); + flutter_windows_view.InjectPendingEvents(&win32window); } } -// Tests key event propagation of non-printable, non-modifier key down events. -TEST(Win32FlutterWindowTest, NonPrintableKeyDownPropagation) { - constexpr WPARAM virtual_key = VK_LEFT; - constexpr WPARAM scan_code = 10; +// Tests key event propagation of modifier key down events. This are different +// from non-printable events in that they call MapVirtualKey, resulting in a +// slightly different code path. +TEST(Win32FlutterWindowTest, ModifierKeyDownPropagation) { + constexpr WPARAM virtual_key = VK_LSHIFT; + constexpr WPARAM scan_code = 20; constexpr char32_t character = 0; MockWin32FlutterWindow win32window; std::deque pending_events; auto window_binding_handler = std::make_unique<::testing::NiceMock>(); TestFlutterWindowsView flutter_windows_view( - std::move(window_binding_handler), - [&pending_events](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT { - // Simulate the event loop by just sending the event sent to - // "SendInput" directly to the window. - const KEYBDINPUT kbdinput = pInputs->ki; - const UINT message = - (kbdinput.dwFlags & KEYEVENTF_KEYUP) ? WM_KEYUP : WM_KEYDOWN; - const LPARAM lparam = CreateKeyEventLparam( - kbdinput.wScan, kbdinput.dwFlags & KEYEVENTF_EXTENDEDKEY); - // SendEvent will typically add the events to a queue, so we do too. - // 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). - pending_events.push_back(SimulatedEvent{message, virtual_key, lparam}); - return 1; - }); + std::move(window_binding_handler), virtual_key, false /* is_printable */); win32window.SetView(&flutter_windows_view); + LPARAM lparam = CreateKeyEventLparam(scan_code); // Test an event not handled by the framework - test_response = false; - flutter_windows_view.SetEngine(std::move(GetTestEngine())); - EXPECT_CALL( - *flutter_windows_view.key_event_handler, - KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) - .Times(2) - .RetiresOnSaturation(); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, _, _, _, _, _)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)).Times(0); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, TextHook(_, _)).Times(0); - LPARAM lparam = CreateKeyEventLparam(scan_code); - EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), - 0); - while (pending_events.size() > 0) { - SimulatedEvent event = pending_events.front(); - win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); - pending_events.pop_front(); + { + test_response = false; + 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 */)) + .Times(2) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.key_event_handler, TextHook(_, _)) + .Times(0); + 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); } - pending_events.clear(); // Test an event handled by the framework - test_response = true; - EXPECT_CALL( - *flutter_windows_view.key_event_handler, - KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, false)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*flutter_windows_view.text_input_plugin, - KeyboardHook(_, _, _, _, _, _)) - .Times(0); - EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), - 0); - while (pending_events.size() > 0) { - SimulatedEvent event = pending_events.front(); - win32window.InjectWindowMessage(event.message, event.wparam, event.lparam); - pending_events.pop_front(); + { + test_response = true; + EXPECT_CALL(*flutter_windows_view.key_event_handler, + KeyboardHook(_, virtual_key, scan_code, WM_KEYDOWN, character, + false /* extended */)) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(*flutter_windows_view.text_input_plugin, + KeyboardHook(_, _, _, _, _, _)) + .Times(0); + EXPECT_EQ(win32window.InjectWindowMessage(WM_KEYDOWN, virtual_key, lparam), + 0); + flutter_windows_view.InjectPendingEvents(&win32window); } } From 76ce7c4feb8fcd997885ceab48a82c8039599098 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 22 Jan 2021 14:12:50 -0800 Subject: [PATCH 7/7] Review Changes 2 --- shell/platform/windows/flutter_windows_view.cc | 6 ++++-- shell/platform/windows/key_event_handler.cc | 13 +++++++++++++ .../windows/testing/mock_window_binding_handler.h | 1 + .../windows/win32_flutter_window_unittests.cc | 8 ++++++-- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 88e1417da731a..a5c709357305b 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -46,9 +46,11 @@ void FlutterWindowsView::SetEngine( binding_handler_->GetDpiScale()); } -void FlutterWindowsView::RegisterKeyboardHookHandlers(flutter::BinaryMessenger* messenger) { +void FlutterWindowsView::RegisterKeyboardHookHandlers( + flutter::BinaryMessenger* messenger) { AddKeyboardHookHandler(std::make_unique(messenger)); - AddKeyboardHookHandler(std::make_unique(messenger, this)); + AddKeyboardHookHandler( + std::make_unique(messenger, this)); } void FlutterWindowsView::AddKeyboardHookHandler( diff --git a/shell/platform/windows/key_event_handler.cc b/shell/platform/windows/key_event_handler.cc index e57f4e6bc26e2..b4b1b8149c3fd 100644 --- a/shell/platform/windows/key_event_handler.cc +++ b/shell/platform/windows/key_event_handler.cc @@ -88,6 +88,18 @@ int GetModsForKeyState() { return mods; } +// 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 below 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. uint64_t CalculateEventId(int scancode, int action, bool extended) { // Calculate a key event ID based on the scan code of the key pressed, // and the flags we care about. @@ -95,6 +107,7 @@ uint64_t CalculateEventId(int scancode, int action, bool extended) { (extended ? KEYEVENTF_EXTENDEDKEY : 0x0)) << 16); } + } // namespace KeyEventHandler::KeyEventHandler(flutter::BinaryMessenger* messenger, diff --git a/shell/platform/windows/testing/mock_window_binding_handler.h b/shell/platform/windows/testing/mock_window_binding_handler.h index e8b18229f2123..50786a4b5da51 100644 --- a/shell/platform/windows/testing/mock_window_binding_handler.h +++ b/shell/platform/windows/testing/mock_window_binding_handler.h @@ -29,6 +29,7 @@ class MockWindowBindingHandler : public WindowBindingHandler { MOCK_METHOD0(OnWindowResized, void()); MOCK_METHOD0(GetPhysicalWindowBounds, PhysicalWindowBounds()); MOCK_METHOD1(UpdateFlutterCursor, void(const std::string& cursor_name)); + MOCK_METHOD1(UpdateCursorRect, void(const Rect& rect)); }; } // namespace testing diff --git a/shell/platform/windows/win32_flutter_window_unittests.cc b/shell/platform/windows/win32_flutter_window_unittests.cc index a7ad7305367bc..8ba4b15f8d425 100644 --- a/shell/platform/windows/win32_flutter_window_unittests.cc +++ b/shell/platform/windows/win32_flutter_window_unittests.cc @@ -9,6 +9,7 @@ #include "flutter/shell/platform/windows/testing/engine_embedder_api_modifier.h" #include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h" #include "flutter/shell/platform/windows/testing/win32_flutter_window_test.h" +#include "flutter/shell/platform/windows/text_input_plugin_delegate.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -75,10 +76,11 @@ class SpyKeyEventHandler : public KeyboardHookHandler { // A text input plugin that can be spied on while it forwards calls to the real // text input plugin. -class SpyTextInputPlugin : public KeyboardHookHandler { +class SpyTextInputPlugin : public KeyboardHookHandler, + public TextInputPluginDelegate { public: SpyTextInputPlugin(flutter::BinaryMessenger* messenger) { - real_implementation_ = std::make_unique(messenger); + real_implementation_ = std::make_unique(messenger, this); ON_CALL(*this, KeyboardHook(_, _, _, _, _, _)) .WillByDefault( Invoke(real_implementation_.get(), &TextInputPlugin::KeyboardHook)); @@ -97,6 +99,8 @@ class SpyTextInputPlugin : public KeyboardHookHandler { MOCK_METHOD2(TextHook, void(FlutterWindowsView* window, const std::u16string& text)); + virtual void OnCursorRectUpdated(const Rect& rect) {} + private: std::unique_ptr real_implementation_; };