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
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 03 Feb 2017 14:30:22 +0900
changeset 340873 21f71652e49132a779ceccba50f92765a38154c1
parent 340872 6c56e3e2d6675be68f4164feaddbff6694293e97
child 340874 448a2d9d62c2951f6da51cd9ff5aa96ac88c67e8
push id31317
push usercbook@mozilla.com
push dateMon, 06 Feb 2017 11:55:02 +0000
treeherdermozilla-central@c5621cb6f907 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1336322, 1336080
milestone54.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 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 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;