Bug 1257759 part.10 PluginInstanceChild should consume WM_*CHAR messages which follow consumed WM_*KEYDOWN or WM_*KEYUP message r=jimm draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 22 Apr 2016 14:22:03 +0900
changeset 355644 b99f8057d5e93035a769af2506292ff7d2cb8f4a
parent 355643 387ce72dcea23a92bd8c774fc54a8bff8da6c844
child 519248 747b7fecc76139d0a04006436cffdb834aeed89f
push id16345
push usermasayuki@d-toybox.com
push dateSat, 23 Apr 2016 09:42:11 +0000
reviewersjimm
bugs1257759
milestone48.0a1
Bug 1257759 part.10 PluginInstanceChild should consume WM_*CHAR messages which follow consumed WM_*KEYDOWN or WM_*KEYUP message r=jimm nsWindow for Windows cannot decide if a preceding WM_*KEYDOWN or WM_*KEYUP which is a preceding message of WM_*CHAR is consumed because nsWindow cannot know if WM_*CHAR message came from same window which received the preceding WM_*KEYDOWN or WM_*KEYUP. Therefore, PluginInstanceChild should do that. MozReview-Commit-ID: 1uuZ0nTJ5Xb
dom/plugins/ipc/PluginInstanceChild.cpp
dom/plugins/ipc/PluginInstanceChild.h
widget/windows/KeyboardLayout.cpp
--- a/dom/plugins/ipc/PluginInstanceChild.cpp
+++ b/dom/plugins/ipc/PluginInstanceChild.cpp
@@ -197,16 +197,19 @@ PluginInstanceChild::PluginInstanceChild
     , mSurfaceType(gfxSurfaceType::Max)
     , mCurrentInvalidateTask(nullptr)
     , mCurrentAsyncSetWindowTask(nullptr)
     , mPendingPluginCall(false)
     , mDoAlphaExtraction(false)
     , mHasPainted(false)
     , mSurfaceDifferenceRect(0,0,0,0)
     , mDestroyed(false)
