Bug 1349255 - Part 1: Make PBrowser.NotifyIMEFocus async to avoid UI jank. r?masayuki,kanru draft
authorHenry Chang <hchang@mozilla.com>
Mon, 08 May 2017 19:07:56 +0800
changeset 652943 216f936a8de33a9b1f30717e3c8ce06279f7c7fa
parent 648573 a6a1f5c1d971dbee67ba6eec7ead7902351ddca2
child 652944 54fdbf5e1127b386f5b94f72aeab907b842be449
push id76223
push userbmo:sawang@mozilla.com
push dateFri, 25 Aug 2017 13:56:34 +0000
reviewersmasayuki, kanru
bugs1349255, 1217700
milestone57.0a1
Bug 1349255 - Part 1: Make PBrowser.NotifyIMEFocus async to avoid UI jank. r?masayuki,kanru 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
@@ -441,17 +441,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
@@ -1763,115 +1763,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,