Bug 1336080 - When NativeKey::GetFollowingCharMessage() founds different message when it fails to remove a found char message, it should retry to remove the newly found message if it's caused by same physical key. r=m_kato, a=jcristau
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 02 Feb 2017 23:26:45 +0900
changeset 375942 4c3463d88059615faf6cc4ff55b792d699d79c1c
parent 375941 65e5f8c2d8160bb7a078726ccad358f647418959
child 375943 ee21650b00374a2363f08866dff5f64cbcce63ce
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, jcristau
bugs1336080
milestone53.0a2
Bug 1336080 - When NativeKey::GetFollowingCharMessage() founds different message when it fails to remove a found char message, it should retry to remove the newly found message if it's caused by same physical key. r=m_kato, a=jcristau When NativeKey::GetFollowingCharMessage() tries to remove a char message from the queue, the message might be changed by odd keyboard layout or something. In such case, if the new char message is also caused by same physical key, the char message must be overwritten. Then, we should take the new char message instead. Note that this patch saves original found char message into kFoundCharMsg and it's logged by each points for indicating if this case has occurred. MozReview-Commit-ID: HAduq8sfwFt
widget/windows/KeyboardLayout.cpp
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -2837,16 +2837,17 @@ NativeKey::GetFollowingCharMessage(MSG& 
   if (!WinUtils::PeekMessage(&nextKeyMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
                              PM_NOREMOVE | PM_NOYIELD) ||
       !IsCharMessage(nextKeyMsg)) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Verbose,
       ("%p   NativeKey::GetFollowingCharMessage(), there are no char messages",
        this));
     return false;
   }
+  const MSG kFoundCharMsg = nextKeyMsg;
 
   AutoRestore<MSG> saveLastRemovingMsg(mRemovingMsg);
   mRemovingMsg = nextKeyMsg;
 
   mReceivedMsg = sEmptyMSG;
   AutoRestore<MSG> ensureToClearRecivedMsg(mReceivedMsg);
 
   // On Metrofox, PeekMessage() sometimes returns WM_NULL even if we specify
@@ -2867,80 +2868,101 @@ NativeKey::GetFollowingCharMessage(MSG& 
       // recursive handling.  So, let's handle it in this instance.
       if (!IsEmptyMSG(mReceivedMsg)) {
         // If focus is moved to different window, we shouldn't handle it on
         // the widget.  Let's discard it for now.
         if (mReceivedMsg.hwnd != nextKeyMsg.hwnd) {
           MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
             ("%p   NativeKey::GetFollowingCharMessage(), WARNING, received a "
              "char message during removing it from the queue, but it's for "
-             "different window, mReceivedMsg=%s, nextKeyMsg=%s",
-             this, ToString(mReceivedMsg).get(), ToString(nextKeyMsg).get()));
+             "different window, mReceivedMsg=%s, nextKeyMsg=%s, "
+             "kFoundCharMsg=%s",
+             this, ToString(mReceivedMsg).get(), ToString(nextKeyMsg).get(),
+             ToString(kFoundCharMsg).get()));
           // There might still exist char messages, the loop of calling
           // this method should be continued.
           aCharMsg.message = WM_NULL;
           return true;
         }
         // Even if the received message is different from what we tried to
         // remove from the queue, let's take the received message as a part of
         // the result of this key sequence.
         if (mReceivedMsg.message != nextKeyMsg.message ||
             mReceivedMsg.wParam != nextKeyMsg.wParam ||
             mReceivedMsg.lParam != nextKeyMsg.lParam) {
           MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
             ("%p   NativeKey::GetFollowingCharMessage(), WARNING, received a "
              "char message during removing it from the queue, but it's "
              "differnt from what trying to remove from the queue, "
-             "aCharMsg=%s, nextKeyMsg=%s",
-             this, ToString(mReceivedMsg).get(), ToString(nextKeyMsg).get()));
+             "aCharMsg=%s, nextKeyMsg=%s, kFoundCharMsg=%s",
+             this, ToString(mReceivedMsg).get(), ToString(nextKeyMsg).get(),
+             ToString(kFoundCharMsg).get()));
         } else {
           MOZ_LOG(sNativeKeyLogger, LogLevel::Verbose,
             ("%p   NativeKey::GetFollowingCharMessage(), succeeded to retrieve "
-             "next char message via another instance, aCharMsg=%s",
-             this, ToString(mReceivedMsg).get()));
+             "next char message via another instance, aCharMsg=%s, "
+             "kFoundCharMsg=%s",
+             this, ToString(mReceivedMsg).get(),
+             ToString(kFoundCharMsg).get()));
         }
         aCharMsg = mReceivedMsg;
         return 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)) {
         MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
           ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
            "remove a char message, but it's already gone from all message "
-           "queues, nextKeyMsg=%s",
-           this, ToString(nextKeyMsg).get()));
+           "queues, nextKeyMsg=%s, kFoundCharMsg=%s",
+           this, ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
+        return true;
+      }
+      // The next key message is redirected to different window created by our
+      // thread, we should do nothing because we must not have focus.
+      if (nextKeyMsgInAllWindows.hwnd != mMsg.hwnd) {
+        aCharMsg = nextKeyMsgInAllWindows;
+        MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+          ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
+           "remove a char message, but found in another message queue, "
+           "nextKeyMsgInAllWindows=%s, nextKeyMsg=%s, kFoundCharMsg=%s",
+           this, ToString(nextKeyMsgInAllWindows).get(),
+           ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
         return true;
       }
