Bug 1257759 part.6 Keep event order between keyboard events and IME events in a plugin process r=jimm
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 14 Apr 2016 17:28:49 +0900
changeset 332510 2b6daf0220a4d81cdb4825f7261b5627c80c8f00
parent 332509 1952b7fec843cbb6e1b402c7a0e2a42ba9ba335f
child 332511 502b6b849493d82d320ff091a440ab144c79ac62
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1257759
milestone48.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 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)
         {