Bug 1302956 part.2 NativeKey shouldn't handle char message if it's created during another instance is trying to remove a char message from the queue r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 23 Sep 2016 16:40:01 +0900
changeset 416980 59d9463a2b983a6a69c2589ba986a9fa5ceb2ecc
parent 416869 e10076513f9507cd00877d35176e9cf1eb54963f
child 531998 63e2186f7b0aad8fbdaa5f4970deee584b94cbeb
push id30301
push usermasayuki@d-toybox.com
push dateFri, 23 Sep 2016 10:59:16 +0000
reviewersm_kato
bugs1302956
milestone52.0a1
Bug 1302956 part.2 NativeKey shouldn't handle char message if it's created during another instance is trying to remove a char message from the queue r?m_kato When NativeKey tries to remove a char message from the queue, another NativeKey instance might be created due to odd API hook or something. In such case, the old instance should handle the received message and the new instance should do nothing for keeping the order of message handling. This patch makes: * NativeKey::GetFollowingCharMsg() store removing char message to mRemovingMsg before calling PeekMessage() with PM_REMOVE. * NativeKey::InitWithChar() set the old instance's mReceivedMsg to the received char message and do nothing even if HandleCharMessage() is called later. * NativeKey::GetFollowingCharMsg() return received char message if mReceivedMsg is set during a call of PeekMessage(). MozReview-Commit-ID: KXYW0GgC9AY
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -1195,16 +1195,18 @@ NativeKey* NativeKey::sLatestInstance = 
 LazyLogModule sNativeKeyLogger("NativeKeyWidgets");
 
 NativeKey::NativeKey(nsWindowBase* aWidget,
                      const MSG& aMessage,
                      const ModifierKeyState& aModKeyState,
                      HKL aOverrideKeyboardLayout,
                      nsTArray<FakeCharMsg>* aFakeCharMsgs)
   : mLastInstance(sLatestInstance)
+  , mRemovingMsg(EmptyMSG())
+  , mReceivedMsg(EmptyMSG())
   , mWidget(aWidget)
   , mDispatcher(aWidget->GetTextEventDispatcher())
   , mMsg(aMessage)
   , mFocusedWndBeforeDispatch(::GetFocus())
   , mDOMKeyCode(0)
   , mKeyNameIndex(KEY_NAME_INDEX_Unidentified)
   , mCodeNameIndex(CODE_NAME_INDEX_UNKNOWN)
   , mModKeyState(aModKeyState)
@@ -1425,16 +1427,32 @@ NativeKey::InitWithKeyChar()
         default:
           MOZ_CRASH("Unsupported mOriginalVirtualKeyCode");
       }
       break;
     }
     case WM_CHAR:
     case WM_UNICHAR:
     case WM_SYSCHAR:
+      // If there is another instance and it is trying to remove a char message
+      // from the queue, this message should be handled in the old instance.
+      if (IsAnotherInstanceRemovingCharMessage()) {
+        // XXX Do we need to make mReceivedMsg an array?
+        MOZ_ASSERT(IsEmptyMSG(mLastInstance->mReceivedMsg));
+        MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+          ("%p   NativeKey::InitWithKeyChar(), WARNING, detecting another "
+           "instance is trying to remove a char message, so, this instance "
+           "should do nothing, mLastInstance=0x%p, mRemovingMsg=%s, "
+           "mReceivedMsg=%s",
+           this, mLastInstance, ToString(mLastInstance->mRemovingMsg).get(),
+           ToString(mLastInstance->mReceivedMsg).get()));
+        mLastInstance->mReceivedMsg = mMsg;
+        return;
+      }
+
       // NOTE: If other applications like a11y tools sends WM_*CHAR without
       //       scancode, we cannot compute virtual keycode.  I.e., with such
       //       applications, we cannot generate proper KeyboardEvent.code value.
 
       // We cannot compute the virtual key code from WM_CHAR message on WinXP
       // if it's caused by an extended key.
       if (!CanComputeVirtualKeyCodeFromScanCode()) {
         break;
@@ -2457,16 +2475,25 @@ NativeKey::HandleCharMessage(const MSG& 
 {
   MOZ_ASSERT(IsKeyDownMessage() || IsPrintableCharMessage(mMsg));
   MOZ_ASSERT(IsPrintableCharMessage(aCharMsg.message));
 
   if (aEventDispatched) {
     *aEventDispatched = false;
   }
 
+  if (IsPrintableCharMessage(mMsg) && IsAnotherInstanceRemovingCharMessage()) {
+    MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
+      ("%p   NativeKey::HandleCharMessage(), WARNING, does nothing because "
+       "the message should be handled in another instance removing this "
+       "message", this));
+    // Consume this for now because it will be handled by another instance.
+    return true;
+  }
+
   // Alt+Space key is handled by OS, we shouldn't touch it.
   if (mModKeyState.IsAlt() && !mModKeyState.IsControl() &&
       mVirtualKeyCode == VK_SPACE) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
       ("%p   NativeKey::HandleCharMessage(), doesn't dispatch keypress "
        "event due to Alt+Space", this));
     return false;
   }
