Bug 981960 Retry to remove found char message with GetMessage() when PeekMessage() fails to remove the message from the queue r=jimm
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 12 Mar 2014 20:04:17 +0900
changeset 191389 6df586c17279231c5c8385a751767f4b2dcd4208
parent 191388 91f4d6270e24c84a67b49e5a71583e4af041dd93
child 191390 f9d3af502654d59c4fc04c0e6bfeec848bc11439
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs981960
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 981960 Retry to remove found char message with GetMessage() when PeekMessage() fails to remove the message from the queue r=jimm
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -1310,16 +1310,30 @@ GetMessageName(UINT aMessage)
     case WM_NULL:        return "WM_NULL";
     default:             return "Unknown";
   }
 }
 
 #endif // #ifdef MOZ_CRASHREPORTER
 
 bool
+NativeKey::MayBeSameCharMessage(const MSG& aCharMsg1,
+                                const MSG& aCharMsg2) const
+{
+  // NOTE: Although, we don't know when this case occurs, the scan code value
+  //       in lParam may be changed from 0 to something.  The changed value
+  //       is different from the scan code of handling keydown message.
+  static const LPARAM kScanCodeMask = 0x00FF0000;
+  return
+    aCharMsg1.message == aCharMsg2.message &&
+    aCharMsg1.wParam == aCharMsg2.wParam &&
+    (aCharMsg1.lParam & ~kScanCodeMask) == (aCharMsg2.lParam & ~kScanCodeMask);
+}
+
+bool
 NativeKey::GetFollowingCharMessage(MSG& aCharMsg) const
 {
   MOZ_ASSERT(IsKeyDownMessage());
 
   aCharMsg.message = WM_NULL;
 
   if (mFakeCharMsgs) {
     FakeCharMsg& fakeCharMsg = mFakeCharMsgs->ElementAt(0);
@@ -1346,44 +1360,54 @@ NativeKey::GetFollowingCharMessage(MSG& 
       !IsCharMessage(nextKeyMsg)) {
     return false;
   }
 
   // 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++) {
-    static const LPARAM kScanCodeMask = 0x00FF0000;
-
-    MSG removedMsg;
+    MSG removedMsg, nextKeyMsgInAllWindows;
+    bool doCrash = false;
     if (!WinUtils::PeekMessage(&removedMsg, mMsg.hwnd,
                                nextKeyMsg.message, nextKeyMsg.message,
                                PM_REMOVE | PM_NOYIELD)) {
-      MSG nextKeyMsgInAllWindows;
+      // We meets unexpected case.  We should collect the message queue state
+      // and crash for reporting the bug.
+      doCrash = true;
       // The char message is redirected to different thread's window by focus
       // move or something or just cancelled by external application.
       if (!WinUtils::PeekMessage(&nextKeyMsgInAllWindows, 0,
                                  WM_KEYFIRST, WM_KEYLAST,
                                  PM_NOREMOVE | PM_NOYIELD)) {
         return true;
       }
-      // The char message is redirected to different window created by our
-      // thread.
-      if (nextKeyMsgInAllWindows.message == nextKeyMsg.message &&
-          nextKeyMsgInAllWindows.wParam == nextKeyMsg.wParam &&
-          (nextKeyMsgInAllWindows.lParam & ~kScanCodeMask) ==
-            (nextKeyMsg.lParam & ~kScanCodeMask) &&
-          nextKeyMsgInAllWindows.hwnd != mMsg.hwnd) {
-        aCharMsg = nextKeyMsgInAllWindows;
-        return true;
+      if (MayBeSameCharMessage(nextKeyMsgInAllWindows, nextKeyMsg)) {
+        // The char message is redirected to different window created by our
+        // thread.
+        if (nextKeyMsgInAllWindows.hwnd != mMsg.hwnd) {
+          aCharMsg = nextKeyMsgInAllWindows;
+          return true;
+        }
+        // The found char message still in the queue, but PeekMessage() failed
+        // to remove it only with PM_REMOVE.  Although, we don't know why this
+        // occurs.  However, this occurs acctually.
+        // Try to remove the char message with GetMessage() again.
+        if (WinUtils::GetMessage(&removedMsg, mMsg.hwnd,
+                                 nextKeyMsg.message, nextKeyMsg.message)) {
+          // Cancel to crash, but we need to check the removed message value.
+          doCrash = false;
+        }
       }
-      // Otherwise, perhaps, we meet a PeekMessage()'s bug.  Let's crash and
-      // collect the detail.
+    }
+
+    if (doCrash) {
 #ifdef MOZ_CRASHREPORTER
-      nsPrintfCString info("\nHandling message: %s (0x%08X), wParam: 0x%08X, "
+      nsPrintfCString info("\nPeekMessage() failed to remove char message! "
+                           "\nHandling message: %s (0x%08X), wParam: 0x%08X, "
                            "lParam: 0x%08X, hwnd=0x%p, InSendMessageEx()=%s, \n"
                            "Found message: %s (0x%08X), wParam: 0x%08X, "
                            "lParam: 0x%08X, hwnd=0x%p, "
                            "\nWM_NULL has been removed: %d, "
                            "\nNext key message in all windows: %s (0x%08X), "
                            "wParam: 0x%08X, lParam: 0x%08X, hwnd=0x%p, "
                            "time=%d, ",
                            GetMessageName(mMsg.message),
@@ -1421,22 +1445,20 @@ NativeKey::GetFollowingCharMessage(MSG& 
     // Retry for the strange case.
     if (removedMsg.message == WM_NULL) {
       continue;
     }
 
     // NOTE: Although, we don't know when this case occurs, the scan code value
     //       in lParam may be changed from 0 to something.  The changed value
     //       is different from the scan code of handling keydown message.
-    if (removedMsg.message != nextKeyMsg.message ||
-        removedMsg.wParam != nextKeyMsg.wParam ||
-        (removedMsg.lParam & ~kScanCodeMask) !=
-          (nextKeyMsg.lParam & ~kScanCodeMask)) {
+    if (!MayBeSameCharMessage(removedMsg, nextKeyMsg)) {
 #ifdef MOZ_CRASHREPORTER
-      nsPrintfCString info("\nHandling message: %s (0x%08X), wParam: 0x%08X, "
+      nsPrintfCString info("\nPeekMessage() removed unexpcted char message! "
+                           "\nHandling message: %s (0x%08X), wParam: 0x%08X, "
                            "lParam: 0x%08X, hwnd=0x%p, InSendMessageEx()=%s, "
                            "\nFound message: %s (0x%08X), wParam: 0x%08X, "
                            "lParam: 0x%08X, hwnd=0x%p, "
                            "\nRemoved message: %s (0x%08X), wParam: 0x%08X, "
                            "lParam: 0x%08X, hwnd=0x%p, ",
                            GetMessageName(mMsg.message),
                            mMsg.message, mMsg.wParam, mMsg.lParam, mMsg.hwnd,
                            GetResultOfInSendMessageEx().get(),
@@ -1484,17 +1506,18 @@ NativeKey::GetFollowingCharMessage(MSG& 
 #endif // #ifdef MOZ_CRASHREPORTER
       MOZ_CRASH("PeekMessage() removed unexpected message");
     }
 
     aCharMsg = removedMsg;
     return true;
   }
 #ifdef MOZ_CRASHREPORTER
-  nsPrintfCString info("\nHandling message: %s (0x%08X), wParam: 0x%08X, "
+  nsPrintfCString info("\nWe lost following char message! "
+                       "\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,
@@ -1512,43 +1535,45 @@ NativeKey::DispatchPluginEventsAndDiscar
 
   // 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 = false;
   MSG msg;
   while (GetFollowingCharMessage(msg)) {
-    if (mWidget->Destroyed()) {
-      MOZ_CRASH(
-        "NativeKey tries to dispatch a plugin event on destroyed widget");
+    if (msg.message == WM_NULL) {
+      continue;
     }
+    anyCharMessagesRemoved = true;
+    // If the window handle is changed, focused window must be changed.
+    // So, plugin shouldn't handle it anymore.
+    if (msg.hwnd != mMsg.hwnd) {
+      break;
+    }
+    MOZ_RELEASE_ASSERT(!mWidget->Destroyed(),
+      "NativeKey tries to dispatch a plugin event on destroyed widget");
     mWidget->DispatchPluginEvent(msg);
     if (mWidget->Destroyed()) {
       return true;
     }
-
-    anyCharMessagesRemoved = 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");
-        }
+        MOZ_RELEASE_ASSERT(msg.message == WM_NULL,
+                           "Unexpected message was removed");
         continue;
       }
-      if (mWidget->Destroyed()) {
-        MOZ_CRASH(
-          "NativeKey tries to dispatch a plugin event on destroyed widget");
-      }
+      MOZ_RELEASE_ASSERT(!mWidget->Destroyed(),
+        "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;
 }
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -363,16 +363,17 @@ private:
   bool IsSysCharMessage(const MSG& aMSG) const
   {
     return IsSysCharMessage(aMSG.message);
   }
   bool IsSysCharMessage(UINT aMessage) const
   {
     return (aMessage == WM_SYSCHAR || aMessage == WM_SYSDEADCHAR);
   }
+  bool MayBeSameCharMessage(const MSG& aCharMsg1, const MSG& aCharMsg2) const;
   bool IsFollowedByDeadCharMessage() const;
 
   /**
    * GetFollowingCharMessage() returns following char message of handling
    * keydown event.  If the message is found, this method returns true.
    * Otherwise, returns false.
    *
    * WARNING: Even if this returns true, aCharMsg may be WM_NULL or its