Bug 1349255 - Part 1: Make PBrowser.NotifyIMEFocus async to avoid UI jank. r=kanru,masayuki
authorHenry Chang <hchang@mozilla.com>
Mon, 08 May 2017 19:07:56 +0800
changeset 426452 8e03ce51525218b48c89ffd9b6ec8c62ae815944
parent 426451 142bbe1e613391635c0d5429f3ad076a179c9dc4
child 426453 0c78ece467a8c3ec69e74a03d70fcbf5ea14162e
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskanru, masayuki
bugs1349255, 1217700
milestone57.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 1349255 - Part 1: Make PBrowser.NotifyIMEFocus async to avoid UI jank. r=kanru,masayuki Telemetry and some performance profiles show that Msg_NotifyIMEFocus can take a few seconds to complete, and jank the browser. With bug 1217700, it removes the necessity of sync Msg_NotifyIMEFocus, so in this patch we make this async for performance improvement. MozReview-Commit-ID: 15eUwMJ2Q7H
dom/ipc/PBrowser.ipdl
dom/ipc/TabChild.h
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
ipc/ipdl/sync-messages.ini
widget/IMEData.h
widget/PuppetWidget.cpp
widget/PuppetWidget.h
--- a/dom/ipc/PBrowser.ipdl
+++ b/dom/ipc/PBrowser.ipdl
@@ -204,18 +204,18 @@ parent:
     /**
      * Notifies chrome that there is a focus change involving an editable
      * object (input, textarea, document, contentEditable. etc.)
      *
      *  contentCache Cache of content
      *  notification Whole data of the notification
      *  requests     Requests of notification for IME of the native widget
      */