@@ -2812,17 +2839,17 @@ NativeKey::MayBeSameCharMessage(const MS
   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
+NativeKey::GetFollowingCharMessage(MSG& aCharMsg)
 {
   MOZ_ASSERT(IsKeyDownMessage());
   MOZ_ASSERT(!IsKeyMessageOnPlugin());
 
   aCharMsg.message = WM_NULL;
 
   if (mFakeCharMsgs) {
     for (size_t i = 0; i < mFakeCharMsgs->Length(); i++) {
@@ -2851,28 +2878,74 @@ NativeKey::GetFollowingCharMessage(MSG& 
                              PM_NOREMOVE | PM_NOYIELD) ||
       !IsCharMessage(nextKeyMsg)) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Verbose,
       ("%p   NativeKey::GetFollowingCharMessage(), there are no char messages",
        this));
     return false;
   }
 
+  AutoRestore<MSG> saveLastRemovingMsg(mRemovingMsg);
+  mRemovingMsg = nextKeyMsg;
+
+  mReceivedMsg = EmptyMSG();
+  AutoRestore<MSG> ensureToClearRecivedMsg(mReceivedMsg);
+
   // 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, nextKeyMsgInAllWindows;
     bool doCrash = false;
     if (!WinUtils::PeekMessage(&removedMsg, mMsg.hwnd,
                                nextKeyMsg.message, nextKeyMsg.message,
                                PM_REMOVE | PM_NOYIELD)) {
       // We meets unexpected case.  We should collect the message queue state
       // and crash for reporting the bug.
       doCrash = true;
+
+      // If another instance was created for the removing message during trying
+      // to remove a char message, the instance didn't handle it for preventing
+      // 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()));
+          // 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()));
+        } else {
+          MOZ_LOG(sNativeKeyLogger, LogLevel::Verbose,
+            ("%p   NativeKey::GetFollowingCharMessage(), succeeded to retrieve "
+             "next char message via another instance, aCharMsg=%s",
+             this, ToString(mReceivedMsg).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 "
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -255,16 +255,22 @@ public:
   /**
    * Returns true if aChar is a control character which shouldn't be inputted
    * into focused text editor.
    */
   static bool IsControlChar(char16_t aChar);
 
 private:
   NativeKey* mLastInstance;
+  // mRemovingMsg is set at removing a char message from
+  // GetFollowingCharMessage().
+  MSG mRemovingMsg;
+  // mReceivedMsg is set when another instance starts to handle the message
+  // unexpectedly.
+  MSG mReceivedMsg;
   RefPtr<nsWindowBase> mWidget;
   RefPtr<TextEventDispatcher> mDispatcher;
   HKL mKeyboardLayout;
   MSG mMsg;
   // mFollowingCharMsgs stores WM_CHAR, WM_SYSCHAR, WM_DEADCHAR or
   // WM_SYSDEADCHAR message which follows WM_KEYDOWN.
   // Note that the stored messaged are already removed from the queue.
   nsTArray<MSG> mFollowingCharMsgs;
@@ -451,17 +457,17 @@ private:
   /**
    * 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
    *          hwnd may be different window.
    */
-  bool GetFollowingCharMessage(MSG& aCharMsg) const;
+  bool GetFollowingCharMessage(MSG& aCharMsg);
 
   /**
    * Whether the key event can compute virtual keycode from the scancode value.
    */
   bool CanComputeVirtualKeyCodeFromScanCode() const;
 
   /**
    * Wraps MapVirtualKeyEx() with MAPVK_VSC_TO_VK.
@@ -549,16 +555,36 @@ private:
   // due to (perhaps) odd API hook.  NativeKey should do nothing if given
   // message is tried to be retrieved by another instance.
 
   /**
    * sLatestInstacne is a pointer to the newest instance of NativeKey which is
    * handling a key or char message(s).
    */
   static NativeKey* sLatestInstance;
+
+  static const MSG EmptyMSG()
+  {
+    static bool sInitialized = false;
+    static MSG sEmptyMSG;
+    if (!sInitialized) {
+      memset(&sEmptyMSG, 0, sizeof(sEmptyMSG));
+      sInitialized = true;
+    }
+    return sEmptyMSG;
+  }
+  static bool IsEmptyMSG(const MSG& aMSG)
+  {
+    return !memcmp(&aMSG, &EmptyMSG(), sizeof(MSG));
+  }
+
+  bool IsAnotherInstanceRemovingCharMessage() const
+  {
+    return mLastInstance && !IsEmptyMSG(mLastInstance->mRemovingMsg);
+  }
 };
 
 class KeyboardLayout
 {
   friend class NativeKey;
 
 private:
   KeyboardLayout();