+#ifdef XP_WIN
+    , mLastKeyEventConsumed(false)
+#endif // #ifdef XP_WIN
     , mStackDepth(0)
 {
     memset(&mWindow, 0, sizeof(mWindow));
     mWindow.type = NPWindowTypeWindow;
     mData.ndata = (void*) this;
     mData.pdata = nullptr;
 #if defined(MOZ_X11) && defined(XP_UNIX) && !defined(XP_MACOSX)
     mWindow.ws_info = &mWsInfo;
@@ -1438,16 +1441,40 @@ PluginInstanceChild::Initialize()
     return true;
 }
 
 bool
 PluginInstanceChild::RecvHandledWindowedPluginKeyEvent(
                        const NativeEventData& aKeyEventData,
                        const bool& aIsConsumed)
 {
+#if defined(OS_WIN)
+    const WinNativeKeyEventData* eventData =
+        static_cast<const WinNativeKeyEventData*>(aKeyEventData);
+    switch (eventData->mMessage) {
+        case WM_KEYDOWN:
+        case WM_SYSKEYDOWN:
+        case WM_KEYUP:
+        case WM_SYSKEYUP:
+            mLastKeyEventConsumed = aIsConsumed;
+            break;
+        case WM_CHAR:
+        case WM_SYSCHAR:
+        case WM_DEADCHAR:
+        case WM_SYSDEADCHAR:
+            // If preceding keydown or keyup event is consumed by the chrome
+            // process, we should consume WM_*CHAR messages too.
+            if (mLastKeyEventConsumed) {
+              return true;
+            }
+        default:
+            MOZ_CRASH("Needs to handle all messages posted to the parent");
+    }
+#endif // #if defined(OS_WIN)
+
     // Unknown key input shouldn't be sent to plugin for security.
     // XXX Is this possible if a plugin process which posted the message
     //     already crashed and this plugin process is recreated?
     if (NS_WARN_IF(!mPostingKeyEvents && !mPostingKeyEventsOutdated)) {
         return true;
     }
 
     // If there is outdated posting key events, we should consume the key
@@ -1463,18 +1490,16 @@ PluginInstanceChild::RecvHandledWindowed
     // we should discard the event since if we send the event to plugin,
     // the plugin may be confused and the result may be broken because
     // the event order is shuffled.
     if (aIsConsumed || sIsIMEComposing) {
         return true;
     }
 
 #if defined(OS_WIN)
-    const WinNativeKeyEventData* eventData =
-        static_cast<const WinNativeKeyEventData*>(aKeyEventData);
     UINT message = 0;
     switch (eventData->mMessage) {
         case WM_KEYDOWN:
             message = MOZ_WM_KEYDOWN;
             break;
         case WM_SYSKEYDOWN:
             message = MOZ_WM_SYSKEYDOWN;
             break;
@@ -1665,16 +1690,17 @@ PluginInstanceChild::PluginWindowProcInt
         }
 
         case WM_SETFOCUS:
             // If this gets focus, ensure that there is no pending key events.
             // Even if there were, we should ignore them for performance reason.
             // Although, such case shouldn't occur.
             NS_WARN_IF(self->mPostingKeyEvents > 0);
             self->mPostingKeyEvents = 0;
+            self->mLastKeyEventConsumed = false;
             break;
 
         case WM_KEYDOWN:
         case WM_SYSKEYDOWN:
         case WM_KEYUP:
         case WM_SYSKEYUP:
             if (self->MaybePostKeyMessage(message, wParam, lParam)) {
                 // If PreHandleKeyMessage() posts the message to the parent
--- a/dom/plugins/ipc/PluginInstanceChild.h
+++ b/dom/plugins/ipc/PluginInstanceChild.h
@@ -666,16 +666,24 @@ private:
     // Cached rectangle rendered to previous surface(mBackSurface)
     // Used for reading back to current surface and syncing data,
     // in plugin coordinates.
     nsIntRect mSurfaceDifferenceRect;
 
     // Has this instance been destroyed, either by ActorDestroy or NPP_Destroy?
     bool mDestroyed;
 
+#ifdef XP_WIN
+    // WM_*CHAR messages are never consumed by chrome process's widget.
+    // So, if preceding keydown or keyup event is consumed by reserved
+    // shortcut key in the chrome process, we shouldn't send the following
+    // WM_*CHAR messages to the plugin.
+    bool mLastKeyEventConsumed;
+#endif // #ifdef XP_WIN
+
     // While IME in the process has composition, this is set to true.
     // Otherwise, false.
     static bool sIsIMEComposing;
 
     // A counter is incremented by AutoStackHelper to indicate that there is an
     // active plugin call which should be preventing shutdown.
 public:
     class AutoStackHelper {
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -1502,17 +1502,17 @@ NativeKey::HandleKeyDownMessage(bool* aE
     }
 
     bool isIMEEnabled = WinUtils::IsIMEEnabled(mWidget->GetInputContext());
     EventMessage keyDownMessage =
       IsKeyMessageOnPlugin() ? eKeyDownOnPlugin : eKeyDown;
     WidgetKeyboardEvent keydownEvent(true, keyDownMessage, mWidget);
     nsEventStatus status = InitKeyEvent(keydownEvent, mModKeyState, &mMsg);
     bool dispatched =
-      mDispatcher->DispatchKeyboardEvent(eKeyDown, keydownEvent, status,
+      mDispatcher->DispatchKeyboardEvent(keyDownMessage, keydownEvent, status,
                                          const_cast<NativeKey*>(this));
     if (aEventDispatched) {
       *aEventDispatched = dispatched;
     }
     if (!dispatched) {
       // If the keydown event wasn't fired, there must be composition.
       // we don't need to do anything anymore.
       return false;
@@ -1760,21 +1760,21 @@ NativeKey::HandleKeyUpMessage(bool* aEve
     return false;
   }
 
   nsresult rv = mDispatcher->BeginNativeInputTransaction();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return true;
   }
 
-  WidgetKeyboardEvent keyupEvent(true, eKeyUp, mWidget);
+  EventMessage keyUpMessage = IsKeyMessageOnPlugin() ? eKeyUpOnPlugin : eKeyUp;
+  WidgetKeyboardEvent keyupEvent(true, keyUpMessage, mWidget);
   nsEventStatus status = InitKeyEvent(keyupEvent, mModKeyState, &mMsg);
-  EventMessage keyUpMessage = IsKeyMessageOnPlugin() ? eKeyUpOnPlugin : eKeyUp;
   bool dispatched =
-    mDispatcher->DispatchKeyboardEvent(eKeyUp, keyupEvent, status,
+    mDispatcher->DispatchKeyboardEvent(keyUpMessage, keyupEvent, status,
                                        const_cast<NativeKey*>(this));
   if (aEventDispatched) {
     *aEventDispatched = dispatched;
   }
   return status == nsEventStatus_eConsumeNoDefault;
 }
 
 bool