Fix dead key handling on Win32 (again) (flutter#28125)

- Fix dead character crashes the channel handler.
- Fix dead key messages are redispatched, making the message a regular character message.

Author: dkwingsmt

shell/platform/windows/keyboard_key_channel_handler.cc

@@ -103,6 +103,17 @@ int GetModsForKeyState() {
 #endif
 }
 
+// Revert the "character" for a dead key to its normal value, or the argument
+// unchanged otherwise.
+//
+// When a dead key is pressed, the WM_KEYDOWN's lParam is mapped to a special
+// value: the "normal character" | 0x80000000.  For example, when pressing
+// "dead key caret" (one that makes the following e into ê), its mapped
+// character is 0x8000005E. "Reverting" it gives 0x5E, which is character '^'.
+uint32_t _UndeadChar(uint32_t ch) {
+  return ch & ~0x80000000;
+}
+
 }  // namespace
 
 KeyboardKeyChannelHandler::KeyboardKeyChannelHandler(
@@ -130,7 +141,7 @@ void KeyboardKeyChannelHandler::KeyboardHook(
   event.AddMember(kKeyCodeKey, key, allocator);
   event.AddMember(kScanCodeKey, scancode | (extended ? kScancodeExtended : 0),
                   allocator);
-  event.AddMember(kCharacterCodePointKey, character, allocator);
+  event.AddMember(kCharacterCodePointKey, _UndeadChar(character), allocator);
   event.AddMember(kKeyMapKey, kWindowsKeyMap, allocator);
   event.AddMember(kModifiersKey, GetModsForKeyState(), allocator);
 

shell/platform/windows/keyboard_key_channel_handler_unittests.cc

@@ -16,6 +16,7 @@ namespace testing {
 
 namespace {
 static constexpr char kScanCodeKey[] = "scanCode";
+static constexpr char kCharacterCodePointKey[] = "characterCodePoint";
 static constexpr int kHandledScanCode = 0x14;
 static constexpr int kUnhandledScanCode = 0x15;
 static constexpr int kUnhandledScanCodeExtended = 0xe015;
@@ -108,5 +109,31 @@ TEST(KeyboardKeyChannelHandlerTest, ExtendedKeysAreSentToRedispatch) {
   EXPECT_EQ(received_scancode, kUnhandledScanCode);
 }
 
+TEST(KeyboardKeyChannelHandlerTest, DeadKeysDoNotCrash) {
+  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);
+          uint32_t character = (*message_doc)[kCharacterCodePointKey].GetUint();
+          EXPECT_EQ(character, (uint32_t)'^');
+        }
+        return true;
+      });
+
+  KeyboardKeyChannelHandler handler(&messenger);
+  // Extended key flag is passed to redispatched events if set.
+  handler.KeyboardHook(0xDD, 0x1a, WM_KEYDOWN, 0x8000005E, false, false,
+                       [](bool handled) {});
+
+  // EXPECT is done during the callback above.
+}
+
 }  // namespace testing
 }  // namespace flutter

shell/platform/windows/keyboard_key_embedder_handler.cc

@@ -27,17 +27,9 @@ constexpr SHORT kStateMaskToggled = 0x01;
 constexpr SHORT kStateMaskPressed = 0x80;
 
 const char* empty_character = "";
-}  // namespace
-
-// Get some bits of the char, from the start'th bit from the right (excluded)
-// to the end'th bit from the right (included).
-//
-// For example, _GetBit(0x1234, 8, 4) => 0x3.
-char _GetBit(char32_t ch, size_t start, size_t end) {
-  return (ch >> end) & ((1 << (start - end)) - 1);
-}
 
-// Revert the "character" for a dead key to its normal value.
+// Revert the "character" for a dead key to its normal value, or the argument
+// unchanged otherwise.
 //
 // When a dead key is pressed, the WM_KEYDOWN's lParam is mapped to a special
 // value: the "normal character" | 0x80000000.  For example, when pressing
