Bug 1257759 part.6 Keep event order between keyboard events and IME events in a plugin process r=jimm draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 14 Apr 2016 17:28:49 +0900
changeset 355640 42e304c45cd980f339b29526ab65854d196198ad
parent 355639 8140456de278956d2d594e85c7b397ae366b4962
child 355641 c6852423e47c40e9953b72061262730f7cce35d7
push id16345
push usermasayuki@d-toybox.com
push dateSat, 23 Apr 2016 09:42:11 +0000
reviewersjimm
bugs1257759
milestone48.0a1
Bug 1257759 part.6 Keep event order between keyboard events and IME events in a plugin process r=jimm On Windows, applications cannot handle IME messages asynchronously. Therefore, we cannot put off to send IME messages to plugin even if there are some pending keyboard events which were posted to the parent process. This patch makes PluginInstanceChild consume all key events which are returned from the parent process during IME composition. And when an IME composition is committed, mark pending key events as outdated. MozReview-Commit-ID: 7P3LEJ6pDir
dom/plugins/ipc/PluginInstanceChild.cpp
dom/plugins/ipc/PluginInstanceChild.h
--- a/dom/plugins/ipc/PluginInstanceChild.cpp
+++ b/dom/plugins/ipc/PluginInstanceChild.cpp
@@ -135,30 +135,33 @@ CreateDrawTargetForSurface(gfxASurface *
                                              &format);
   if (!drawTarget) {
     NS_RUNTIMEABORT("CreateDrawTargetForSurface failed in plugin");
   }
   aSurface->SetData(&kDrawTarget, drawTarget, nullptr);
   return drawTarget;
 }
 
+bool PluginInstanceChild::sIsIMEComposing = false;
+
 PluginInstanceChild::PluginInstanceChild(const NPPluginFuncs* aPluginIface,
                                          const nsCString& aMimeType,
                                          const uint16_t& aMode,
                                          const InfallibleTArray<nsCString>& aNames,
                                          const InfallibleTArray<nsCString>& aValues)
     : mPluginIface(aPluginIface)
     , mMimeType(aMimeType)
     , mMode(aMode)
     , mNames(aNames)
     , mValues(aValues)
 #if defined(XP_DARWIN)
     , mContentsScaleFactor(1.0)
 #endif
     , mPostingKeyEvents(0)
+    , mPostingKeyEventsOutdated(0)
     , mDrawingModel(kDefaultDrawingModel)
     , mCurrentDirectSurface(nullptr)
     , mAsyncInvalidateMutex("PluginInstanceChild::mAsyncInvalidateMutex")
     , mAsyncInvalidateTask(0)
     , mCachedWindowActor(nullptr)
     , mCachedElementActor(nullptr)
 #ifdef MOZ_WIDGET_GTK
     , mXEmbed(false)
