Bug 1336322 - NativeKey::GetFollowingCharMessage() should treat the char message has gone if PeekMessage() failed to remove found char message and next key message becomes non-char message or different key's char message. r=m_kato, a=jcristau
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 03 Feb 2017 14:30:22 +0900
changeset 378227 ee21650b00374a2363f08866dff5f64cbcce63ce
parent 378226 4c3463d88059615faf6cc4ff55b792d699d79c1c
child 378228 05fae22830c0365b8173be4a2584ff0375b547a1
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, jcristau
bugs1336322, 1336080
milestone53.0a2
Bug 1336322 - NativeKey::GetFollowingCharMessage() should treat the char message has gone if PeekMessage() failed to remove found char message and next key message becomes non-char message or different key's char message. r=m_kato, a=jcristau This patch depends on bug 1336080. When PeekMessage() fails to remove found char message, NativeKey::GetFollowingCharMessage() tries to check next key message in the queue again. Then, when next key message becomes non-char message, such as WM_KEYDOWN or WM_KEYUP, the char message must be removed by odd keyboard layout or something. Similarly, when next key message is a char message but it's caused by different key, the found char message must be removed by one of them too. So, in these cases, NativeKey::GetFollowingCharMessage() should treat the key operation is already handled or canceled by the odd keyboard layout or somebody else. Additionally, in the latter case, following char message should be handled as orphan char message(s) as usual. MozReview-Commit-ID: 8ahs8I0HUQ2
widget/windows/KeyboardLayout.cpp
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -2912,35 +2912,64 @@ NativeKey::GetFollowingCharMessage(MSG& 
       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, kFoundCharMsg=%s",
            this, ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
-        return true;
+        return true; // XXX should return false in this case
       }
       // 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 becomes non-char message, this key operation
+      // may have already been consumed or canceled.
+      if (!IsCharMessage(nextKeyMsgInAllWindows)) {
+        MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+          ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
+           "remove a char message and next key message becomes non-char "
+           "message, nextKeyMsgInAllWindows=%s, nextKeyMsg=%s, "
+           "kFoundCharMsg=%s",
+           this, ToString(nextKeyMsgInAllWindows).get(),
+           ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
+        MOZ_ASSERT(!mCharMessageHasGone);
+        mFollowingCharMsgs.Clear();
+        mCharMessageHasGone = true;
+        return false;
+      }
+      // If next key message is still a char message but different key message,
+      // we should treat current key operation is consumed or canceled and
+      // next char message should be handled as an orphan char message later.
+      if (!IsSamePhysicalKeyMessage(nextKeyMsgInAllWindows, kFoundCharMsg)) {
+        MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+          ("%p   NativeKey::GetFollowingCharMessage(), WARNING, failed to "
+           "remove a char message and next key message becomes differnt key's "
+           "char message, nextKeyMsgInAllWindows=%s, nextKeyMsg=%s, "
+           "kFoundCharMsg=%s",
+           this, ToString(nextKeyMsgInAllWindows).get(),
+           ToString(nextKeyMsg).get(), ToString(kFoundCharMsg).get()));
+        MOZ_ASSERT(!mCharMessageHasGone);
+        mFollowingCharMsgs.Clear();
+        mCharMessageHasGone = true;
+        return false;
+      }
       // 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)) {
+      if (nextKeyMsgInAllWindows.message != nextKeyMsg.message) {
         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;