Bug 1334947 Treat a keydown event as inputting empty text if following char message has gone and gets WM_NULL message at calling PeekMessage() for removing a found char message r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 30 Jan 2017 15:43:09 +0900
changeset 467897 3844e835877c6365df8c6a3aa73e50b2c3c9aa72
parent 467877 b60810f29f91a75fbb5706441e5710c6f879674a
child 467912 f881c9f28fc5eb6eaf93976df9f3c1d29a170792
push id43295
push usermasayuki@d-toybox.com
push dateMon, 30 Jan 2017 09:07:13 +0000
reviewersm_kato
bugs1334947
milestone54.0a1
Bug 1334947 Treat a keydown event as inputting empty text if following char message has gone and gets WM_NULL message at calling PeekMessage() for removing a found char message r?m_kato There are still a lot of crash due to failing to get following WM_CHAR message. Almost half of them are, we found a WM_CHAR message but it's gone at removing the message from the queue. Although, we're still not sure what happen actually. It could be possible if somebody hooks PeekMessage() or something. Then, we can assume that the found message isn't necessary for the user because it must be removed by somebody when it became unnecessary or is handled. This patch mark a new bool flag, mCharMessageHasGone, as true in such case. Then, NativeKey will dispatch keypress events without following char messages and mark the event as consumed. MozReview-Commit-ID: mporX1sihC
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -1218,16 +1218,17 @@ NativeKey::NativeKey(nsWindowBase* aWidg
   , mModKeyState(aModKeyState)
   , mVirtualKeyCode(0)
   , mOriginalVirtualKeyCode(0)
   , mShiftedLatinChar(0)
   , mUnshiftedLatinChar(0)
   , mScanCode(0)
   , mIsExtended(false)
   , mIsDeadKey(false)
+  , mCharMessageHasGone(false)
   , mFakeCharMsgs(aFakeCharMsgs && aFakeCharMsgs->Length() ?
                     aFakeCharMsgs : nullptr)
 {
   MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
     ("%p NativeKey::NativeKey(aWidget=0x%p { GetWindowHandle()=0x%p }, "
      "aMessage=%s, aModKeyState=%s), sLatestInstance=0x%p",
      this, aWidget, aWidget->GetWindowHandle(), ToString(aMessage).get(),
      ToString(aModKeyState).get(), sLatestInstance));
@@ -1255,30 +1256,32 @@ NativeKey::NativeKey(nsWindowBase* aWidg
   MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
     ("%p   NativeKey::NativeKey(), mKeyboardLayout=0x%08X, "
      "mFocusedWndBeforeDispatch=0x%p, mDOMKeyCode=0x%04X, "
      "mKeyNameIndex=%s, mCodeNameIndex=%s, mModKeyState=%s, "
      "mVirtualKeyCode=%s, mOriginalVirtualKeyCode=%s, "
      "mCommittedCharsAndModifiers=%s, mInputtingStringAndModifiers=%s, "
      "mShiftedString=%s, mUnshiftedString=%s, mShiftedLatinChar=%s, "
      "mUnshiftedLatinChar=%s, mScanCode=0x%04X, mIsExtended=%s, "
-     "mIsDeadKey=%s, mIsPrintableKey=%s, mIsOverridingKeyboardLayout=%s",
+     "mIsDeadKey=%s, mIsPrintableKey=%s, mCharMessageHasGone=%s, "
+     "mIsOverridingKeyboardLayout=%s",
      this, mKeyboardLayout, mFocusedWndBeforeDispatch,
      GetDOMKeyCodeName(mDOMKeyCode).get(), ToString(mKeyNameIndex).get(),
      ToString(mCodeNameIndex).get(),
      ToString(mModKeyState).get(),
      GetVirtualKeyCodeName(mVirtualKeyCode).get(),
      GetVirtualKeyCodeName(mOriginalVirtualKeyCode).get(),
      ToString(mCommittedCharsAndModifiers).get(),
      ToString(mInputtingStringAndModifiers).get(),
      ToString(mShiftedString).get(), ToString(mUnshiftedString).get(),
      GetCharacterCodeName(mShiftedLatinChar).get(),
      GetCharacterCodeName(mUnshiftedLatinChar).get(),
      mScanCode, GetBoolName(mIsExtended), GetBoolName(mIsDeadKey),
-     GetBoolName(mIsPrintableKey), GetBoolName(mIsOverridingKeyboardLayout)));
+     GetBoolName(mIsPrintableKey), GetBoolName(mCharMessageHasGone),
+     GetBoolName(mIsOverridingKeyboardLayout)));
 }
 
 void
 NativeKey::InitWithKeyChar()
 {
   mScanCode = WinUtils::GetScanCode(mMsg.lParam);
   mIsExtended = WinUtils::IsExtendedScanCode(mMsg.lParam);
   switch (mMsg.message) {
@@ -1882,16 +1885,23 @@ NativeKey::InitKeyEvent(WidgetKeyboardEv
     MOZ_CRASH("NativeKey tries to dispatch a key event on destroyed widget");
   }
 
   LayoutDeviceIntPoint point(0, 0);
   mWidget->InitEvent(aKeyEvent, &point);
 
   switch (aKeyEvent.mMessage) {
     case eKeyDown:
+      // If it was followed by a char message but it was consumed by somebody,
+      // we should mark it as consumed because somebody must have handled it
+      // and we should prevent to do "double action" for the key operation.
+      if (mCharMessageHasGone) {
+        aKeyEvent.PreventDefaultBeforeDispatch();
+      }
+      MOZ_FALLTHROUGH;
     case eKeyDownOnPlugin:
       aKeyEvent.mKeyCode = mDOMKeyCode;
       // Unique id for this keydown event and its associated keypress.
       sUniqueKeyEventId++;
       aKeyEvent.mUniqueId = sUniqueKeyEventId;
       break;
     case eKeyUp:
     case eKeyUpOnPlugin:
@@ -1901,16 +1911,19 @@ NativeKey::InitKeyEvent(WidgetKeyboardEv
       // triggering the menu bar for ALT key accelerators used in assistive
       // technologies such as Window-Eyes and ZoomText or for switching open
       // state of IME.
       if (mOriginalVirtualKeyCode == VK_MENU && mMsg.message != WM_SYSKEYUP) {
         aKeyEvent.PreventDefaultBeforeDispatch();
       }
       break;
     case eKeyPress:
+      MOZ_ASSERT(!mCharMessageHasGone,
+                 "If following char message was consumed by somebody, "
+                 "keydown event should be consumed above");
       aKeyEvent.mUniqueId = sUniqueKeyEventId;
       break;
     default:
       MOZ_CRASH("Invalid event message");
   }
 
   aKeyEvent.mIsRepeat = IsRepeat();
   aKeyEvent.mKeyNameIndex = mKeyNameIndex;
@@ -2421,16 +2434,20 @@ NativeKey::HandleKeyDownMessage(bool* aE
     MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
       ("%p   NativeKey::HandleKeyDownMessage(), not dispatching keypress "
        "event because preceding keydown event was consumed",
        this));
     MaybeDispatchPluginEventsForRemovedCharMessages();
     return true;
   }
 
+  MOZ_ASSERT(!mCharMessageHasGone,
+             "If following char message was consumed by somebody, "
+             "keydown event should have been consumed before dispatch");
+
   // If mCommittedCharsAndModifiers was initialized with following char
   // messages, we should dispatch keypress events with its information.
   if (IsFollowedByPrintableCharOrSysCharMessage()) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
       ("%p   NativeKey::HandleKeyDownMessage(), tries to be dispatching "
        "keypress events with retrieved char messages...", this));
     return DispatchKeyPressEventsWithRetrievedCharMessages();
   }