@@ -1438,22 +1441,34 @@ PluginInstanceChild::Initialize()
 bool
 PluginInstanceChild::RecvHandledWindowedPluginKeyEvent(
                        const NativeEventData& aKeyEventData,
                        const bool& aIsConsumed)
 {
     // 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)) {
+    if (NS_WARN_IF(!mPostingKeyEvents && !mPostingKeyEventsOutdated)) {
+        return true;
+    }
+
+    // If there is outdated posting key events, we should consume the key
+    // events.
+    if (mPostingKeyEventsOutdated) {
+        mPostingKeyEventsOutdated--;
         return true;
     }
 
     mPostingKeyEvents--;
-    if (aIsConsumed) {
+
+    // If composition has been started after posting the key event,
+    // 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) {
@@ -1619,16 +1634,17 @@ PluginInstanceChild::PluginWindowProcInt
     if (!self) {
         NS_NOTREACHED("Badness!");
         return 0;
     }
 
     NS_ASSERTION(self->mPluginWindowHWND == hWnd, "Wrong window!");
     NS_ASSERTION(self->mPluginWndProc != PluginWindowProc, "Self-referential windowproc. Infinite recursion will happen soon.");
 
+    bool isIMECompositionMessage = false;
     switch (message) {
         // Adobe's shockwave positions the plugin window relative to the browser
         // frame when it initializes. With oopp disabled, this wouldn't have an
         // effect. With oopp, GeckoPluginWindow is a child of the parent plugin
         // window, so the move offsets the child within the parent. Generally
         // we don't want plugins moving or sizing our window, so we prevent
         // these changes here.
         case WM_WINDOWPOSCHANGING: {
@@ -1688,23 +1704,48 @@ PluginInstanceChild::PluginWindowProcInt
             break;
         case MOZ_WM_DEADCHAR:
             message = WM_DEADCHAR;
             break;
         case MOZ_WM_SYSDEADCHAR:
             message = WM_SYSDEADCHAR;
             break;
 
+        case WM_IME_STARTCOMPOSITION:
+            isIMECompositionMessage = true;
+            sIsIMEComposing = true;
+            break;
+        case WM_IME_ENDCOMPOSITION:
+            isIMECompositionMessage = true;
+            sIsIMEComposing = false;
+            break;
+        case WM_IME_COMPOSITION:
+            isIMECompositionMessage = true;
+            // XXX Some old IME may not send WM_IME_COMPOSITION_START or
+            //     WM_IME_COMPSOITION_END properly.  So, we need to check
+            //     WM_IME_COMPSOITION and if it includes commit string.
+            sIsIMEComposing = !(lParam & GCS_RESULTSTR);
+            break;
+
         // The plugin received keyboard focus, let the parent know so the dom
         // is up to date.
         case WM_MOUSEACTIVATE:
             self->CallPluginFocusChange(true);
             break;
     }
 
+    // When a composition is committed, there may be pending key
+    // events which were posted to the parent process before starting
+    // the composition.  Then, we shouldn't handle it since they are
+    // now outdated.
+    if (isIMECompositionMessage && !sIsIMEComposing) {
+        self->mPostingKeyEventsOutdated += self->mPostingKeyEvents;
+        self->mPostingKeyEvents = 0;
+    }
+
     // Prevent lockups due to plugins making rpc calls when the parent
     // is making a synchronous SendMessage call to the child window. Add
     // more messages as needed.
     if ((InSendMessageEx(nullptr)&(ISMEX_REPLIED|ISMEX_SEND)) == ISMEX_SEND) {
         switch(message) {
             case WM_CHILDACTIVATE:
             case WM_KILLFOCUS:
             ReplyMessage(0);
@@ -1756,16 +1797,24 @@ PluginInstanceChild::PluginWindowProcInt
     return res;
 }
 
 bool
 PluginInstanceChild::ShouldPostKeyMessage(UINT message,
                                           WPARAM wParam,
                                           LPARAM lParam)
 {
+    // If there is a composition, we shouldn't post the key message to the
+    // parent process because we cannot handle IME messages asynchronously.
+    // Therefore, if we posted key events to the parent process, the event
+    // order of the posted key events and IME events are shuffled.
+    if (sIsIMEComposing) {
+      return false;
+    }
+
     // If there are some pending keyboard events which are not handled in
     // the parent process, we should post the message for avoiding to shuffle
     // the key event order.
     if (mPostingKeyEvents) {
         return true;
     }
 
     // If we are not waiting calls of RecvOnKeyEventHandledBeforePlugin(),
--- a/dom/plugins/ipc/PluginInstanceChild.h
+++ b/dom/plugins/ipc/PluginInstanceChild.h
@@ -410,16 +410,17 @@ private:
     InfallibleTArray<nsCString> mValues;
     NPP_t mData;
     NPWindow mWindow;
 #if defined(XP_DARWIN)
     double mContentsScaleFactor;
 #endif
     double mCSSZoomFactor;
     uint32_t mPostingKeyEvents;
+    uint32_t mPostingKeyEventsOutdated;
     int16_t               mDrawingModel;
 
     NPAsyncSurface* mCurrentDirectSurface;
 
     // The surface hashtables below serve a few purposes. They let us verify
     // and retain extra information about plugin surfaces, and they let us
     // free shared memory that the plugin might forget to release.
     struct DirectBitmap {
@@ -665,16 +666,20 @@ 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;
 
+    // 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 {
     public:
         explicit AutoStackHelper(PluginInstanceChild* instance)
             : mInstance(instance)
         {