Track lock key down state instead of lock state (#20836)

This converts the GTK keyboard code to track the key down states of the lock modifiers NumLock and CapsLock so that they represent the actual "down" state of the key, rather than the lock state itself.

GTK tracks the lock state, and Flutter expects the down state.

ci/licenses_golden/licenses_flutter

@@ -1279,6 +1279,7 @@ FILE: ../../../flutter/shell/platform/linux/fl_json_method_codec.cc
 FILE: ../../../flutter/shell/platform/linux/fl_json_method_codec_test.cc
 FILE: ../../../flutter/shell/platform/linux/fl_key_event_plugin.cc
 FILE: ../../../flutter/shell/platform/linux/fl_key_event_plugin.h
+FILE: ../../../flutter/shell/platform/linux/fl_key_event_plugin_test.cc
 FILE: ../../../flutter/shell/platform/linux/fl_message_codec.cc
 FILE: ../../../flutter/shell/platform/linux/fl_message_codec_test.cc
 FILE: ../../../flutter/shell/platform/linux/fl_method_call.cc

shell/platform/linux/BUILD.gn

@@ -140,6 +140,7 @@ executable("flutter_linux_unittests") {
     "fl_dart_project_test.cc",
     "fl_json_message_codec_test.cc",
     "fl_json_method_codec_test.cc",
+    "fl_key_event_plugin_test.cc",
     "fl_message_codec_test.cc",
     "fl_method_channel_test.cc",
     "fl_method_codec_test.cc",

shell/platform/linux/fl_key_event_plugin.cc

@@ -23,7 +23,8 @@ static constexpr char kLinuxKeymap[] = "linux";
 struct _FlKeyEventPlugin {
   GObject parent_instance;
 
-  FlBasicMessageChannel* channel;
+  FlBasicMessageChannel* channel = nullptr;
+  GAsyncReadyCallback response_callback = nullptr;
 };
 
 G_DEFINE_TYPE(FlKeyEventPlugin, fl_key_event_plugin, G_TYPE_OBJECT)
@@ -42,35 +43,92 @@ static void fl_key_event_plugin_class_init(FlKeyEventPluginClass* klass) {
 
 static void fl_key_event_plugin_init(FlKeyEventPlugin* self) {}
 
-FlKeyEventPlugin* fl_key_event_plugin_new(FlBinaryMessenger* messenger) {
+FlKeyEventPlugin* fl_key_event_plugin_new(FlBinaryMessenger* messenger,
+                                          GAsyncReadyCallback response_callback,
+                                          const char* channel_name) {
   g_return_val_if_fail(FL_IS_BINARY_MESSENGER(messenger), nullptr);
 
   FlKeyEventPlugin* self = FL_KEY_EVENT_PLUGIN(
       g_object_new(fl_key_event_plugin_get_type(), nullptr));
 
   g_autoptr(FlJsonMessageCodec) codec = fl_json_message_codec_new();
-  self->channel = fl_basic_message_channel_new(messenger, kChannelName,
-                                               FL_MESSAGE_CODEC(codec));
+  self->channel = fl_basic_message_channel_new(
+      messenger, channel_name == nullptr ? kChannelName : channel_name,
+      FL_MESSAGE_CODEC(codec));
+  self->response_callback = response_callback;
 
   return self;
 }
 
 void fl_key_event_plugin_send_key_event(FlKeyEventPlugin* self,
-                                        GdkEventKey* event) {
+                                        GdkEventKey* event,
+                                        gpointer user_data) {
   g_return_if_fail(FL_IS_KEY_EVENT_PLUGIN(self));
   g_return_if_fail(event != nullptr);
 
   const gchar* type;
-  if (event->type == GDK_KEY_PRESS)
-    type = kTypeValueDown;
-  else if (event->type == GDK_KEY_RELEASE)
-    type = kTypeValueUp;
-  else
-    return;
+  switch (event->type) {
+    case GDK_KEY_PRESS:
+      type = kTypeValueDown;
+      break;
+    case GDK_KEY_RELEASE:
+      type = kTypeValueUp;
+      break;
+    default:
+      return;
+  }
 
   int64_t scan_code = event->hardware_keycode;
   int64_t unicodeScalarValues = gdk_keyval_to_unicode(event->keyval);
 
+  // For most modifier keys, GTK keeps track of the "pressed" state of the
+  // modifier keys. Flutter uses this information to keep modifier keys from
+  // being "stuck" when a key-up event is lost because it happens after the app
+  // loses focus.
+  //
+  // For Lock keys (ShiftLock, CapsLock, NumLock), however, GTK keeps track of
+  // the state of the locks themselves, not the "pressed" state of the key.
+  //
+  // Since Flutter expects the "pressed" state of the modifier keys, the lock
+  // state for these keys is discarded here, and it is substituted for the
+  // pressed state of the key.
+  //
+  // This code has the flaw that if a key event is missed due to the app losing
+  // focus, then this state will still think the key is pressed when it isn't,
+  // but that is no worse than for "regular" keys until we implement the
+  // sync/cancel events on app focus changes.
+  //
+  // This is necessary to do here instead of in the framework because Flutter
+  // does modifier key syncing in the framework, and will turn on/off these keys
+  // as being "pressed" whenever the lock is on, which breaks a lot of
+  // interactions (for example, if shift-lock is on, tab traversal is broken).
+  //
+  // TODO(gspencergoog): get rid of this tracked state when we are tracking the
+  // state of all keys and sending sync/cancel events when focus is gained/lost.
+
+  // Remove lock states from state mask.
+  guint state = event->state & ~(GDK_LOCK_MASK | GDK_MOD2_MASK);
+
+  static bool shift_lock_pressed = false;
+  static bool caps_lock_pressed = false;
+  static bool num_lock_pressed = false;
+  switch (event->keyval) {
+    case GDK_KEY_Num_Lock:
+      num_lock_pressed = event->type == GDK_KEY_PRESS;
+      break;
+    case GDK_KEY_Caps_Lock:
+      caps_lock_pressed = event->type == GDK_KEY_PRESS;
+      break;
+    case GDK_KEY_Shift_Lock:
+      shift_lock_pressed = event->type == GDK_KEY_PRESS;
+      break;
+  }
+
+  // Add back in the state matching the actual pressed state of the lock keys,
+  // not the lock states.
+  state |= (shift_lock_pressed || caps_lock_pressed) ? GDK_LOCK_MASK : 0x0;
+  state |= num_lock_pressed ? GDK_MOD2_MASK : 0x0;
+
   g_autoptr(FlValue) message = fl_value_new_map();
   fl_value_set_string_take(message, kTypeKey, fl_value_new_string(type));
   fl_value_set_string_take(message, kKeymapKey,
@@ -80,13 +138,12 @@ void fl_key_event_plugin_send_key_event(FlKeyEventPlugin* self,
                            fl_value_new_string(kGtkToolkit));
   fl_value_set_string_take(message, kKeyCodeKey,
                            fl_value_new_int(event->keyval));
-  fl_value_set_string_take(message, kModifiersKey,
-                           fl_value_new_int(event->state));
+  fl_value_set_string_take(message, kModifiersKey, fl_value_new_int(state));
   if (unicodeScalarValues != 0) {
     fl_value_set_string_take(message, kUnicodeScalarValuesKey,
                              fl_value_new_int(unicodeScalarValues));
   }
 
-  fl_basic_message_channel_send(self->channel, message, nullptr, nullptr,
-                                nullptr);
+  fl_basic_message_channel_send(self->channel, message, nullptr,
+                                self->response_callback, user_data);
 }

shell/platform/linux/fl_key_event_plugin.h

@@ -27,23 +27,35 @@ G_DECLARE_FINAL_TYPE(FlKeyEventPlugin,
 /**
  * fl_key_event_plugin_new:
  * @messenger: an #FlBinaryMessenger.
+ * @response_callback: the callback to call when a response is received.  If not
+ *                     given (nullptr), then the default response callback is
+ *                     used.
+ * @channel_name: the name of the channel to send key events on. If not given
+ *                (nullptr), then the standard key event channel name is used.
+ *                Typically used for tests to send on a test channel.
  *
  * Creates a new plugin that implements SystemChannels.keyEvent from the
  * Flutter services library.
  *
  * Returns: a new #FlKeyEventPlugin.
  */
-FlKeyEventPlugin* fl_key_event_plugin_new(FlBinaryMessenger* messenger);
+FlKeyEventPlugin* fl_key_event_plugin_new(
+    FlBinaryMessenger* messenger,
+    GAsyncReadyCallback response_callback = nullptr,
+    const char* channel_name = nullptr);
 
 /**
  * fl_key_event_plugin_send_key_event:
  * @plugin: an #FlKeyEventPlugin.
  * @event: a #GdkEventKey.
+ * @user_data: a pointer to user data to send to the response callback via the
+ *             messenger.
  *
  * Sends a key event to Flutter.
  */
 void fl_key_event_plugin_send_key_event(FlKeyEventPlugin* plugin,
-                                        GdkEventKey* event);
+                                        GdkEventKey* event,
+                                        gpointer user_data = nullptr);
 
 G_END_DECLS
 

shell/platform/linux/fl_key_event_plugin_test.cc

+// 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/linux/fl_key_event_plugin.h"
+
+#include <iostream>
+#include "gtest/gtest.h"
+
+#include "flutter/shell/platform/linux/fl_binary_messenger_private.h"
+#include "flutter/shell/platform/linux/fl_engine_private.h"
+#include "flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h"
+#include "flutter/shell/platform/linux/public/flutter_linux/fl_standard_message_codec.h"
+#include "flutter/shell/platform/linux/testing/mock_renderer.h"
+
+// Creates a mock engine that responds to platform messages.
+static FlEngine* make_mock_engine() {
+  g_autoptr(FlDartProject) project = fl_dart_project_new();
+  g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new();
+  g_autoptr(FlEngine) engine = fl_engine_new(project, FL_RENDERER(renderer));
+  g_autoptr(GError) engine_error = nullptr;
+  EXPECT_TRUE(fl_engine_start(engine, &engine_error));
+  EXPECT_EQ(engine_error, nullptr);
+
+  return static_cast<FlEngine*>(g_object_ref(engine));
+}
+
+const char* expected_value = nullptr;
+
+// Called when the message response is received in the send_key_event test.
+static void echo_response_cb(GObject* object,
+                             GAsyncResult* result,
+                             gpointer user_data) {
+  g_autoptr(GError) error = nullptr;
+  g_autoptr(FlValue) message = fl_basic_message_channel_send_finish(
+      FL_BASIC_MESSAGE_CHANNEL(object), result, &error);
+  EXPECT_NE(message, nullptr);
+  EXPECT_EQ(error, nullptr);
+
+  EXPECT_EQ(fl_value_get_type(message), FL_VALUE_TYPE_MAP);
+  EXPECT_STREQ(fl_value_to_string(message), expected_value);
+  g_main_loop_quit(static_cast<GMainLoop*>(user_data));
+}
+
+// Test sending a letter "A";
+TEST(FlKeyEventPluginTest, SendKeyEvent) {
+  g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0);
+
+  g_autoptr(FlEngine) engine = make_mock_engine();
+  FlBinaryMessenger* messenger = fl_binary_messenger_new(engine);
+  g_autoptr(FlKeyEventPlugin) plugin =
+      fl_key_event_plugin_new(messenger, echo_response_cb, "test/echo");
+
+  char string[] = "A";
+  GdkEventKey key_event = GdkEventKey{
+      GDK_KEY_PRESS,                         // event type
+      nullptr,                               // window (not needed)
+      FALSE,                                 // event was sent explicitly
+      12345,                                 // time
+      0x0,                                   // modifier state
+      GDK_KEY_A,                             // key code
+      1,                                     // length of string representation
+      reinterpret_cast<gchar*>(&string[0]),  // string representation
+      0x04,                                  // scan code
+      0,                                     // keyboard group
+      0,                                     // is a modifier
+  };
+
+  expected_value =
+      "{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65, "
+      "modifiers: 0, unicodeScalarValues: 65}";
+  fl_key_event_plugin_send_key_event(plugin, &key_event, loop);
+
+  // Blocks here until echo_response_cb is called.
+  g_main_loop_run(loop);
+
+  key_event = GdkEventKey{
+      GDK_KEY_RELEASE,                       // event type
+      nullptr,                               // window (not needed)
+      FALSE,                                 // event was sent explicitly
+      12345,                                 // time
+      0x0,                                   // modifier state
+      GDK_KEY_A,                             // key code
+      1,                                     // length of string representation
+      reinterpret_cast<gchar*>(&string[0]),  // string representation
+      0x04,                                  // scan code
+      0,                                     // keyboard group
+      0,                                     // is a modifier
+  };
+
+  expected_value =
+      "{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65, "
+      "modifiers: 0, unicodeScalarValues: 65}";
+  fl_key_event_plugin_send_key_event(plugin, &key_event, loop);
+
+  // Blocks here until echo_response_cb is called.
+  g_main_loop_run(loop);
+}
+
+void test_lock_event(guint key_code,
+                     const char* down_expected,
+                     const char* up_expected) {
+  g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0);
+
+  g_autoptr(FlEngine) engine = make_mock_engine();
+  FlBinaryMessenger* messenger = fl_binary_messenger_new(engine);
+  g_autoptr(FlKeyEventPlugin) plugin =
+      fl_key_event_plugin_new(messenger, echo_response_cb, "test/echo");
+
+  GdkEventKey key_event = GdkEventKey{
+      GDK_KEY_PRESS,  // event type
+      nullptr,        // window (not needed)
+      FALSE,          // event was sent explicitly
+      12345,          // time
+      0x10,           // modifier state
+      key_code,       // key code
+      1,              // length of string representation
+      nullptr,        // string representation
+      0x04,           // scan code
+      0,              // keyboard group
+      0,              // is a modifier
+  };
+
+  expected_value = down_expected;
+  fl_key_event_plugin_send_key_event(plugin, &key_event, loop);
+
+  // Blocks here until echo_response_cb is called.
+  g_main_loop_run(loop);
+
+  key_event.type = GDK_KEY_RELEASE;
+
+  expected_value = up_expected;
+  fl_key_event_plugin_send_key_event(plugin, &key_event, loop);
+
+  // Blocks here until echo_response_cb is called.
+  g_main_loop_run(loop);
+}
+
+// Test sending a "NumLock" keypress.
+TEST(FlKeyEventPluginTest, SendNumLockKeyEvent) {
+  test_lock_event(GDK_KEY_Num_Lock,
+                  "{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, "
+                  "keyCode: 65407, modifiers: 16}",
+                  "{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, "
+                  "keyCode: 65407, modifiers: 0}");
+}
+
+// Test sending a "CapsLock" keypress.
+TEST(FlKeyEventPluginTest, SendCapsLockKeyEvent) {
+  test_lock_event(GDK_KEY_Caps_Lock,
+                  "{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, "
+                  "keyCode: 65509, modifiers: 2}",
+                  "{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, "
+                  "keyCode: 65509, modifiers: 0}");
+}
+
+// Test sending a "ShiftLock" keypress.
+TEST(FlKeyEventPluginTest, SendShiftLockKeyEvent) {
+  test_lock_event(GDK_KEY_Shift_Lock,
+                  "{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, "
+                  "keyCode: 65510, modifiers: 2}",
+                  "{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, "
+                  "keyCode: 65510, modifiers: 0}");
+}