Bug 962140 Remove following char message of keydown message with found message r=jimm, a=lsblakk
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 11 Feb 2014 14:29:17 +0900
changeset 176355 0fe8271076588bc841831c0fde68eb7b6d758ede
parent 176354 e0abf66b4e045184e5bb98c5a12dae47ae4b008b
child 176356 84f6530e4f9c3fed6b639340249a194a596cedd0
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, lsblakk
bugs962140
milestone28.0
Bug 962140 Remove following char message of keydown message with found message r=jimm, a=lsblakk
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -732,31 +732,16 @@ NativeKey::NativeKey(nsWindowBase* aWidg
 
   mIsDeadKey =
     (IsFollowedByDeadCharMessage() ||
      keyboardLayout->IsDeadKey(mOriginalVirtualKeyCode, mModKeyState));
   mIsPrintableKey = KeyboardLayout::IsPrintableCharKey(mOriginalVirtualKeyCode);
 }
 
 bool
-NativeKey::IsFollowedByCharMessage() const
-{
-  MSG nextMsg;
-  if (mFakeCharMsgs) {
-    nextMsg = mFakeCharMsgs->ElementAt(0).GetCharMsg(mMsg.hwnd);
-  } else {
-    if (!WinUtils::PeekMessage(&nextMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
-                               PM_NOREMOVE | PM_NOYIELD)) {
-      return false;
-    }
-  }
-  return IsCharMessage(nextMsg);
-}
-
-bool
 NativeKey::IsFollowedByDeadCharMessage() const
 {
   MSG nextMsg;
   if (mFakeCharMsgs) {
     nextMsg = mFakeCharMsgs->ElementAt(0).GetCharMsg(mMsg.hwnd);
   } else {
     if (!WinUtils::PeekMessage(&nextMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
                                PM_NOREMOVE | PM_NOYIELD)) {
@@ -1070,18 +1055,19 @@ NativeKey::HandleKeyDownMessage(bool* aE
 
   // If we won't be getting a WM_CHAR, WM_SYSCHAR or WM_DEADCHAR, synthesize a
   // keypress for almost all keys
   if (NeedsToHandleWithoutFollowingCharMessages()) {
     return (DispatchPluginEventsAndDiscardsCharMessages() ||
             DispatchKeyPressEventsWithKeyboardLayout());
   }
 
-  if (IsFollowedByCharMessage()) {
-    return DispatchKeyPressEventForFollowingCharMessage();
+  MSG followingCharMsg;
+  if (GetFollowingCharMessage(followingCharMsg)) {
+    return DispatchKeyPressEventForFollowingCharMessage(followingCharMsg);
   }
 
   if (!mModKeyState.IsControl() && !mModKeyState.IsAlt() &&
       !mModKeyState.IsWin() && mIsPrintableKey) {
     // If this is simple KeyDown event but next message is not WM_CHAR,
     // this event may not input text, so we should ignore this event.
     // See bug 314130.
     return false;
@@ -1306,129 +1292,164 @@ GetMessageName(UINT aMessage)
     case WM_SYSDEADCHAR: return "WM_SYSDEADCHAR";
     case WM_UNICHAR:     return "WM_UNICHAR";
     default:             return "Unknown";
   }
 }
 
 #endif // #ifdef MOZ_CRASHREPORTER
 
-MSG
-NativeKey::RemoveFollowingCharMessage() const
+bool
+NativeKey::GetFollowingCharMessage(MSG& aCharMsg) const
 {
-  MOZ_ASSERT(IsFollowedByCharMessage());
+  MOZ_ASSERT(IsKeyDownMessage());
+
+  aCharMsg.message = WM_NULL;
 
   if (mFakeCharMsgs) {
-    MOZ_ASSERT(!mFakeCharMsgs->ElementAt(0).mConsumed,
-      "Doesn't assume that it's used for removing two or more char messages");
-    mFakeCharMsgs->ElementAt(0).mConsumed = true;
-    return mFakeCharMsgs->ElementAt(0).GetCharMsg(mMsg.hwnd);
+    FakeCharMsg& fakeCharMsg = mFakeCharMsgs->ElementAt(0);
+    if (fakeCharMsg.mConsumed) {
+      return false;
+    }
+    MSG charMsg = fakeCharMsg.GetCharMsg(mMsg.hwnd);
+    fakeCharMsg.mConsumed = true;
+    if (!IsCharMessage(charMsg)) {
+      return false;
+    }
+    aCharMsg = charMsg;
+    return true;
+  }
+
+  // If next key message is not char message, we should give up to find a
+  // related char message for the handling keydown event for now.
+  // Note that it's possible other applications may send other key message
+  // after we call TranslateMessage(). That may cause PeekMessage() failing
+  // to get char message for the handling keydown message.
+  MSG nextKeyMsg;
+  if (!WinUtils::PeekMessage(&nextKeyMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
+                             PM_NOREMOVE | PM_NOYIELD) ||
+      !IsCharMessage(nextKeyMsg)) {
+    return false;
   }
 
-  MSG msg;
-  if (!WinUtils::PeekMessage(&msg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
-                             PM_REMOVE | PM_NOYIELD)) {
-#ifdef MOZ_CRASHREPORTER
-    nsPrintfCString info("Handling message: %s (0x%08X), wParam: 0x%08X, "
-                         "lParam: 0x%08X, InSendMessageEx()=%s",
-                         GetMessageName(mMsg.message),
-                         mMsg.message, mMsg.wParam, mMsg.lParam,
-                         GetResultOfInSendMessageEx().get());
-    CrashReporter::AppendAppNotesToCrashReport(info);
-#endif // #ifdef MOZ_CRASHREPORTER
-    MOZ_CRASH("We lost the following char message");
-  }
-  if (!IsCharMessage(msg)) {
+  // On Metrofox, PeekMessage() sometimes returns WM_NULL even if we specify
+  // the message range.  So, if it returns WM_NULL, we should retry to get
+  // the following char message it was found above.
+  for (uint32_t i = 0; i < 5; i++) {
+    MSG removedMsg;
+    if (!WinUtils::PeekMessage(&removedMsg, nextKeyMsg.hwnd,
+                               nextKeyMsg.message, nextKeyMsg.message,
+                               PM_REMOVE | PM_NOYIELD)) {
 #ifdef MOZ_CRASHREPORTER
-    nsPrintfCString info("Handling message: %s (0x%08X), wParam: 0x%08X, "
-                         "lParam: 0x%08X, InSendMessageEx()=%s, "
-                         "Next key message: %s (0x%08X), "
-                         "wParam: 0x%08X, lParam: 0x%08X",
-                         GetMessageName(mMsg.message),
-                         mMsg.message, mMsg.wParam, mMsg.lParam,
-                         GetResultOfInSendMessageEx().get(),
-                         GetMessageName(msg.message),
-                         msg.message, msg.wParam, msg.lParam);
-    CrashReporter::AppendAppNotesToCrashReport(info);
+      nsPrintfCString info("\nHandling message: %s (0x%08X), wParam: 0x%08X, "
+                           "lParam: 0x%08X, InSendMessageEx()=%s, \n"
+                           "Found message: %s (0x%08X), wParam: 0x%08X, "
+                           "lParam: 0x%08X",
+                           GetMessageName(mMsg.message),
+                           mMsg.message, mMsg.wParam, mMsg.lParam,
+                           GetResultOfInSendMessageEx().get(),
+                           GetMessageName(nextKeyMsg.message),
+                           nextKeyMsg.message, nextKeyMsg.wParam,
+                           nextKeyMsg.lParam);
+      CrashReporter::AppendAppNotesToCrashReport(info);
 #endif // #ifdef MOZ_CRASHREPORTER
-    MOZ_CRASH("Next key message isn't a char message");
-  }
+      MOZ_CRASH("We lost the following char message");
+    }
 
-  return msg;
-}
+    // Retry for the strange case.
+    if (removedMsg.message == WM_NULL) {
+      continue;
+    }
 
-bool
-NativeKey::RemoveMessageAndDispatchPluginEvent(UINT aFirstMsg,
-                                               UINT aLastMsg) const
-{
-  MSG msg;
-  if (mFakeCharMsgs) {
-    DebugOnly<bool> found = false;
-    for (uint32_t i = 0; i < mFakeCharMsgs->Length(); i++) {
-      FakeCharMsg& fakeCharMsg = mFakeCharMsgs->ElementAt(i);
-      if (fakeCharMsg.mConsumed) {
-        continue;
-      }
-      MSG fakeMsg = fakeCharMsg.GetCharMsg(mMsg.hwnd);
-      if (fakeMsg.message < aFirstMsg || fakeMsg.message > aLastMsg) {
-        continue;
-      }
-      fakeCharMsg.mConsumed = true;
-      msg = fakeMsg;
-      found = true;
-      break;
+    if (removedMsg.message != nextKeyMsg.message ||
+        removedMsg.wParam != nextKeyMsg.wParam ||
+        removedMsg.lParam != nextKeyMsg.lParam) {
+#ifdef MOZ_CRASHREPORTER
+      nsPrintfCString info("\nHandling message: %s (0x%08X), wParam: 0x%08X, "
+                           "lParam: 0x%08X, InSendMessageEx()=%s, \n"
+                           "Found message: %s (0x%08X), wParam: 0x%08X, "
+                           "lParam: 0x%08X, \nRemoved message: %s (0x%08X), "
+                           "wParam: 0x%08X, lParam: 0x%08X",
+                           GetMessageName(mMsg.message),
+                           mMsg.message, mMsg.wParam, mMsg.lParam,
+                           GetResultOfInSendMessageEx().get(),
+                           GetMessageName(nextKeyMsg.message),
+                           nextKeyMsg.message, nextKeyMsg.wParam,
+                           nextKeyMsg.lParam,
+                           GetMessageName(removedMsg.message),
+                           removedMsg.message, removedMsg.wParam,
+                           removedMsg.lParam);
+      CrashReporter::AppendAppNotesToCrashReport(info);
+#endif // #ifdef MOZ_CRASHREPORTER
+      MOZ_CRASH("PeekMessage() removed unexpected message");
     }
-    MOZ_ASSERT(found, "Fake char message must be found");
-  } else {
-    WinUtils::PeekMessage(&msg, mMsg.hwnd, aFirstMsg, aLastMsg,
-                          PM_REMOVE | PM_NOYIELD);
+
+    aCharMsg = removedMsg;
+    return true;
   }
-  if (mWidget->Destroyed()) {
-    MOZ_CRASH("NativeKey tries to dispatch a plugin event on destroyed widget");
-  }
-  mWidget->DispatchPluginEvent(msg);
-  return mWidget->Destroyed();
+#ifdef MOZ_CRASHREPORTER
+  nsPrintfCString info("\nHandling message: %s (0x%08X), wParam: 0x%08X, "
+                       "lParam: 0x%08X, InSendMessageEx()=%s, \n"
+                       "Found message: %s (0x%08X), wParam: 0x%08X, "
+                       "lParam: 0x%08X, removed a lot of WM_NULL",
+                       GetMessageName(mMsg.message),
+                       mMsg.message, mMsg.wParam, mMsg.lParam,
+                       GetResultOfInSendMessageEx().get(),
+                       GetMessageName(nextKeyMsg.message),
+                       nextKeyMsg.message, nextKeyMsg.wParam,
+                       nextKeyMsg.lParam);
+  CrashReporter::AppendAppNotesToCrashReport(info);
+#endif // #ifdef MOZ_CRASHREPORTER
+  MOZ_CRASH("We lost the following char message");
+  return false;
 }
 
 bool
 NativeKey::DispatchPluginEventsAndDiscardsCharMessages() const
 {
   MOZ_ASSERT(IsKeyDownMessage());
 
-  if (mFakeCharMsgs) {
-    for (uint32_t i = 0; i < mFakeCharMsgs->Length(); i++) {
-      if (RemoveMessageAndDispatchPluginEvent(WM_KEYFIRST, WM_KEYLAST)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   // Remove a possible WM_CHAR or WM_SYSCHAR messages from the message queue.
   // They can be more than one because of:
   //  * Dead-keys not pairing with base character
   //  * Some keyboard layouts may map up to 4 characters to the single key
-  bool anyCharMessagesRemoved = true;
+  bool anyCharMessagesRemoved = false;
   MSG msg;
-  bool gotMsg =
-    WinUtils::PeekMessage(&msg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
-                          PM_NOREMOVE | PM_NOYIELD);
-  while (gotMsg && IsCharMessage(msg)) {
-    if (RemoveMessageAndDispatchPluginEvent(WM_KEYFIRST, WM_KEYLAST)) {
+  while (GetFollowingCharMessage(msg)) {
+    if (mWidget->Destroyed()) {
+      MOZ_CRASH(
+        "NativeKey tries to dispatch a plugin event on destroyed widget");
+    }
+    mWidget->DispatchPluginEvent(msg);
+    if (mWidget->Destroyed()) {
       return true;
     }
+
     anyCharMessagesRemoved = true;
-    gotMsg = WinUtils::PeekMessage(&msg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
-                                   PM_NOREMOVE | PM_NOYIELD);
   }
 
-  if (!anyCharMessagesRemoved &&
-      mDOMKeyCode == NS_VK_BACK && IsIMEDoingKakuteiUndo() &&
-      RemoveMessageAndDispatchPluginEvent(WM_CHAR, WM_CHAR)) {
-    return true;
+  if (!mFakeCharMsgs && !anyCharMessagesRemoved &&
+      mDOMKeyCode == NS_VK_BACK && IsIMEDoingKakuteiUndo()) {
+    // This is for a hack for ATOK and WXG.  So, PeekMessage() must scceed!
+    while (WinUtils::PeekMessage(&msg, mMsg.hwnd, WM_CHAR, WM_CHAR,
+                                 PM_REMOVE | PM_NOYIELD)) {
+      if (msg.message != WM_CHAR) {
+        if (msg.message != WM_NULL) {
+          MOZ_CRASH("Unexpected message was removed");
+        }
+        continue;
+      }
+      if (mWidget->Destroyed()) {
+        MOZ_CRASH(
+          "NativeKey tries to dispatch a plugin event on destroyed widget");
+      }
+      mWidget->DispatchPluginEvent(msg);
+      return mWidget->Destroyed();
+    }
+    MOZ_CRASH("NativeKey failed to get WM_CHAR for ATOK or WXG");
   }
 
   return false;
 }
 
 bool
 NativeKey::DispatchKeyPressEventsWithKeyboardLayout() const
 {
@@ -1589,63 +1610,64 @@ NativeKey::DispatchKeyPressEventsWithKey
       return true;
     }
   }
 
   return defaultPrevented;
 }
 
 bool
-NativeKey::DispatchKeyPressEventForFollowingCharMessage() const
+NativeKey::DispatchKeyPressEventForFollowingCharMessage(
+             const MSG& aCharMsg) const
 {
   MOZ_ASSERT(IsKeyDownMessage());
 
-  MSG msg = RemoveFollowingCharMessage();
   if (mFakeCharMsgs) {
-    if (IsDeadCharMessage(msg)) {
+    if (IsDeadCharMessage(aCharMsg)) {
       return false;
     }
 #ifdef DEBUG
     if (mIsPrintableKey) {
       nsPrintfCString log(
         "mOriginalVirtualKeyCode=0x%02X, mCommittedCharsAndModifiers={ "
         "mChars=[ 0x%04X, 0x%04X, 0x%04X, 0x%04X, 0x%04X ], mLength=%d }, "
         "wParam=0x%04X",
         mOriginalVirtualKeyCode, mCommittedCharsAndModifiers.mChars[0],
         mCommittedCharsAndModifiers.mChars[1],
         mCommittedCharsAndModifiers.mChars[2],
         mCommittedCharsAndModifiers.mChars[3],
         mCommittedCharsAndModifiers.mChars[4],
-        mCommittedCharsAndModifiers.mLength, msg.wParam);
+        mCommittedCharsAndModifiers.mLength, aCharMsg.wParam);
       if (mCommittedCharsAndModifiers.IsEmpty()) {
         log.Insert("length is zero: ", 0);
         NS_ERROR(log.get());
         NS_ABORT();
-      } else if (mCommittedCharsAndModifiers.mChars[0] != msg.wParam) {
+      } else if (mCommittedCharsAndModifiers.mChars[0] != aCharMsg.wParam) {
         log.Insert("character mismatch: ", 0);
         NS_ERROR(log.get());
         NS_ABORT();
       }
     }
 #endif // #ifdef DEBUG
-    return HandleCharMessage(msg);
+    return HandleCharMessage(aCharMsg);
   }
 
-  if (IsDeadCharMessage(msg)) {
+  if (IsDeadCharMessage(aCharMsg)) {
     if (!mWidget->PluginHasFocus()) {
       return false;
     }
-    return (mWidget->DispatchPluginEvent(msg) || mWidget->Destroyed());
+    return (mWidget->DispatchPluginEvent(aCharMsg) || mWidget->Destroyed());
   }
 
-  bool defaultPrevented = HandleCharMessage(msg);
+  bool defaultPrevented = HandleCharMessage(aCharMsg);
   // If a syschar keypress wasn't processed, Windows may want to
   // handle it to activate a native menu.
-  if (!defaultPrevented && IsSysCharMessage(msg)) {
-    ::DefWindowProcW(msg.hwnd, msg.message, msg.wParam, msg.lParam);
+  if (!defaultPrevented && IsSysCharMessage(aCharMsg)) {
+    ::DefWindowProcW(aCharMsg.hwnd, aCharMsg.message,
+                     aCharMsg.wParam, aCharMsg.lParam);
   }
   return defaultPrevented;
 }
 
 /*****************************************************************************
  * mozilla::widget::KeyboardLayout
  *****************************************************************************/
 
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -323,22 +323,16 @@ private:
 
   /**
    * "Kakutei-Undo" of ATOK or WXG (both of them are Japanese IME) causes
    * strange WM_KEYDOWN/WM_KEYUP/WM_CHAR message pattern.  So, when this
    * returns true, the caller needs to be careful for processing the messages.
    */
   bool IsIMEDoingKakuteiUndo() const;
 
-  /*
-   * Dispatches a plugin event after the specified message is removed.
-   * Returns true if the widget is destoyed.  Otherwise, false.
-   */
-  bool RemoveMessageAndDispatchPluginEvent(UINT aFirstMsg, UINT aLastMsg) const;
-
   bool IsKeyDownMessage() const
   {
     return (mMsg.message == WM_KEYDOWN || mMsg.message == WM_SYSKEYDOWN);
   }
   bool IsKeyUpMessage() const
   {
     return (mMsg.message == WM_KEYUP || mMsg.message == WM_SYSKEYUP);
   }
@@ -369,19 +363,18 @@ private:
   bool IsSysCharMessage(const MSG& aMSG) const
   {
     return IsSysCharMessage(aMSG.message);
   }
   bool IsSysCharMessage(UINT aMessage) const
   {
     return (aMessage == WM_SYSCHAR || aMessage == WM_SYSDEADCHAR);
   }
-  bool IsFollowedByCharMessage() const;
   bool IsFollowedByDeadCharMessage() const;
-  MSG RemoveFollowingCharMessage() const;
+  bool GetFollowingCharMessage(MSG& aCharMsg) const;
 
   /**
    * Wraps MapVirtualKeyEx() with MAPVK_VSC_TO_VK.
    */
   uint8_t ComputeVirtualKeyCodeFromScanCode() const;
 
   /**
    * Wraps MapVirtualKeyEx() with MAPVK_VSC_TO_VK_EX.
@@ -418,20 +411,20 @@ private:
    * WM_KEYDOWN or WM_SYSKEYDOWN message.  Additionally, dispatches plugin
    * events if it's necessary.
    * Returns true if the widget is destroyed.  Otherwise, false.
    */
   bool DispatchPluginEventsAndDiscardsCharMessages() const;
 
   /**
    * DispatchKeyPressEventForFollowingCharMessage() dispatches keypress event
-   * for following WM_*CHAR message.
+   * for following WM_*CHAR message which is removed and set to aCharMsg.
    * Returns true if the event is consumed.  Otherwise, false.
    */
-  bool DispatchKeyPressEventForFollowingCharMessage() const;
+  bool DispatchKeyPressEventForFollowingCharMessage(const MSG& aCharMsg) const;
 
   /**
    * Checkes whether the key event down message is handled without following
    * WM_CHAR messages.  For example, if following WM_CHAR message indicates
    * control character input, the WM_CHAR message is unclear whether it's
    * caused by a printable key with Ctrl or just a function key such as Enter
    * or Backspace.
    */