+      // If next key message is still a char message but the message is changed,
+      // we should retry to remove the new message with PeekMessage() again.
+      if (IsCharMessage(nextKeyMsgInAllWindows) &&
+          nextKeyMsgInAllWindows.message != nextKeyMsg.message &&
+          IsSamePhysicalKeyMessage(nextKeyMsgInAllWindows, kFoundCharMsg)) {
+        MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+          ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
+           "remove a char message due to message change, let's retry to "
+           "remove the message with newly found char message, ",
+           "nextKeyMsgInAllWindows=%s, nextKeyMsg=%s, kFoundCharMsg=%s",
+           this, ToString(nextKeyMsgInAllWindows).get(),
+           ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
+        nextKeyMsg = nextKeyMsgInAllWindows;
+        continue;
+      }
       if (MayBeSameCharMessage(nextKeyMsgInAllWindows, nextKeyMsg)) {
-        // The char message is redirected to different window created by our
-        // thread.
-        if (nextKeyMsgInAllWindows.hwnd != mMsg.hwnd) {
-          aCharMsg = nextKeyMsgInAllWindows;
-          MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
-            ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
-             "remove a char message, but found in another message queue, "
-             "nextKeyMsgInAllWindows=%s",
-             this, ToString(nextKeyMsgInAllWindows).get()));
-          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)) {
           MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
             ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
              "remove a char message, but succeeded with GetMessage(), "
-             "removedMsg=%s",
-             this, ToString(removedMsg).get()));
+             "removedMsg=%s, kFoundCharMsg=%s",
+             this, ToString(removedMsg).get(), ToString(kFoundCharMsg).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) {
@@ -2970,17 +2992,17 @@ NativeKey::GetFollowingCharMessage(MSG& 
                            "\nFound message: %s, "
                            "\nWM_NULL has been removed: %d, "
                            "\nNext key message in all windows: %s, "
                            "time=%d, ",
                            KeyboardLayout::GetActiveLayout(),
                            KeyboardLayout::GetActiveLayoutName().get(),
                            ToString(mMsg).get(),
                            GetResultOfInSendMessageEx().get(),
-                           ToString(nextKeyMsg).get(), i,
+                           ToString(kFoundCharMsg).get(), i,
                            ToString(nextKeyMsgInAllWindows).get(),
                            nextKeyMsgInAllWindows.time);
       CrashReporter::AppendAppNotesToCrashReport(info);
       MSG nextMsg;
       if (WinUtils::PeekMessage(&nextMsg, 0, 0, 0,
                                 PM_NOREMOVE | PM_NOYIELD)) {
         nsPrintfCString info("\nNext message in all windows: %s, time=%d",
                              ToString(nextMsg).get(), nextMsg.time);
@@ -3009,32 +3031,33 @@ NativeKey::GetFollowingCharMessage(MSG& 
       if (!WinUtils::PeekMessage(&newNextKeyMsg, mMsg.hwnd,
                                  WM_KEYFIRST, WM_KEYLAST,
                                  PM_NOREMOVE | PM_NOYIELD)) {
         // If there is no key message, we should mark this keydown as consumed
         // because the key operation may have already been handled or canceled.
         MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
           ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
            "remove a char message because it's gone during removing it from "
-           "the queue, nextKeyMsg=%s",
-           this, ToString(nextKeyMsg).get()));
+           "the queue, nextKeyMsg=%s, kFoundCharMsg=%s",
+           this, ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
         MOZ_ASSERT(!mCharMessageHasGone);
         mFollowingCharMsgs.Clear();
         mCharMessageHasGone = true;
         return false;
       }
       if (!IsCharMessage(newNextKeyMsg)) {
         // If next key message becomes a non-char message, we should mark this
         // keydown as consumed because the key operation may have already been
         // handled or canceled.
         MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
           ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
            "remove a char message because it's gone during removing it from "
-           "the queue, nextKeyMsg=%s, newNextKeyMsg=%s",
-           this, ToString(nextKeyMsg).get(), ToString(newNextKeyMsg).get()));
+           "the queue, nextKeyMsg=%s, newNextKeyMsg=%s, kFoundCharMsg=%s",
+           this, ToString(nextKeyMsg).get(), ToString(newNextKeyMsg).get(),
+           ToString(kFoundCharMsg).get()));
         MOZ_ASSERT(!mCharMessageHasGone);
         mFollowingCharMsgs.Clear();
         mCharMessageHasGone = true;
         return false;
       }
       MOZ_LOG(sNativeKeyLogger, LogLevel::Debug,
         ("%p   NativeKey::GetFollowingCharMessage(), there is the message "
          "which is being tried to be removed from the queue, trying again...",
@@ -3070,40 +3093,42 @@ NativeKey::GetFollowingCharMessage(MSG& 
     // the possible scenarios.
     if (IsCharMessage(removedMsg) &&
         IsSamePhysicalKeyMessage(removedMsg, nextKeyMsg)) {
       aCharMsg = removedMsg;
       MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
         ("%p   NativeKey::GetFollowingCharMessage(), WARNING, succeeded to "
          "remove a char message, but the removed message was changed from "
          "the found message except their scancode, aCharMsg=%s, "
-         "nextKeyMsg(the found message)=%s",
-         this, ToString(aCharMsg).get(), ToString(nextKeyMsg).get()));
+         "nextKeyMsg=%s, kFoundCharMsg=%s",
+         this, ToString(aCharMsg).get(), ToString(nextKeyMsg).get(),
+         ToString(kFoundCharMsg).get()));
       return true;
     }
 
     // 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.
     MOZ_LOG(sNativeKeyLogger, LogLevel::Error,
       ("%p   NativeKey::GetFollowingCharMessage(), FAILED, removed message "
        "is really different from what we have already found, removedMsg=%s, "
-       "nextKeyMsg=%s",
-       this, ToString(removedMsg).get(), ToString(nextKeyMsg).get()));
+       "nextKeyMsg=%s, kFoundCharMsg=%s",
+       this, ToString(removedMsg).get(), ToString(nextKeyMsg).get(),
+       ToString(kFoundCharMsg).get()));
 #ifdef MOZ_CRASHREPORTER
     nsPrintfCString info("\nPeekMessage() removed unexpcted char message! "
                          "\nActive keyboard layout=0x%08X (%s), "
                          "\nHandling message: %s, InSendMessageEx()=%s, "
                          "\nFound message: %s, "
                          "\nRemoved message: %s, ",
                          KeyboardLayout::GetActiveLayout(),
                          KeyboardLayout::GetActiveLayoutName().get(),
                          ToString(mMsg).get(),
                          GetResultOfInSendMessageEx().get(),
-                         ToString(nextKeyMsg).get(),
+                         ToString(kFoundCharMsg).get(),
                          ToString(removedMsg).get());
     CrashReporter::AppendAppNotesToCrashReport(info);
     // What's the next key message?
     MSG nextKeyMsgAfter;
     if (WinUtils::PeekMessage(&nextKeyMsgAfter, mMsg.hwnd,
                               WM_KEYFIRST, WM_KEYLAST,
                               PM_NOREMOVE | PM_NOYIELD)) {
       nsPrintfCString info("\nNext key message after unexpected char message "
@@ -3137,17 +3162,17 @@ NativeKey::GetFollowingCharMessage(MSG& 
   nsPrintfCString info("\nWe lost following char message! "
                        "\nActive keyboard layout=0x%08X (%s), "
                        "\nHandling message: %s, InSendMessageEx()=%s, \n"
                        "Found message: %s, removed a lot of WM_NULL",
                        KeyboardLayout::GetActiveLayout(),
                        KeyboardLayout::GetActiveLayoutName().get(),
                        ToString(mMsg).get(),
                        GetResultOfInSendMessageEx().get(),
-                       ToString(nextKeyMsg).get());
+                       ToString(kFoundCharMsg).get());
   CrashReporter::AppendAppNotesToCrashReport(info);
 #endif // #ifdef MOZ_CRASHREPORTER
   MOZ_CRASH("We lost the following char message");
   return false;
 }
 
 bool
 NativeKey::MaybeDispatchPluginEventsForRemovedCharMessages() const