Move keymap from FlKeyboardViewDelegate to FlKeyboardManager (#55942)

Ideally the tests would mock gdk_keymap_lookup_key, but I wasn't able to get it working so I've added fl_keyboard_manager_set_lookup_key_handler for now.

Author: robert-ancell

shell/platform/linux/BUILD.gn

@@ -248,6 +248,7 @@ executable("flutter_linux_unittests") {
     "testing/mock_engine.cc",
     "testing/mock_epoxy.cc",
     "testing/mock_im_context.cc",
+    "testing/mock_keymap.cc",
     "testing/mock_plugin_registrar.cc",
     "testing/mock_renderer.cc",
     "testing/mock_settings.cc",

shell/platform/linux/fl_keyboard_manager.cc

@@ -118,6 +118,9 @@ struct _FlKeyboardManager {
 
   GWeakRef view_delegate;
 
+  FlKeyboardManagerLookupKeyHandler lookup_key_handler;
+  gpointer lookup_key_handler_user_data;
+
   FlKeyboardManagerRedispatchEventHandler redispatch_handler;
   gpointer redispatch_handler_user_data;
 
@@ -157,10 +160,19 @@ struct _FlKeyboardManager {
   // It is set up when the manager is initialized and is not changed ever after.
   std::unique_ptr<std::map<uint64_t, const LayoutGoal*>>
       logical_to_mandatory_goals;
+
+  GdkKeymap* keymap;
+  gulong keymap_keys_changed_cb_id;  // Signal connection ID for
+                                     // keymap-keys-changed
 };
 
 G_DEFINE_TYPE(FlKeyboardManager, fl_keyboard_manager, G_TYPE_OBJECT);
 
+static void keymap_keys_changed_cb(FlKeyboardManager* self) {
+  g_clear_object(&self->derived_layout);
+  self->derived_layout = fl_keyboard_layout_new();
+}
+
 // This is an exact copy of g_ptr_array_find_with_equal_func.  Somehow CI
 // reports that can not find symbol g_ptr_array_find_with_equal_func, despite
 // the fact that it runs well locally.
@@ -292,13 +304,18 @@ static void responder_handle_channel_event_callback(bool handled,
   responder_handle_event_callback(handled, user_data_ptr, FALSE);
 }
 
-static uint16_t convert_key_to_char(FlKeyboardViewDelegate* view_delegate,
+static uint16_t convert_key_to_char(FlKeyboardManager* self,
                                     guint keycode,
                                     gint group,
                                     gint level) {
   GdkKeymapKey key = {keycode, group, level};
   constexpr int kBmpMax = 0xD7FF;
-  guint origin = fl_keyboard_view_delegate_lookup_key(view_delegate, &key);
+  guint origin;
+  if (self->lookup_key_handler != nullptr) {
+    origin = self->lookup_key_handler(&key, self->lookup_key_handler_user_data);
+  } else {
+    origin = gdk_keymap_lookup_key(self->keymap, &key);
+  }
   return origin < kBmpMax ? origin : 0xFFFF;
 }
 
@@ -329,8 +346,8 @@ static void guarantee_layout(FlKeyboardManager* self, FlKeyEvent* event) {
   std::string debug_layout_data;
   for (uint16_t keycode = 0; keycode < 128; keycode += 1) {
     std::vector<uint16_t> this_key_clues = {
-        convert_key_to_char(view_delegate, keycode, group, 0),
-        convert_key_to_char(view_delegate, keycode, group, 1),  // Shift
+        convert_key_to_char(self, keycode, group, 0),
+        convert_key_to_char(self, keycode, group, 1),  // Shift
     };
     debug_format_layout_data(debug_layout_data, keycode, this_key_clues[0],
                              this_key_clues[1]);
@@ -344,8 +361,8 @@ static void guarantee_layout(FlKeyboardManager* self, FlKeyEvent* event) {
   for (const LayoutGoal& keycode_goal : layout_goals) {
     uint16_t keycode = keycode_goal.keycode;
     std::vector<uint16_t> this_key_clues = {
-        convert_key_to_char(view_delegate, keycode, group, 0),
-        convert_key_to_char(view_delegate, keycode, group, 1),  // Shift
+        convert_key_to_char(self, keycode, group, 0),
+        convert_key_to_char(self, keycode, group, 1),  // Shift
     };
 
     // The logical key should be the first available clue from below:
@@ -404,6 +421,10 @@ static void fl_keyboard_manager_dispose(GObject* object) {
   g_ptr_array_free(self->pending_responds, TRUE);
   g_ptr_array_free(self->pending_redispatches, TRUE);
   g_clear_object(&self->derived_layout);
+  if (self->keymap_keys_changed_cb_id != 0) {
+    g_signal_handler_disconnect(self->keymap, self->keymap_keys_changed_cb_id);
+    self->keymap_keys_changed_cb_id = 0;
+  }
 
   G_OBJECT_CLASS(fl_keyboard_manager_parent_class)->dispose(object);
 }
@@ -430,6 +451,10 @@ static void fl_keyboard_manager_init(FlKeyboardManager* self) {
   self->pending_redispatches = g_ptr_array_new_with_free_func(g_object_unref);
 
   self->last_sequence_id = 1;
+
+  self->keymap = gdk_keymap_get_for_display(gdk_display_get_default());
+  self->keymap_keys_changed_cb_id = g_signal_connect_swapped(
+      self->keymap, "keys-changed", G_CALLBACK(keymap_keys_changed_cb), self);
 }
 
 FlKeyboardManager* fl_keyboard_manager_new(
@@ -511,16 +536,25 @@ GHashTable* fl_keyboard_manager_get_pressed_state(FlKeyboardManager* self) {
       self->key_embedder_responder);
 }
 
+void fl_keyboard_manager_set_lookup_key_handler(
+    FlKeyboardManager* self,
+    FlKeyboardManagerLookupKeyHandler lookup_key_handler,
+    gpointer user_data) {
+  g_return_if_fail(FL_IS_KEYBOARD_MANAGER(self));
+  self->lookup_key_handler = lookup_key_handler;
+  self->lookup_key_handler_user_data = user_data;
+}
+
 void fl_keyboard_manager_notify_layout_changed(FlKeyboardManager* self) {
   g_return_if_fail(FL_IS_KEYBOARD_MANAGER(self));
-  g_clear_object(&self->derived_layout);
-  self->derived_layout = fl_keyboard_layout_new();
+  keymap_keys_changed_cb(self);
 }
 
 void fl_keyboard_manager_set_redispatch_handler(
     FlKeyboardManager* self,
     FlKeyboardManagerRedispatchEventHandler redispatch_handler,
     gpointer user_data) {
+  g_return_if_fail(FL_IS_KEYBOARD_MANAGER(self));
   self->redispatch_handler = redispatch_handler;
   self->redispatch_handler_user_data = user_data;
 }

shell/platform/linux/fl_keyboard_manager.h

@@ -94,11 +94,26 @@ void fl_keyboard_manager_sync_modifier_if_needed(FlKeyboardManager* manager,
  */
 GHashTable* fl_keyboard_manager_get_pressed_state(FlKeyboardManager* manager);
 
+typedef guint (*FlKeyboardManagerLookupKeyHandler)(const GdkKeymapKey* key,
+                                                   gpointer user_data);
+
+/**
+ * fl_keyboard_manager_set_lookup_key_handler:
+ * @manager: the #FlKeyboardManager self.
+ *
+ * Set the handler for key lookup, for testing purposes only.
+ */
+void fl_keyboard_manager_set_lookup_key_handler(
+    FlKeyboardManager* manager,
+    FlKeyboardManagerLookupKeyHandler lookup_key_handler,
+    gpointer user_data);
+
 /**
  * fl_keyboard_manager_notify_layout_changed:
  * @manager: the #FlKeyboardManager self.
  *
- * Notify the manager the keyboard layout has changed.
+ * Notify the manager the keyboard layout has changed, for testing purposes
+ * only.
  */
 void fl_keyboard_manager_notify_layout_changed(FlKeyboardManager* manager);
 

shell/platform/linux/fl_keyboard_manager_test.cc

@@ -15,6 +15,7 @@
 #include "flutter/shell/platform/linux/public/flutter_linux/fl_method_codec.h"
 #include "flutter/shell/platform/linux/public/flutter_linux/fl_standard_method_codec.h"
 #include "flutter/shell/platform/linux/testing/fl_test.h"
+#include "flutter/shell/platform/linux/testing/mock_keymap.h"
 #include "flutter/shell/platform/linux/testing/mock_text_input_handler.h"
 #include "flutter/testing/testing.h"
 
@@ -282,7 +283,6 @@ struct _FlMockViewDelegate {
   FlMockKeyBinaryMessenger* messenger;
   EmbedderCallHandler embedder_handler;
   bool text_filter_result;
-  const MockLayoutData* layout_data;
 };
 
 static void fl_mock_view_keyboard_delegate_iface_init(
@@ -329,24 +329,10 @@ static gboolean fl_mock_view_keyboard_text_filter_key_press(
   return self->text_filter_result;
 }
 
-static guint fl_mock_view_keyboard_lookup_key(
-    FlKeyboardViewDelegate* view_delegate,
-    const GdkKeymapKey* key) {
-  FlMockViewDelegate* self = FL_MOCK_VIEW_DELEGATE(view_delegate);
-  guint8 group = static_cast<guint8>(key->group);
-  EXPECT_LT(group, self->layout_data->size());
-  const MockGroupLayoutData* group_layout = (*self->layout_data)[group];
-  EXPECT_TRUE(group_layout != nullptr);
-  EXPECT_TRUE(key->level == 0 || key->level == 1);
-  bool shift = key->level == 1;
-  return (*group_layout)[key->keycode * 2 + shift];
-}
-
 static void fl_mock_view_keyboard_delegate_iface_init(
     FlKeyboardViewDelegateInterface* iface) {
   iface->send_key_event = fl_mock_view_keyboard_send_key_event;
   iface->text_filter_key_press = fl_mock_view_keyboard_text_filter_key_press;
-  iface->lookup_key = fl_mock_view_keyboard_lookup_key;
 }
 
 static FlMockViewDelegate* fl_mock_view_delegate_new() {
@@ -371,11 +357,6 @@ static void fl_mock_view_set_text_filter_result(FlMockViewDelegate* self,
   self->text_filter_result = result;
 }
 
-static void fl_mock_view_set_layout(FlMockViewDelegate* self,
-                                    const MockLayoutData* layout) {
-  self->layout_data = layout;
-}
-
 /***** End FlMockViewDelegate *****/
 
 class KeyboardTester {
@@ -389,6 +370,20 @@ class KeyboardTester {
 
     manager_ = fl_keyboard_manager_new(FL_BINARY_MESSENGER(view_->messenger),
                                        FL_KEYBOARD_VIEW_DELEGATE(view_));
+    fl_keyboard_manager_set_lookup_key_handler(
+        manager_,
+        [](const GdkKeymapKey* key, gpointer user_data) {
+          KeyboardTester* self = reinterpret_cast<KeyboardTester*>(user_data);
+          guint8 group = static_cast<guint8>(key->group);
+          EXPECT_LT(group, self->layout_data_->size());
+          const MockGroupLayoutData* group_layout =
+              (*self->layout_data_)[group];
+          EXPECT_TRUE(group_layout != nullptr);
+          EXPECT_TRUE(key->level == 0 || key->level == 1);
+          bool shift = key->level == 1;
+          return (*group_layout)[key->keycode * 2 + shift];
+        },
+        this);
     fl_keyboard_manager_set_redispatch_handler(
         manager_,
         [](FlKeyEvent* event, gpointer user_data) {
@@ -516,7 +511,7 @@ class KeyboardTester {
   }
 
   void setLayout(const MockLayoutData& layout) {
-    fl_mock_view_set_layout(view_, &layout);
+    layout_data_ = &layout;
     if (manager_ != nullptr) {
       fl_keyboard_manager_notify_layout_changed(manager_);
     }
@@ -527,6 +522,7 @@ class KeyboardTester {
   FlKeyboardManager* manager_ = nullptr;
   GPtrArray* redispatched_events_ = nullptr;
   bool during_redispatch_ = false;
+  const MockLayoutData* layout_data_;
 
   static gboolean _flushChannelMessagesCb(gpointer data) {
     g_autoptr(GMainLoop) loop = reinterpret_cast<GMainLoop*>(data);
@@ -538,6 +534,7 @@ class KeyboardTester {
 // Make sure that the keyboard can be disposed without crashes when there are
 // unresolved pending events.
 TEST(FlKeyboardManagerTest, DisposeWithUnresolvedPends) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   std::vector<CallRecord> call_records;
 
@@ -558,6 +555,7 @@ TEST(FlKeyboardManagerTest, DisposeWithUnresolvedPends) {
 }
 
 TEST(FlKeyboardManagerTest, SingleDelegateWithAsyncResponds) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   std::vector<CallRecord> call_records;
   g_autoptr(GPtrArray) redispatched =
@@ -645,6 +643,7 @@ TEST(FlKeyboardManagerTest, SingleDelegateWithAsyncResponds) {
 }
 
 TEST(FlKeyboardManagerTest, SingleDelegateWithSyncResponds) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   gboolean handler_handled = false;
   std::vector<CallRecord> call_records;
@@ -692,6 +691,7 @@ TEST(FlKeyboardManagerTest, SingleDelegateWithSyncResponds) {
 }
 
 TEST(FlKeyboardManagerTest, WithTwoAsyncDelegates) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   std::vector<CallRecord> call_records;
   g_autoptr(GPtrArray) redispatched =
@@ -751,6 +751,7 @@ TEST(FlKeyboardManagerTest, WithTwoAsyncDelegates) {
 }
 
 TEST(FlKeyboardManagerTest, TextInputHandlerReturnsFalse) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   g_autoptr(GPtrArray) redispatched =
       g_ptr_array_new_with_free_func(g_object_unref);
@@ -774,6 +775,7 @@ TEST(FlKeyboardManagerTest, TextInputHandlerReturnsFalse) {
 }
 
 TEST(FlKeyboardManagerTest, TextInputHandlerReturnsTrue) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   g_autoptr(GPtrArray) redispatched =
       g_ptr_array_new_with_free_func(g_object_unref);
@@ -794,6 +796,7 @@ TEST(FlKeyboardManagerTest, TextInputHandlerReturnsTrue) {
 }
 
 TEST(FlKeyboardManagerTest, CorrectLogicalKeyForLayouts) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
 
   std::vector<CallRecord> call_records;
@@ -887,6 +890,7 @@ TEST(FlKeyboardManagerTest, CorrectLogicalKeyForLayouts) {
 }
 
 TEST(FlKeyboardManagerTest, SynthesizeModifiersIfNeeded) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   std::vector<CallRecord> call_records;
   tester.recordEmbedderCallsTo(call_records);
@@ -925,6 +929,7 @@ TEST(FlKeyboardManagerTest, SynthesizeModifiersIfNeeded) {
 }
 
 TEST(FlKeyboardManagerTest, GetPressedState) {
+  ::testing::NiceMock<flutter::testing::MockKeymap> mock_keymap;
   KeyboardTester tester;
   tester.respondToTextInputWith(true);
 

shell/platform/linux/fl_keyboard_view_delegate.cc

@@ -32,13 +32,6 @@ gboolean fl_keyboard_view_delegate_text_filter_key_press(
       self, event);
 }
 
-guint fl_keyboard_view_delegate_lookup_key(FlKeyboardViewDelegate* self,
-                                           const GdkKeymapKey* key) {
-  g_return_val_if_fail(FL_IS_KEYBOARD_VIEW_DELEGATE(self), 0);
-
-  return FL_KEYBOARD_VIEW_DELEGATE_GET_IFACE(self)->lookup_key(self, key);
-}
-
 GHashTable* fl_keyboard_view_delegate_get_keyboard_state(
     FlKeyboardViewDelegate* self) {
   g_return_val_if_fail(FL_IS_KEYBOARD_VIEW_DELEGATE(self), nullptr);

shell/platform/linux/fl_keyboard_view_delegate.h

@@ -42,9 +42,6 @@ struct _FlKeyboardViewDelegateInterface {
   gboolean (*text_filter_key_press)(FlKeyboardViewDelegate* delegate,
                                     FlKeyEvent* event);
 
-  guint (*lookup_key)(FlKeyboardViewDelegate* view_delegate,
-                      const GdkKeymapKey* key);
-
   GHashTable* (*get_keyboard_state)(FlKeyboardViewDelegate* delegate);
 };
 
@@ -77,9 +74,6 @@ gboolean fl_keyboard_view_delegate_text_filter_key_press(
     FlKeyboardViewDelegate* delegate,
     FlKeyEvent* event);
 
-guint fl_keyboard_view_delegate_lookup_key(FlKeyboardViewDelegate* delegate,
-                                           const GdkKeymapKey* key);
-
 /**
  * fl_keyboard_view_delegate_get_keyboard_state:
  *