@@ -2901,16 +2918,32 @@ NativeKey::GetFollowingCharMessage(MSG& 
             ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
              "remove a char message, but succeeded with GetMessage(), "
              "removedMsg=%s",
              this, ToString(removedMsg).get()));
           // Cancel to crash, but we need to check the removed message value.
           doCrash = false;
         }
       }
+      // If we've already removed some WM_NULL messages from the queue and
+      // the found message has already gone from the queue, let's treat the key
+      // as inputting no characters and already consumed.
+      else if (i > 0) {
+        MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+          ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
+           "remove a char message, but removed %d WM_NULL messages",
+           this, i));
+        // If the key is a printable key or a control key but tried to input
+        // a character, mark mCharMessageHasGone true for handling the keydown
+        // event as inputting empty string.
+        MOZ_ASSERT(!mCharMessageHasGone);
+        mFollowingCharMsgs.Clear();
+        mCharMessageHasGone = true;
+        return false;
+      }
       MOZ_LOG(sNativeKeyLogger, LogLevel::Error,
         ("%p   NativeKey::GetFollowingCharMessage(), FAILED, lost target "
          "message to remove, nextKeyMsg=%s",
          this, ToString(nextKeyMsg).get()));
     }
 
     if (doCrash) {
 #ifdef MOZ_CRASHREPORTER
@@ -3094,17 +3127,18 @@ NativeKey::MaybeDispatchPluginEventsForR
   return false;
 }
 
 void
 NativeKey::ComputeInputtingStringWithKeyboardLayout()
 {
   KeyboardLayout* keyboardLayout = KeyboardLayout::GetInstance();
 
-  if (KeyboardLayout::IsPrintableCharKey(mVirtualKeyCode)) {
+  if (KeyboardLayout::IsPrintableCharKey(mVirtualKeyCode) ||
+      mCharMessageHasGone) {
     mInputtingStringAndModifiers = mCommittedCharsAndModifiers;
   } else {
     mInputtingStringAndModifiers.Clear();
   }
   mShiftedString.Clear();
   mUnshiftedString.Clear();
   mShiftedLatinChar = mUnshiftedLatinChar = 0;
 
@@ -3594,22 +3628,30 @@ KeyboardLayout::InitNativeKey(NativeKey&
     }
   }
 
   // When it's followed by non-dead char message(s) for printable character(s),
   // aNativeKey should dispatch eKeyPress events for them rather than
   // information from keyboard layout because respecting WM_(SYS)CHAR messages
   // guarantees that we can always input characters which is expected by
   // the user even if the user uses odd keyboard layout.
-  if (aNativeKey.IsFollowedByPrintableCharOrSysCharMessage()) {
+  // Or, when it was followed by non-dead char message for a printable character
+  // but it's gone at removing the message from the queue, let's treat it
+  // as a key inputting empty string.
+  if (aNativeKey.IsFollowedByPrintableCharOrSysCharMessage() ||
+      aNativeKey.mCharMessageHasGone) {
     MOZ_ASSERT(!aNativeKey.IsCharMessage(aNativeKey.mMsg));
-    // Initialize mCommittedCharsAndModifiers with following char messages.
-    aNativeKey.
-      InitCommittedCharsAndModifiersWithFollowingCharMessages(aModKeyState);
-    MOZ_ASSERT(!aNativeKey.mCommittedCharsAndModifiers.IsEmpty());
+    if (aNativeKey.IsFollowedByPrintableCharOrSysCharMessage()) {
+      // Initialize mCommittedCharsAndModifiers with following char messages.
+      aNativeKey.
+        InitCommittedCharsAndModifiersWithFollowingCharMessages(aModKeyState);
+      MOZ_ASSERT(!aNativeKey.mCommittedCharsAndModifiers.IsEmpty());
+    } else {
+      aNativeKey.mCommittedCharsAndModifiers.Clear();
+    }
     aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
 
     // If it's not in dead key sequence, we don't need to do anymore here.
     if (!IsInDeadKeySequence()) {
       return;
     }
 
     // If it's in dead key sequence and dead char is inputted as is, we need to
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -377,16 +377,21 @@ private:
   WORD    mScanCode;
   bool    mIsExtended;
   bool    mIsDeadKey;
   // mIsPrintableKey is true if the key may be a printable key without
   // any modifier keys.  Otherwise, false.
   // Please note that the event may not cause any text input even if this
   // is true.  E.g., it might be dead key state or Ctrl key may be pressed.
   bool    mIsPrintableKey;
+  // mCharMessageHasGone is true if the message is a keydown message and
+  // it's followed by at least one char message but it's gone at removing
+  // from the queue.  This could occur if PeekMessage() or something is
+  // hooked by odd tool.
+  bool    mCharMessageHasGone;
   // mIsOverridingKeyboardLayout is true if the instance temporarily overriding
   // keyboard layout with specified by the constructor.
   bool    mIsOverridingKeyboardLayout;
 
   nsTArray<FakeCharMsg>* mFakeCharMsgs;
 
   // When a keydown event is dispatched at handling WM_APPCOMMAND, the computed
   // virtual keycode is set to this.  Even if we consume WM_APPCOMMAND message,