[Win32, Keyboard] TextInputPlugin is no longer a KeyboardHandlerBase (flutter#30456)

* Impl

* format

* Comment

Author: dkwingsmt

shell/platform/windows/flutter_window_win32_unittests.cc

@@ -35,17 +35,13 @@ static constexpr int32_t kDefaultPointerDeviceId = 0;
 class SpyKeyboardKeyHandler : public KeyboardHandlerBase {
  public:
   SpyKeyboardKeyHandler(flutter::BinaryMessenger* messenger,
-                        KeyboardKeyHandler::EventDispatcher redispatch_event) {
-    real_implementation_ =
-        std::make_unique<KeyboardKeyHandler>(redispatch_event);
+                        KeyboardKeyHandler::EventDispatcher dispatch_event) {
+    real_implementation_ = std::make_unique<KeyboardKeyHandler>(dispatch_event);
     real_implementation_->AddDelegate(
         std::make_unique<KeyboardKeyChannelHandler>(messenger));
     ON_CALL(*this, KeyboardHook(_, _, _, _, _, _))
         .WillByDefault(Invoke(real_implementation_.get(),
                               &KeyboardKeyHandler::KeyboardHook));
-    ON_CALL(*this, TextHook(_))
-        .WillByDefault(
-            Invoke(real_implementation_.get(), &KeyboardKeyHandler::TextHook));
   }
 
   MOCK_METHOD6(KeyboardHook,
@@ -55,7 +51,6 @@ class SpyKeyboardKeyHandler : public KeyboardHandlerBase {
                     char32_t character,
                     bool extended,
                     bool was_down));
-  MOCK_METHOD1(TextHook, void(const std::u16string& text));
   MOCK_METHOD0(ComposeBeginHook, void());
   MOCK_METHOD0(ComposeCommitHook, void());
   MOCK_METHOD0(ComposeEndHook, void());
@@ -68,10 +63,11 @@ class SpyKeyboardKeyHandler : public KeyboardHandlerBase {
 
 // A text input plugin that can be spied on while it forwards calls to the real
 // text input plugin.
-class SpyTextInputPlugin : public KeyboardHandlerBase,
+class SpyTextInputPlugin : public TextInputPlugin,
                            public TextInputPluginDelegate {
  public:
-  SpyTextInputPlugin(flutter::BinaryMessenger* messenger) {
+  SpyTextInputPlugin(flutter::BinaryMessenger* messenger)
+      : TextInputPlugin(messenger, this) {
     real_implementation_ = std::make_unique<TextInputPlugin>(messenger, this);
     ON_CALL(*this, KeyboardHook(_, _, _, _, _, _))
         .WillByDefault(
@@ -82,7 +78,7 @@ class SpyTextInputPlugin : public KeyboardHandlerBase,
   }
 
   MOCK_METHOD6(KeyboardHook,
-               bool(int key,
+               void(int key,
                     int scancode,
                     int action,
                     char32_t character,
@@ -246,7 +242,7 @@ class TestFlutterWindowsView : public FlutterWindowsView {
   }
 
  protected:
-  void RegisterKeyboardHandlers(
+  std::unique_ptr<KeyboardHandlerBase> CreateKeyboardKeyHandler(
       flutter::BinaryMessenger* messenger,
       flutter::KeyboardKeyHandler::EventDispatcher dispatch_event,
       flutter::KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state)
@@ -255,12 +251,16 @@ class TestFlutterWindowsView : public FlutterWindowsView {
         messenger, [this](UINT cInputs, LPINPUT pInputs, int cbSize) -> UINT {
           return this->SendInput(cInputs, pInputs, cbSize);
         });
-    auto spy_text_input_plugin =
-        std::make_unique<SpyTextInputPlugin>(messenger);
     key_event_handler = spy_key_event_handler.get();
-    text_input_plugin = spy_text_input_plugin.get();
-    AddKeyboardHandler(std::move(spy_key_event_handler));
-    AddKeyboardHandler(std::move(spy_text_input_plugin));
+    return spy_key_event_handler;
+  }
+
+  std::unique_ptr<TextInputPlugin> CreateTextInputPlugin(
+      flutter::BinaryMessenger* messenger) override {
+    auto spy_key_event_handler =
+        std::make_unique<SpyTextInputPlugin>(messenger);
+    text_input_plugin = spy_key_event_handler.get();
+    return spy_key_event_handler;
   }
 
  private:
@@ -357,7 +357,6 @@ TEST(FlutterWindowWin32Test, NonPrintableKeyDownPropagation) {
                 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);
     win32window.InjectMessages(1,
                                Win32Message{WM_KEYDOWN, virtual_key, lparam});
@@ -410,7 +409,6 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) {
                 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);
     win32window.InjectMessages(
         1, Win32Message{WM_SYSKEYDOWN, virtual_key, lparam, kWmResultDefault});
@@ -436,7 +434,7 @@ TEST(FlutterWindowWin32Test, SystemKeyDownPropagation) {
 // 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(FlutterWindowWin32Test, CharKeyDownPropagation) {
-  // ::testing::InSequence in_sequence;
+  ::testing::InSequence in_sequence;
 
   constexpr WPARAM virtual_key = 65;  // The "A" key, which produces a character
   constexpr WPARAM scan_code = 30;
@@ -463,9 +461,6 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) {
                 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();
@@ -485,7 +480,6 @@ TEST(FlutterWindowWin32Test, CharKeyDownPropagation) {
     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);
     win32window.InjectMessages(2, Win32Message{WM_KEYDOWN, virtual_key, lparam},
                                Win32Message{WM_CHAR, virtual_key, lparam});
@@ -521,7 +515,6 @@ TEST(FlutterWindowWin32Test, ModifierKeyDownPropagation) {
                 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);

shell/platform/windows/flutter_windows_view.cc

@@ -52,25 +52,26 @@ void FlutterWindowsView::SetEngine(
   engine_->SetView(this);
 
   internal_plugin_registrar_ =
-      std::make_unique<flutter::PluginRegistrar>(engine_->GetRegistrar());
+      std::make_unique<PluginRegistrar>(engine_->GetRegistrar());
 
   // Set up the system channel handlers.
   auto internal_plugin_messenger = internal_plugin_registrar_->messenger();
   InitializeKeyboard();
   platform_handler_ = PlatformHandler::Create(internal_plugin_messenger, this);
-  cursor_handler_ = std::make_unique<flutter::CursorHandler>(
-      internal_plugin_messenger, binding_handler_.get());
+  cursor_handler_ = std::make_unique<CursorHandler>(internal_plugin_messenger,
+                                                    binding_handler_.get());
 
   PhysicalWindowBounds bounds = binding_handler_->GetPhysicalWindowBounds();
 
   SendWindowMetrics(bounds.width, bounds.height,
                     binding_handler_->GetDpiScale());
 }
 
-void FlutterWindowsView::RegisterKeyboardHandlers(
-    flutter::BinaryMessenger* messenger,
-    flutter::KeyboardKeyHandler::EventDispatcher dispatch_event,
-    flutter::KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state) {
+std::unique_ptr<KeyboardHandlerBase>
+FlutterWindowsView::CreateKeyboardKeyHandler(
+    BinaryMessenger* messenger,
+    KeyboardKeyHandler::EventDispatcher dispatch_event,
+    KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state) {
   // There must be only one handler that receives |SendInput|, i.e. only one
   // handler that might redispatch events. (See the documentation of
   // |KeyboardKeyHandler| to learn about redispatching.)
@@ -79,24 +80,23 @@ void FlutterWindowsView::RegisterKeyboardHandlers(
   // of the event. In order to allow the same real event in the future, the
   // handler is "toggled" when events pass through, therefore the redispatching
   // algorithm does not allow more than 1 handler that takes |SendInput|.
-  auto key_handler =
-      std::make_unique<flutter::KeyboardKeyHandler>(dispatch_event);
-  key_handler->AddDelegate(std::make_unique<KeyboardKeyEmbedderHandler>(
-      [this](const FlutterKeyEvent& event, FlutterKeyEventCallback callback,
-             void* user_data) {
-        return engine_->SendKeyEvent(event, callback, user_data);
-      },
-      get_key_state));
-  key_handler->AddDelegate(
+  auto keyboard_key_handler =
+      std::make_unique<KeyboardKeyHandler>(dispatch_event);
+  keyboard_key_handler->AddDelegate(
+      std::make_unique<KeyboardKeyEmbedderHandler>(
+          [this](const FlutterKeyEvent& event, FlutterKeyEventCallback callback,
+                 void* user_data) {
+            return engine_->SendKeyEvent(event, callback, user_data);
+          },
+          get_key_state));
+  keyboard_key_handler->AddDelegate(
       std::make_unique<KeyboardKeyChannelHandler>(messenger));
-  AddKeyboardHandler(std::move(key_handler));
-  AddKeyboardHandler(
-      std::make_unique<flutter::TextInputPlugin>(messenger, this));
+  return keyboard_key_handler;
 }
 
-void FlutterWindowsView::AddKeyboardHandler(
-    std::unique_ptr<flutter::KeyboardHandlerBase> handler) {
-  keyboard_handlers_.push_back(std::move(handler));
+std::unique_ptr<TextInputPlugin> FlutterWindowsView::CreateTextInputPlugin(
+    BinaryMessenger* messenger) {
+  return std::make_unique<TextInputPlugin>(messenger, this);
 }
 
 uint32_t FlutterWindowsView::GetFrameBufferId(size_t width, size_t height) {
@@ -129,7 +129,6 @@ void FlutterWindowsView::ForceRedraw() {
 }
 
 void FlutterWindowsView::OnPreEngineRestart() {
-  keyboard_handlers_.clear();
   InitializeKeyboard();
 }
 
@@ -268,16 +267,16 @@ void FlutterWindowsView::OnResetImeComposing() {
 void FlutterWindowsView::InitializeKeyboard() {
   auto internal_plugin_messenger = internal_plugin_registrar_->messenger();
 #ifdef WINUWP
-  flutter::KeyboardKeyHandler::EventDispatcher dispatch_event = nullptr;
-  flutter::KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state =
-      nullptr;
+  KeyboardKeyHandler::EventDispatcher dispatch_event = nullptr;
+  KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state = nullptr;
 #else
-  flutter::KeyboardKeyHandler::EventDispatcher dispatch_event = SendInput;
-  flutter::KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state =
-      GetKeyState;
+  KeyboardKeyHandler::EventDispatcher dispatch_event = SendInput;
+  KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state = GetKeyState;
 #endif
-  RegisterKeyboardHandlers(internal_plugin_messenger, dispatch_event,
-                           get_key_state);
+  keyboard_key_handler_ = std::move(CreateKeyboardKeyHandler(
+      internal_plugin_messenger, dispatch_event, get_key_state));
+  text_input_plugin_ =
+      std::move(CreateTextInputPlugin(internal_plugin_messenger));
 }
 
 // Sends new size  information to FlutterEngine.
@@ -381,9 +380,7 @@ void FlutterWindowsView::SendPointerLeave(PointerState* state) {
 }
 
 void FlutterWindowsView::SendText(const std::u16string& text) {
-  for (const auto& handler : keyboard_handlers_) {
-    handler->TextHook(text);
-  }
+  text_input_plugin_->TextHook(text);
 }
 
 bool FlutterWindowsView::SendKey(int key,
@@ -392,39 +389,32 @@ bool FlutterWindowsView::SendKey(int key,
                                  char32_t character,
                                  bool extended,
                                  bool was_down) {
-  for (const auto& handler : keyboard_handlers_) {
-    if (handler->KeyboardHook(key, scancode, action, character, extended,
-                              was_down)) {
-      // key event was handled, so don't send to other handlers.
-      return true;
-    }
+  if (keyboard_key_handler_->KeyboardHook(key, scancode, action, character,
+                                          extended, was_down)) {
+    return true;
   }
+
+  text_input_plugin_->KeyboardHook(key, scancode, action, character, extended,
+                                   was_down);
+
   return false;
 }
 
 void FlutterWindowsView::SendComposeBegin() {
-  for (const auto& handler : keyboard_handlers_) {
-    handler->ComposeBeginHook();
-  }
+  text_input_plugin_->ComposeBeginHook();
 }
 
 void FlutterWindowsView::SendComposeCommit() {
-  for (const auto& handler : keyboard_handlers_) {
-    handler->ComposeCommitHook();
-  }
+  text_input_plugin_->ComposeCommitHook();
 }
 
 void FlutterWindowsView::SendComposeEnd() {
-  for (const auto& handler : keyboard_handlers_) {
-    handler->ComposeEndHook();
-  }
+  text_input_plugin_->ComposeEndHook();
 }
 
 void FlutterWindowsView::SendComposeChange(const std::u16string& text,
                                            int cursor_pos) {
-  for (const auto& handler : keyboard_handlers_) {
-    handler->ComposeChangeHook(text, cursor_pos);
-  }
+  text_input_plugin_->ComposeChangeHook(text, cursor_pos);
 }
 
 void FlutterWindowsView::SendScroll(double x,

shell/platform/windows/flutter_windows_view.h

@@ -22,9 +22,9 @@
 #include "flutter/shell/platform/windows/flutter_windows_engine.h"
 #include "flutter/shell/platform/windows/keyboard_handler_base.h"
 #include "flutter/shell/platform/windows/keyboard_key_embedder_handler.h"
-#include "flutter/shell/platform/windows/keyboard_key_handler.h"
 #include "flutter/shell/platform/windows/platform_handler.h"
 #include "flutter/shell/platform/windows/public/flutter_windows.h"
+#include "flutter/shell/platform/windows/text_input_plugin.h"
 #include "flutter/shell/platform/windows/text_input_plugin_delegate.h"
 #include "flutter/shell/platform/windows/window_binding_handler.h"
 #include "flutter/shell/platform/windows/window_binding_handler_delegate.h"
@@ -171,20 +171,20 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate,
   void OnResetImeComposing() override;
 
  protected:
-  // Called to create the keyboard hook handlers.
+  // Called to create keyboard key handler.
   //
   // The provided |dispatch_event| is where to inject events into the system,
   // while |get_key_state| is where to acquire keyboard states. They will be
   // the system APIs in production classes, but might be replaced with mock
   // functions in unit tests.
-  virtual void RegisterKeyboardHandlers(
-      flutter::BinaryMessenger* messenger,
-      flutter::KeyboardKeyHandler::EventDispatcher dispatch_event,
-      flutter::KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state);
+  virtual std::unique_ptr<KeyboardHandlerBase> CreateKeyboardKeyHandler(
+      BinaryMessenger* messenger,
+      KeyboardKeyHandler::EventDispatcher dispatch_event,
+      KeyboardKeyEmbedderHandler::GetKeyStateHandler get_key_state);
 
-  // Used by RegisterKeyboardHandlers to add a new keyboard hook handler.
-  void AddKeyboardHandler(
-      std::unique_ptr<flutter::KeyboardHandlerBase> handler);
+  // Called to create text input plugin.
+  virtual std::unique_ptr<TextInputPlugin> CreateTextInputPlugin(
+      BinaryMessenger* messenger);
 
  private:
   // Struct holding the state of an individual pointer. The engine doesn't keep
@@ -322,19 +322,22 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate,
   std::unordered_map<int32_t, std::unique_ptr<PointerState>> pointer_states_;
 
   // The plugin registrar managing internal plugins.
-  std::unique_ptr<flutter::PluginRegistrar> internal_plugin_registrar_;
+  std::unique_ptr<PluginRegistrar> internal_plugin_registrar_;
 
   // Handlers for keyboard events from Windows.
-  std::vector<std::unique_ptr<flutter::KeyboardHandlerBase>> keyboard_handlers_;
+  std::unique_ptr<KeyboardHandlerBase> keyboard_key_handler_;
+
+  // Handlers for text events from Windows.
+  std::unique_ptr<TextInputPlugin> text_input_plugin_;
 
   // Handler for the flutter/platform channel.
-  std::unique_ptr<flutter::PlatformHandler> platform_handler_;
+  std::unique_ptr<PlatformHandler> platform_handler_;
 
   // Handler for cursor events.
-  std::unique_ptr<flutter::CursorHandler> cursor_handler_;
+  std::unique_ptr<CursorHandler> cursor_handler_;
 
   // Currently configured WindowBindingHandler for view.
-  std::unique_ptr<flutter::WindowBindingHandler> binding_handler_;
+  std::unique_ptr<WindowBindingHandler> binding_handler_;
 
   // Resize events are synchronized using this mutex and the corresponding
   // condition variable.