@@ -47,6 +39,15 @@ uint32_t _UndeadChar(uint32_t ch) {
   return ch & ~0x80000000;
 }
 
+// Get some bits of the char, from the start'th bit from the right (excluded)
+// to the end'th bit from the right (included).
+//
+// For example, _GetBit(0x1234, 8, 4) => 0x3.
+char _GetBit(char32_t ch, size_t start, size_t end) {
+  return (ch >> end) & ((1 << (start - end)) - 1);
+}
+}  // namespace
+
 std::string ConvertChar32ToUtf8(char32_t ch) {
   std::string result;
   assert(0 <= ch && ch <= 0x10FFFF);

shell/platform/windows/keyboard_key_handler.cc

@@ -18,6 +18,11 @@ namespace {
 // emitting a warning on the console about unhandled events.
 static constexpr int kMaxPendingEvents = 1000;
 
+// Returns if a character sent by Win32 is a dead key.
+bool _IsDeadKey(uint32_t ch) {
+  return (ch & 0x80000000) != 0;
+}
+
 // Returns true if this key is a key down event of ShiftRight.
 //
 // This is a temporary solution to
@@ -261,7 +266,14 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id,
       if (event.unreplied == 0) {
         std::unique_ptr<PendingEvent> event_ptr = std::move(*iter);
         pending_responds_.erase(iter);
-        if (!event_ptr->any_handled) {
+        // Don't dispatch handled events or dead key events.
+        //
+        // Redispatching dead keys events makes Win32 ignore the dead key state
+        // and redispatches a normal character without combining it with the
+        // next letter key.
+        const bool should_redispatch =
+            !event_ptr->any_handled && !_IsDeadKey(event_ptr->character);
+        if (should_redispatch) {
           RedispatchEvent(std::move(event_ptr));
         }
       }

shell/platform/windows/keyboard_unittests.cc

@@ -175,9 +175,10 @@ class TestFlutterWindowsView : public FlutterWindowsView {
 
   uint32_t redispatch_char;
 
-  void InjectPendingEvents(MockFlutterWindowWin32* win32window,
-                           uint32_t redispatch_char) {
+  int InjectPendingEvents(MockFlutterWindowWin32* win32window,
+                          uint32_t redispatch_char) {
     std::vector<Win32Message> messages;
+    int num_pending_responds = pending_responds_.size();
     for (const SendInputInfo& input : pending_responds_) {
       const KEYBDINPUT kbdinput = input.kbdinput;
       const UINT message =
@@ -200,6 +201,7 @@ class TestFlutterWindowsView : public FlutterWindowsView {
 
     win32window->InjectMessageList(messages.size(), messages.data());
     pending_responds_.clear();
+    return num_pending_responds;
   }
 
   void SetKeyState(uint32_t key, bool pressed, bool toggled_on) {
@@ -294,10 +296,12 @@ class KeyboardTester {
   // Inject all events called with |SendInput| to the event queue,
   // then process the event queue.
   //
+  // Returns the number of events injected.
+  //
   // If |redispatch_char| is not 0, then WM_KEYDOWN events will
   // also redispatch a WM_CHAR event with that value as lparam.
-  void InjectPendingEvents(uint32_t redispatch_char = 0) {
-    view_->InjectPendingEvents(window_.get(), redispatch_char);
+  int InjectPendingEvents(uint32_t redispatch_char = 0) {
+    return view_->InjectPendingEvents(window_.get(), redispatch_char);
   }
 
   static bool test_response;
@@ -802,7 +806,7 @@ TEST(KeyboardTest, DeadKeyThatCombines) {
                        kNotSynthesized);
   clear_key_calls();
 
-  tester.InjectPendingEvents(0);  // No WM_DEADCHAR messages sent here.
+  EXPECT_EQ(tester.InjectPendingEvents(), 0);
   EXPECT_EQ(key_calls.size(), 0);
   clear_key_calls();