Bug 1166436 part.10 Optimize IME notification handling in PuppetWidget r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 05 Jun 2015 18:28:20 +0900
changeset 247362 d813014e797c44abbf39b10e11dbbb632ef1ae27
parent 247361 f30faf72451ee9afbd08bae7b19824b80ff5f76b
child 247363 c8afeac76c0e0a82e709d9ef0672a37396b74341
push id28864
push userkwierso@gmail.com
push dateFri, 05 Jun 2015 21:49:37 +0000
treeherdermozilla-central@97a39c939c51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1166436
milestone41.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 1166436 part.10 Optimize IME notification handling in PuppetWidget r=m_kato
dom/events/IMEContentObserver.cpp
dom/ipc/PBrowser.ipdl
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
widget/ContentCache.cpp
widget/ContentCache.h
widget/PuppetWidget.cpp
widget/PuppetWidget.h
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -1174,16 +1174,19 @@ IMEContentObserver::FlushMergeableNotifi
 
   // NOTE: Reset each pending flag because sending notification may cause
   //       another change.
 
   if (mTextChangeData.mStored) {
     nsContentUtils::AddScriptRunner(new TextChangeEvent(this, mTextChangeData));
   }
 
+  // Be aware, PuppetWidget depends on the order of this. A selection change
+  // notification should not be sent before a text change notification because
+  // PuppetWidget shouldn't query new text content every selection change.
   if (mIsSelectionChangeEventPending) {
     mIsSelectionChangeEventPending = false;
     nsContentUtils::AddScriptRunner(
       new SelectionChangeEvent(this, mSelectionChangeCausedOnlyByComposition));
   }
 
   if (mIsPositionChangeEventPending) {
     mIsPositionChangeEventPending = false;
--- a/dom/ipc/PBrowser.ipdl
+++ b/dom/ipc/PBrowser.ipdl
@@ -210,38 +210,32 @@ parent:
      *
      *  contentCache Cache of content
      *  causedByComposition true if the change is caused by composition
      */
     prio(urgent) async NotifyIMESelection(ContentCache contentCache,
                                           bool causedByComposition);
 
     /**
-     * Notifies chrome to refresh its text cache 
+     * Notifies chrome of updating its content cache.
+     * This is useful if content is modified but we don't need to notify IME.
      *
      *  contentCache Cache of content
      */
-    prio(urgent) async NotifyIMETextHint(ContentCache contentCache);
+    prio(urgent) 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.
      */
     prio(urgent) sync NotifyIMEMouseButtonEvent(IMENotification notification)
       returns (bool consumedByIME);
 
     /**
-     * Notifies chrome to currect editor rect
-     *
-     *  contentCache Cache of content
-     */
-    prio(urgent) async NotifyIMEEditorRect(ContentCache contentCache);
-
-    /**
      * Notifies chrome to position change
      *
      *  contentCache Cache of content
      */
     prio(urgent) async NotifyIMEPositionChange(ContentCache contentCache);
 
     /**
      * Instructs chrome to end any pending composition
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1972,18 +1972,23 @@ TabParent::RecvNotifyIMESelection(const 
     notification.mSelectionChangeData.mCausedByComposition =
       aCausedByComposition;
     widget->NotifyIME(notification);
   }
   return true;
 }
 
 bool
-TabParent::RecvNotifyIMETextHint(const ContentCache& aContentCache)
+TabParent::RecvUpdateContentCache(const ContentCache& aContentCache)
 {
+  nsCOMPtr<nsIWidget> widget = GetWidget();
+  if (!widget) {
+    return true;
+  }
+
   mContentCache.AssignContent(aContentCache);
   return true;
 }
 
 bool
 TabParent::RecvNotifyIMEMouseButtonEvent(
              const IMENotification& aIMENotification,
              bool* aConsumedByIME)
@@ -1995,23 +2000,16 @@ TabParent::RecvNotifyIMEMouseButtonEvent
     return true;
   }
   nsresult rv = widget->NotifyIME(aIMENotification);
   *aConsumedByIME = rv == NS_SUCCESS_EVENT_CONSUMED;
   return true;
 }
 
 bool
-TabParent::RecvNotifyIMEEditorRect(const ContentCache& aContentCache)
-{
-  mContentCache.AssignContent(aContentCache);
-  return true;
-}
-
-bool
 TabParent::RecvNotifyIMEPositionChange(const ContentCache& aContentCache)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget) {
     return true;
   }
 
   mContentCache.AssignContent(aContentCache);
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -167,20 +167,19 @@ public:
     virtual bool RecvNotifyIMETextChange(const ContentCache& aContentCache,
                                          const uint32_t& aStart,
                                          const uint32_t& aEnd,
                                          const uint32_t& aNewEnd,
                                          const bool& aCausedByComposition) override;
     virtual bool RecvNotifyIMESelectedCompositionRect(const ContentCache& aContentCache) override;
     virtual bool RecvNotifyIMESelection(const ContentCache& aContentCache,
                                         const bool& aCausedByComposition) override;
-    virtual bool RecvNotifyIMETextHint(const ContentCache& aContentCache) override;
+    virtual bool RecvUpdateContentCache(const ContentCache& aContentCache) override;
     virtual bool RecvNotifyIMEMouseButtonEvent(const widget::IMENotification& aEventMessage,
                                                bool* aConsumedByIME) override;
-    virtual bool RecvNotifyIMEEditorRect(const ContentCache& aContentCache) override;
     virtual bool RecvNotifyIMEPositionChange(const ContentCache& aContentCache) override;
     virtual bool RecvEndIMEComposition(const bool& aCancel,
                                        bool* aNoCompositionEvent,
                                        nsString* aComposition) override;
     virtual bool RecvStartPluginIME(const WidgetKeyboardEvent& aKeyboardEvent,
                                     const int32_t& aPanelX,
                                     const int32_t& aPanelY,
                                     nsString* aCommitted) override;
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -20,23 +20,28 @@ using namespace widget;
  * mozilla::ContentCache
  *****************************************************************************/
 
 ContentCache::ContentCache()
   : mCompositionStart(UINT32_MAX)
   , mCompositionEventsDuringRequest(0)
   , mIsComposing(false)
   , mRequestedToCommitOrCancelComposition(false)
+  , mIsChrome(XRE_GetProcessType() == GeckoProcessType_Default)
 {
 }
 
 void
 ContentCache::Clear()
 {
   mText.Truncate();
+  mSelection.Clear();
+  mCaret.Clear();
+  mTextRectArray.Clear();
+  mEditorRect.SetEmpty();
 }
 
 bool
 ContentCache::HandleQueryContentEvent(WidgetQueryContentEvent& aEvent,
                                       nsIWidget* aWidget) const
 {
   MOZ_ASSERT(aWidget);
 
@@ -106,28 +111,38 @@ ContentCache::HandleQueryContentEvent(Wi
   }
   aEvent.mSucceeded = true;
   return true;
 }
 
 bool
 ContentCache::CacheAll(nsIWidget* aWidget)
 {
+  // CacheAll() must be called in content process.
+  if (NS_WARN_IF(mIsChrome)) {
+    return false;
+  }
+
   if (NS_WARN_IF(!CacheText(aWidget)) ||
       NS_WARN_IF(!CacheSelection(aWidget)) ||
       NS_WARN_IF(!CacheTextRects(aWidget)) ||
       NS_WARN_IF(!CacheEditorRect(aWidget))) {
     return false;
   }
   return true;
 }
 
 bool
 ContentCache::CacheSelection(nsIWidget* aWidget)
 {
+  // CacheSelection() must be called in content process.
+  if (NS_WARN_IF(mIsChrome)) {
+    return false;
+  }
+
   mCaret.Clear();
   mSelection.Clear();
 
   nsEventStatus status = nsEventStatus_eIgnore;
   WidgetQueryContentEvent selection(true, NS_QUERY_SELECTED_TEXT, aWidget);
   aWidget->DispatchEvent(&selection, status);
   if (NS_WARN_IF(!selection.mSucceeded)) {
     return false;
@@ -161,44 +176,59 @@ ContentCache::CacheSelection(nsIWidget* 
   }
   mCaret.mRect = caretRect.mReply.mRect;
   return true;
 }
 
 bool
 ContentCache::CacheEditorRect(nsIWidget* aWidget)
 {
+  // CacheEditorRect() must be called in content process.
+  if (NS_WARN_IF(mIsChrome)) {
+    return false;
+  }
+
   nsEventStatus status = nsEventStatus_eIgnore;
   WidgetQueryContentEvent editorRectEvent(true, NS_QUERY_EDITOR_RECT, aWidget);
   aWidget->DispatchEvent(&editorRectEvent, status);
   if (NS_WARN_IF(!editorRectEvent.mSucceeded)) {
     return false;
   }
   mEditorRect = editorRectEvent.mReply.mRect;
   return true;
 }
 
 bool
 ContentCache::CacheText(nsIWidget* aWidget)
 {
+  // CacheText() must be called in content process.
+  if (NS_WARN_IF(mIsChrome)) {
+    return false;
+  }
+
   nsEventStatus status = nsEventStatus_eIgnore;
   WidgetQueryContentEvent queryText(true, NS_QUERY_TEXT_CONTENT, aWidget);
   queryText.InitForQueryTextContent(0, UINT32_MAX);
   aWidget->DispatchEvent(&queryText, status);
   if (NS_WARN_IF(!queryText.mSucceeded)) {
     SetText(EmptyString());
     return false;
   }
   SetText(queryText.mReply.mString);
   return true;
 }
 
 bool
 ContentCache::CacheTextRects(nsIWidget* aWidget)
 {
+  // CacheTextRects() must be called in content process.
+  if (NS_WARN_IF(mIsChrome)) {
+    return false;
+  }
+
   mTextRectArray.Clear();
 
   nsRefPtr<TextComposition> textComposition =
     IMEStateManager::GetTextCompositionFor(aWidget);
   if (NS_WARN_IF(!textComposition)) {
     return true;
   }
 
--- a/widget/ContentCache.h
+++ b/widget/ContentCache.h
@@ -273,15 +273,16 @@ private:
     LayoutDeviceIntRect GetRect(uint32_t aOffset) const;
     LayoutDeviceIntRect GetUnionRect(uint32_t aOffset, uint32_t aLength) const;
   } mTextRectArray;
 
   LayoutDeviceIntRect mEditorRect;
 
   bool mIsComposing;
   bool mRequestedToCommitOrCancelComposition;
+  bool mIsChrome;
 
   friend struct IPC::ParamTraits<ContentCache>;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ContentCache_h
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -669,37 +669,30 @@ PuppetWidget::NotifyIMEOfFocusChange(boo
 #ifndef MOZ_CROSS_PROCESS_IME
   return NS_OK;
 #endif
 
   if (!mTabChild)
     return NS_ERROR_FAILURE;
 
   if (aFocus) {
-    if (NS_WARN_IF(!mContentCache.CacheText(this))) {
+    // When IME gets focus, we should initalize all information of the content.
+    if (NS_WARN_IF(!mContentCache.CacheAll(this))) {
       return NS_ERROR_FAILURE;
     }
-    mTabChild->SendNotifyIMETextHint(mContentCache);
   } else {
+    // When IME loses focus, we don't need to store anything.
     mContentCache.Clear();
   }
 
   mIMEPreferenceOfParent = nsIMEUpdatePreference();
   if (!mTabChild->SendNotifyIMEFocus(aFocus, mContentCache,
                                      &mIMEPreferenceOfParent)) {
     return NS_ERROR_FAILURE;
   }
-
-  // TODO: Optimize this later.
-  if (aFocus) {
-    IMENotification notification(NOTIFY_IME_OF_SELECTION_CHANGE);
-    notification.mSelectionChangeData.mCausedByComposition = false;
-    NotifyIMEOfSelectionChange(notification); // Update selection
-    NotifyIMEOfEditorRect();
-  }
   return NS_OK;
 }
 
 nsresult
 PuppetWidget::NotifyIMEOfUpdateComposition()
 {
 #ifndef MOZ_CROSS_PROCESS_IME
   return NS_OK;
@@ -710,33 +703,16 @@ PuppetWidget::NotifyIMEOfUpdateCompositi
   if (NS_WARN_IF(!mContentCache.CacheTextRects(this)) ||
       NS_WARN_IF(!mContentCache.CacheSelection(this))) {
     return NS_ERROR_FAILURE;
   }
   mTabChild->SendNotifyIMESelectedCompositionRect(mContentCache);
   return NS_OK;
 }
 
-nsresult
-PuppetWidget::NotifyIMEOfEditorRect()
-{
-#ifndef MOZ_CROSS_PROCESS_IME
-  return NS_OK;
-#endif
-  if (NS_WARN_IF(!mTabChild)) {
-    return NS_ERROR_FAILURE;
-  }
-
-  if (NS_WARN_IF(!mContentCache.CacheEditorRect(this))) {
-    return NS_ERROR_FAILURE;
-  }
-  mTabChild->SendNotifyIMEEditorRect(mContentCache);
-  return NS_OK;
-}
-
 nsIMEUpdatePreference
 PuppetWidget::GetIMEUpdatePreference()
 {
 #ifdef MOZ_CROSS_PROCESS_IME
   // e10s requires IME information cache into TabParent
   return nsIMEUpdatePreference(mIMEPreferenceOfParent.mWantUpdates |
                                nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE |
                                nsIMEUpdatePreference::NOTIFY_TEXT_CHANGE |
@@ -755,32 +731,40 @@ PuppetWidget::NotifyIMEOfTextChange(cons
 
 #ifndef MOZ_CROSS_PROCESS_IME
   return NS_OK;
 #endif
 
   if (!mTabChild)
     return NS_ERROR_FAILURE;
 
-  if (NS_WARN_IF(!mContentCache.CacheText(this))) {
+  // FYI: text change notification is the first notification after
+  //      a user operation changes the content.  So, we need to modify
+  //      the cache as far as possible here.
+
+  // When text is changed, selection and text rects must be changed too.
+  if (NS_WARN_IF(!mContentCache.CacheText(this)) ||
+      NS_WARN_IF(!mContentCache.CacheTextRects(this)) ||
+      NS_WARN_IF(!mContentCache.CacheSelection(this))) {
     return NS_ERROR_FAILURE;
   }
-  mTabChild->SendNotifyIMETextHint(mContentCache);
 
   // TabParent doesn't this this to cache.  we don't send the notification
   // if parent process doesn't request NOTIFY_TEXT_CHANGE.
   if (mIMEPreferenceOfParent.WantTextChange() &&
       (mIMEPreferenceOfParent.WantChangesCausedByComposition() ||
        !aIMENotification.mTextChangeData.mCausedByComposition)) {
     mTabChild->SendNotifyIMETextChange(
       mContentCache,
       aIMENotification.mTextChangeData.mStartOffset,
       aIMENotification.mTextChangeData.mOldEndOffset,
       aIMENotification.mTextChangeData.mNewEndOffset,
       aIMENotification.mTextChangeData.mCausedByComposition);
+  } else {
+    mTabChild->SendUpdateContentCache(mContentCache);
   }
   return NS_OK;
 }
 
 nsresult
 PuppetWidget::NotifyIMEOfSelectionChange(
                 const IMENotification& aIMENotification)
 {
@@ -789,16 +773,18 @@ PuppetWidget::NotifyIMEOfSelectionChange
 
 #ifndef MOZ_CROSS_PROCESS_IME
   return NS_OK;
 #endif
 
   if (!mTabChild)
     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(
     aIMENotification.mSelectionChangeData.mOffset,
     aIMENotification.mSelectionChangeData.mLength,
     aIMENotification.mSelectionChangeData.mReversed,
     aIMENotification.mSelectionChangeData.GetWritingMode());
 
   mTabChild->SendNotifyIMESelection(
     mContentCache, aIMENotification.mSelectionChangeData.mCausedByComposition);
--- a/widget/PuppetWidget.h
+++ b/widget/PuppetWidget.h
@@ -257,17 +257,16 @@ private:
   void SetChild(PuppetWidget* aChild);
 
   nsresult IMEEndComposition(bool aCancel);
   nsresult NotifyIMEOfFocusChange(bool aFocus);
   nsresult NotifyIMEOfSelectionChange(const IMENotification& aIMENotification);
   nsresult NotifyIMEOfUpdateComposition();
   nsresult NotifyIMEOfTextChange(const IMENotification& aIMENotification);
   nsresult NotifyIMEOfMouseButtonEvent(const IMENotification& aIMENotification);
-  nsresult NotifyIMEOfEditorRect();
   nsresult NotifyIMEOfPositionChange();
 
   bool CacheEditorRect();
   bool CacheCompositionRects(uint32_t& aStartOffset,
                              nsTArray<mozilla::LayoutDeviceIntRect>& aRectArray,
                              uint32_t& aTargetCauseOffset);
   bool GetCaretRect(mozilla::LayoutDeviceIntRect& aCaretRect, uint32_t aCaretOffset);
   uint32_t GetCaretOffset();