-    nested(inside_cpow) sync NotifyIMEFocus(ContentCache contentCache,
-                                            IMENotification notification)
+    nested(inside_cpow) async NotifyIMEFocus(ContentCache contentCache,
+                                             IMENotification notification)
       returns (IMENotificationRequests requests);
 
     /**
      * Notifies chrome that there has been a change in text content
      * One call can encompass both a delete and an insert operation
      * Only called when NotifyIMEFocus returns PR_TRUE for mWantUpdates
      *
      *  contentCache Cache of content
@@ -248,17 +248,17 @@ parent:
      *
      *  contentCache Cache of content
      */
     nested(inside_cpow) async UpdateContentCache(ContentCache contentCache);
 
     /**
      * Notifies IME of mouse button event on a character in focused editor.
      *
-     * Returns true if the mouse button event is consumd by IME.
+     * Returns true if the mouse button event is consumed by IME.
      */
     nested(inside_cpow) sync NotifyIMEMouseButtonEvent(IMENotification notification)
       returns (bool consumedByIME);
 
     /**
      * Notifies chrome to position change
      *
      *  contentCache Cache of content
@@ -526,17 +526,16 @@ parent:
 
     async AccessKeyNotHandled(WidgetKeyboardEvent event);
 
     async SetHasBeforeUnload(bool aHasBeforeUnload);
 
 child:
     async NativeSynthesisResponse(uint64_t aObserverId, nsCString aResponse);
 
-
 parent:
 
     /**
      * Child informs the parent that the graphics objects are ready for
      * compositing.  This is sent when all pending changes have been
      * sent to the compositor and are ready to be shown on the next composite.
      * @see PCompositor
      * @see RequestNotifyAfterRemotePaint
--- a/dom/ipc/TabChild.h
+++ b/dom/ipc/TabChild.h
@@ -475,17 +475,16 @@ public:
 
   virtual mozilla::ipc::IPCResult RecvLoadRemoteScript(const nsString& aURL,
                                                        const bool& aRunInGlobalScope) override;
 
   virtual mozilla::ipc::IPCResult RecvAsyncMessage(const nsString& aMessage,
                                                    InfallibleTArray<CpowEntry>&& aCpows,
                                                    const IPC::Principal& aPrincipal,
                                                    const ClonedMessageData& aData) override;
-
   virtual mozilla::ipc::IPCResult
   RecvSwappedWithOtherRemoteLoader(const IPCTabContext& aContext) override;
 
   virtual PDocAccessibleChild*
   AllocPDocAccessibleChild(PDocAccessibleChild*, const uint64_t&,
                            const uint32_t&, const IAccessibleHolder&) override;
 
   virtual bool DeallocPDocAccessibleChild(PDocAccessibleChild*) override;
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1892,115 +1892,114 @@ TabParent::RecvHideTooltip()
 
   xulBrowserWindow->HideTooltip();
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvNotifyIMEFocus(const ContentCache& aContentCache,
                               const IMENotification& aIMENotification,
-                              IMENotificationRequests* aRequests)
+                              NotifyIMEFocusResolver&& aResolve)
 {
+  if (mIsDestroyed) {
+    return IPC_OK();
+  }
+
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget) {
-    *aRequests = IMENotificationRequests();
+    aResolve(IMENotificationRequests());
     return IPC_OK();
   }
 
   mContentCache.AssignContent(aContentCache, widget, &aIMENotification);
   IMEStateManager::NotifyIME(aIMENotification, widget, this);
 
+  IMENotificationRequests requests;
   if (aIMENotification.mMessage == NOTIFY_IME_OF_FOCUS) {
-    *aRequests = widget->IMENotificationRequestsRef();
+    requests = widget->IMENotificationRequestsRef();
   }
+  aResolve(requests);
+
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvNotifyIMETextChange(const ContentCache& aContentCache,
                                    const IMENotification& aIMENotification)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
+  if (!widget || !IMEStateManager::DoesTabParentHaveIMEFocus(this)) {
     return IPC_OK();
   }
-
-#ifdef DEBUG
-  const IMENotificationRequests& requests =
-    widget->IMENotificationRequestsRef();
-  NS_ASSERTION(requests.WantTextChange(),
-    "Don't call Send/RecvNotifyIMETextChange without NOTIFY_TEXT_CHANGE");
-#endif
-
   mContentCache.AssignContent(aContentCache, widget, &aIMENotification);
   mContentCache.MaybeNotifyIME(widget, aIMENotification);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvNotifyIMECompositionUpdate(
              const ContentCache& aContentCache,
              const IMENotification& aIMENotification)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
+  if (!widget || !IMEStateManager::DoesTabParentHaveIMEFocus(this)) {
     return IPC_OK();
   }
   mContentCache.AssignContent(aContentCache, widget, &aIMENotification);
   mContentCache.MaybeNotifyIME(widget, aIMENotification);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvNotifyIMESelection(const ContentCache& aContentCache,
                                   const IMENotification& aIMENotification)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
+  if (!widget || !IMEStateManager::DoesTabParentHaveIMEFocus(this)) {
     return IPC_OK();
   }
   mContentCache.AssignContent(aContentCache, widget, &aIMENotification);
   mContentCache.MaybeNotifyIME(widget, aIMENotification);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvUpdateContentCache(const ContentCache& aContentCache)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
+  if (!widget || !IMEStateManager::DoesTabParentHaveIMEFocus(this)) {
     return IPC_OK();
   }
 
   mContentCache.AssignContent(aContentCache, widget);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvNotifyIMEMouseButtonEvent(
              const IMENotification& aIMENotification,
              bool* aConsumedByIME)
 {
 
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
+  if (!widget || !IMEStateManager::DoesTabParentHaveIMEFocus(this)) {
     *aConsumedByIME = false;
     return IPC_OK();
   }
   nsresult rv = IMEStateManager::NotifyIME(aIMENotification, widget, this);
   *aConsumedByIME = rv == NS_SUCCESS_EVENT_CONSUMED;
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvNotifyIMEPositionChange(const ContentCache& aContentCache,
                                        const IMENotification& aIMENotification)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
+  if (!widget || !IMEStateManager::DoesTabParentHaveIMEFocus(this)) {
     return IPC_OK();
   }
   mContentCache.AssignContent(aContentCache, widget, &aIMENotification);
   mContentCache.MaybeNotifyIME(widget, aIMENotification);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -189,17 +189,17 @@ public:
   virtual mozilla::ipc::IPCResult RecvAsyncMessage(const nsString& aMessage,
                                                    InfallibleTArray<CpowEntry>&& aCpows,
                                                    const IPC::Principal& aPrincipal,
                                                    const ClonedMessageData& aData) override;
 
   virtual mozilla::ipc::IPCResult
   RecvNotifyIMEFocus(const ContentCache& aContentCache,
                      const widget::IMENotification& aEventMessage,
-                     widget::IMENotificationRequests* aRequests) override;
+                     NotifyIMEFocusResolver&& aResolve) override;
 
   virtual mozilla::ipc::IPCResult
   RecvNotifyIMETextChange(const ContentCache& aContentCache,
                           const widget::IMENotification& aEventMessage) override;
 
   virtual mozilla::ipc::IPCResult
   RecvNotifyIMECompositionUpdate(const ContentCache& aContentCache,
                                  const widget::IMENotification& aEventMessage) override;
--- a/ipc/ipdl/sync-messages.ini
+++ b/ipc/ipdl/sync-messages.ini
@@ -818,18 +818,16 @@ description =
 [PBackgroundIndexedDBUtils::GetFileReferences]
 description =
 [PBrowser::SyncMessage]
 description =
 [PBrowser::PPluginWidget]
 description =
 [PBrowser::DispatchFocusToTopLevelWindow]
 description =
-[PBrowser::NotifyIMEFocus]
-description =
 [PBrowser::NotifyIMEMouseButtonEvent]
 description =
 [PBrowser::RequestIMEToCommitComposition]
 description =
 [PBrowser::StartPluginIME]
 description =
 [PBrowser::GetInputContext]
 description =
--- a/widget/IMEData.h
+++ b/widget/IMEData.h
@@ -46,17 +46,21 @@ struct IMENotificationRequests final
     // NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR is used when mouse button is pressed
     // or released on a character in the focused editor.  The notification is
     // notified to IME as a mouse event.  If it's consumed by IME, NotifyIME()
     // returns NS_SUCCESS_EVENT_CONSUMED.  Otherwise, it returns NS_OK if it's
     // handled without any error.
     NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR    = 1 << 3,
     // NOTE: NOTIFY_DURING_DEACTIVE isn't supported in environments where two
     //       or more compositions are possible.  E.g., Mac and Linux (GTK).
-    NOTIFY_DURING_DEACTIVE               = 1 << 7
+    NOTIFY_DURING_DEACTIVE               = 1 << 7,
+
+    NOTIFY_ALL = NOTIFY_TEXT_CHANGE |
+                 NOTIFY_POSITION_CHANGE |
+                 NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR,
   };
 
   IMENotificationRequests()
     : mWantUpdates(NOTIFY_NOTHING)
   {
   }
 
   explicit IMENotificationRequests(Notifications aWantUpdates)
@@ -102,17 +106,17 @@ struct IMENotificationRequests final
   {
     return !!(mWantUpdates & NOTIFY_DURING_DEACTIVE);
   }
 
   Notifications mWantUpdates;
 };
 
 /**
- * Contains IMEStatus plus information about the current 
+ * Contains IMEStatus plus information about the current
  * input context that the IME can use as hints if desired.
  */
 
 struct IMEState final
 {
   /**
    * IME enabled states, the mEnabled value of
    * SetInputContext()/GetInputContext() should be one value of following
@@ -772,17 +776,17 @@ struct IMENotification final
     }
     uint32_t NewLength() const
     {
       MOZ_ASSERT(IsValid());
       return mAddedEndOffset - mStartOffset;
     }
 
     // Positive if text is added. Negative if text is removed.
-    int64_t Difference() const 
+    int64_t Difference() const
     {
       return mAddedEndOffset - mRemovedEndOffset;
     }
 
     bool IsInInt32Range() const
     {
       MOZ_ASSERT(IsValid());
       return mStartOffset <= INT32_MAX &&
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -23,16 +23,17 @@
 #include "mozilla/TextEvents.h"
 #include "mozilla/Unused.h"
 #include "BasicLayers.h"
 #include "PuppetWidget.h"
 #include "nsContentUtils.h"
 #include "nsIWidgetListener.h"
 #include "imgIContainer.h"
 #include "nsView.h"
+#include "nsPrintfCString.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using namespace mozilla::hal;
 using namespace mozilla::gfx;
 using namespace mozilla::layers;
 using namespace mozilla::widget;
 
@@ -538,17 +539,17 @@ PuppetWidget::ClearNativeTouchSequence(n
 {
   AutoObserverNotifier notifier(aObserver, "cleartouch");
   if (!mTabChild) {
     return NS_ERROR_FAILURE;
   }
   mTabChild->SendClearNativeTouchSequence(notifier.SaveObserver());
   return NS_OK;
 }
- 
+
 void
 PuppetWidget::SetConfirmedTargetAPZC(
                 uint64_t aInputBlockId,
                 const nsTArray<ScrollableLayerGuid>& aTargets) const
 {
   if (mTabChild) {
     mTabChild->SetTargetAPZC(aInputBlockId, aTargets);
   }
@@ -800,21 +801,29 @@ PuppetWidget::NotifyIMEOfFocusChange(con
         return NS_ERROR_FAILURE;
       }
     }
   } else {
     // When IME loses focus, we don't need to store anything.
     mContentCache.Clear();
   }
 
-  mIMENotificationRequestsOfParent = IMENotificationRequests();
-  if (!mTabChild->SendNotifyIMEFocus(mContentCache, aIMENotification,
-                                     &mIMENotificationRequestsOfParent)) {
-    return NS_ERROR_FAILURE;
-  }
+  mIMENotificationRequestsOfParent =
+  IMENotificationRequests(IMENotificationRequests::NOTIFY_ALL);
+  RefPtr<PuppetWidget> self = this;
+  mTabChild->SendNotifyIMEFocus(mContentCache, aIMENotification)->Then(
+    mTabChild->TabGroup()->EventTargetFor(TaskCategory::UI),
+    __func__,
+    [self] (IMENotificationRequests aRequests) {
+      self->mIMENotificationRequestsOfParent = aRequests;
+    },
+    [self] (mozilla::ipc::PromiseRejectReason aReason) {
+      NS_WARNING("SendNotifyIMEFocus got rejected.");
+    });
+
   return NS_OK;
 }
 
 nsresult
 PuppetWidget::NotifyIMEOfCompositionUpdate(
                 const IMENotification& aIMENotification)
 {
   if (NS_WARN_IF(!mTabChild)) {
@@ -875,17 +884,17 @@ PuppetWidget::NotifyIMEOfSelectionChange
   // available.
   if (NS_WARN_IF(mInputContext.mIMEState.mEnabled == IMEState::PLUGIN)) {
     return NS_ERROR_FAILURE;
   }
 
   // Note that selection change must be notified after text change if it occurs.
   // Therefore, we don't need to query text content again here.
   mContentCache.SetSelection(
-    this, 
+    this,
     aIMENotification.mSelectionChangeData.mOffset,
     aIMENotification.mSelectionChangeData.Length(),
     aIMENotification.mSelectionChangeData.mReversed,
     aIMENotification.mSelectionChangeData.GetWritingMode());
 
   mTabChild->SendNotifyIMESelection(mContentCache, aIMENotification);
 
   return NS_OK;
--- a/widget/PuppetWidget.h
+++ b/widget/PuppetWidget.h
@@ -287,16 +287,17 @@ public:
                           const FrameMetrics::ViewID& aViewId,
                           const CSSRect& aRect,
                           const uint32_t& aFlags) override;
 
   virtual bool HasPendingInputEvent() override;
 
   void HandledWindowedPluginKeyEvent(const NativeEventData& aKeyEventData,
                                      bool aIsConsumed);
+
   virtual nsresult OnWindowedPluginKeyEvent(
                      const NativeEventData& aKeyEventData,
                      nsIKeyEventInPluginCallback* aCallback) override;
 
   virtual void LookUpDictionary(
                  const nsAString& aText,
                  const nsTArray<mozilla::FontRange>& aFontRangeArray,
                  const bool aIsVertical,