Bug 1176954 part.3 Don't send selection change, text change nor composition update notification to IME from TabParent until all events sent to the child process is received by it r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Sat, 11 Jul 2015 10:53:55 +0900
changeset 285772 9d0af72de7c0f193eb56ecd5cd5e61a24d5f7346
parent 285771 b226ef6b9ca3e00b97a3b2f97d99dc1dea5d2616
child 285773 890bb7cdc21a8cfe3653a8176f73b1818936ea2e
push id934
push userraliiev@mozilla.com
push dateMon, 26 Oct 2015 12:58:05 +0000
treeherdermozilla-release@05704e35c1d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1176954
milestone42.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 1176954 part.3 Don't send selection change, text change nor composition update notification to IME from TabParent until all events sent to the child process is received by it r=smaug
dom/ipc/TabParent.cpp
widget/ContentCache.cpp
widget/ContentCache.h
widget/nsIWidget.h
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1991,33 +1991,32 @@ TabParent::RecvNotifyIMETextChange(const
 
   IMENotification notification(NOTIFY_IME_OF_TEXT_CHANGE);
   notification.mTextChangeData.mStartOffset = aStart;
   notification.mTextChangeData.mOldEndOffset = aEnd;
   notification.mTextChangeData.mNewEndOffset = aNewEnd;
   notification.mTextChangeData.mCausedByComposition = aCausedByComposition;
 
   mContentCache.AssignContent(aContentCache, &notification);
-  IMEStateManager::NotifyIME(notification, widget, true);
+  mContentCache.MaybeNotifyIME(widget, notification);
   return true;
 }
 
 bool
 TabParent::RecvNotifyIMESelectedCompositionRect(
              const ContentCache& aContentCache)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget) {
     return true;
   }
 
   IMENotification notification(NOTIFY_IME_OF_COMPOSITION_UPDATE);
   mContentCache.AssignContent(aContentCache, &notification);
-
-  IMEStateManager::NotifyIME(notification, widget, true);
+  mContentCache.MaybeNotifyIME(widget, notification);
   return true;
 }
 
 bool
 TabParent::RecvNotifyIMESelection(const ContentCache& aContentCache,
                                   const bool& aCausedByComposition)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
@@ -2027,20 +2026,19 @@ TabParent::RecvNotifyIMESelection(const 
   IMENotification notification(NOTIFY_IME_OF_SELECTION_CHANGE);
   mContentCache.AssignContent(aContentCache, &notification);
 
   const nsIMEUpdatePreference updatePreference =
     widget->GetIMEUpdatePreference();
   if (updatePreference.WantSelectionChange() &&
       (updatePreference.WantChangesCausedByComposition() ||
        !aCausedByComposition)) {
-    mContentCache.InitNotification(notification);
     notification.mSelectionChangeData.mCausedByComposition =
       aCausedByComposition;
-    IMEStateManager::NotifyIME(notification, widget, true);
+    mContentCache.MaybeNotifyIME(widget, notification);
   }
   return true;
 }
 
 bool
 TabParent::RecvUpdateContentCache(const ContentCache& aContentCache)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
@@ -2087,17 +2085,25 @@ TabParent::RecvNotifyIMEPositionChange(c
   return true;
 }
 
 bool
 TabParent::RecvOnEventNeedingAckReceived()
 {
   // This is called when the child process receives WidgetCompositionEvent or
   // WidgetSelectionEvent.
-  mContentCache.OnEventNeedingAckReceived();
+  nsCOMPtr<nsIWidget> widget = GetWidget();
+  if (!widget) {
+    return true;
+  }
+
+  // While calling OnEventNeedingAckReceived(), TabParent *might* be destroyed
+  // since it may send notifications to IME.
+  nsRefPtr<TabParent> kungFuDeathGrip(this);
+  mContentCache.OnEventNeedingAckReceived(widget);
   return true;
 }
 
 bool
 TabParent::RecvRequestFocus(const bool& aCanRaise)
 {
   nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
   if (!fm) {
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -909,25 +909,32 @@ ContentCacheInParent::OnSelectionEvent(
      GetBoolName(aSelectionEvent.mExpandToClusterBoundary),
      GetBoolName(aSelectionEvent.mUseNativeLineBreak), mPendingEventsNeedingAck,
      GetBoolName(mIsComposing)));
 
   mPendingEventsNeedingAck++;
 }
 
 void
-ContentCacheInParent::OnEventNeedingAckReceived()
+ContentCacheInParent::OnEventNeedingAckReceived(nsIWidget* aWidget)
 {
+  // This is called when the child process receives WidgetCompositionEvent or
+  // WidgetSelectionEvent.
+
   MOZ_LOG(sContentCacheLog, LogLevel::Info,
-    ("ContentCacheInParent: 0x%p OnEventNeedingAckReceived(), "
+    ("ContentCacheInParent: 0x%p OnEventNeedingAckReceived(aWidget=0x%p), "
      "mPendingEventsNeedingAck=%u",
-     this, mPendingEventsNeedingAck));
+     this, aWidget, mPendingEventsNeedingAck));
 
   MOZ_RELEASE_ASSERT(mPendingEventsNeedingAck > 0);
-  mPendingEventsNeedingAck--;
+  if (--mPendingEventsNeedingAck) {
+    return;
+  }
+
+  FlushPendingNotifications(aWidget);
 }
 
 uint32_t
 ContentCacheInParent::RequestToCommitComposition(nsIWidget* aWidget,
                                                  bool aCancel,
                                                  nsAString& aLastString)
 {
   MOZ_LOG(sContentCacheLog, LogLevel::Info,
@@ -946,25 +953,93 @@ ContentCacheInParent::RequestToCommitCom
 
   mRequestedToCommitOrCancelComposition = false;
   aLastString = mCommitStringByRequest;
   mCommitStringByRequest.Truncate(0);
   return mCompositionEventsDuringRequest;
 }
 
 void
-ContentCacheInParent::InitNotification(IMENotification& aNotification) const
+ContentCacheInParent::MaybeNotifyIME(nsIWidget* aWidget,
+                                     IMENotification& aNotification)
 {
-  if (NS_WARN_IF(aNotification.mMessage != NOTIFY_IME_OF_SELECTION_CHANGE)) {
+  if (aNotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE) {
+    aNotification.mSelectionChangeData.mOffset = mSelection.StartOffset();
+    aNotification.mSelectionChangeData.mLength = mSelection.Length();
+    aNotification.mSelectionChangeData.mReversed = mSelection.Reversed();
+    aNotification.mSelectionChangeData.SetWritingMode(mSelection.mWritingMode);
+  }
+
+  if (!mPendingEventsNeedingAck) {
+    IMEStateManager::NotifyIME(aNotification, aWidget, true);
     return;
   }
-  aNotification.mSelectionChangeData.mOffset = mSelection.StartOffset();
-  aNotification.mSelectionChangeData.mLength = mSelection.Length();
-  aNotification.mSelectionChangeData.mReversed = mSelection.Reversed();
-  aNotification.mSelectionChangeData.SetWritingMode(mSelection.mWritingMode);
+
+  switch (aNotification.mMessage) {
+    case NOTIFY_IME_OF_SELECTION_CHANGE:
+      mPendingSelectionChange.MergeWith(aNotification);
+      break;
+    case NOTIFY_IME_OF_TEXT_CHANGE:
+      mPendingTextChange.MergeWith(aNotification);
+      break;
+    case NOTIFY_IME_OF_COMPOSITION_UPDATE:
+      mPendingCompositionUpdate.MergeWith(aNotification);
+      break;
+    default:
+      MOZ_CRASH("Unsupported notification");
+      break;
+  }
+}
+
+void
+ContentCacheInParent::FlushPendingNotifications(nsIWidget* aWidget)
+{
+  MOZ_ASSERT(!mPendingEventsNeedingAck);
+
+  // New notifications which are notified during flushing pending notifications
+  // should be merged again.
+  mPendingEventsNeedingAck++;
+
+  nsCOMPtr<nsIWidget> kungFuDeathGrip(aWidget);
+
+  // First, text change notification should be sent because selection change
+  // notification notifies IME of current selection range in the latest content.
+  // So, IME may need the latest content before that.
+  if (mPendingTextChange.HasNotification()) {
+    IMENotification notification(mPendingTextChange);
+    if (!aWidget->Destroyed()) {
+      mPendingTextChange.Clear();
+      IMEStateManager::NotifyIME(notification, aWidget, true);
+    }
+  }
+
+  if (mPendingSelectionChange.HasNotification()) {
+    IMENotification notification(mPendingSelectionChange);
+    if (!aWidget->Destroyed()) {
+      mPendingSelectionChange.Clear();
+      IMEStateManager::NotifyIME(notification, aWidget, true);
+    }
+  }
+
+  // Finally, send composition update notification because it notifies IME of
+  // finishing handling whole sending events.
+  if (mPendingCompositionUpdate.HasNotification()) {
+    IMENotification notification(mPendingCompositionUpdate);
+    if (!aWidget->Destroyed()) {
+      mPendingCompositionUpdate.Clear();
+      IMEStateManager::NotifyIME(notification, aWidget, true);
+    }
+  }
+
+  if (!--mPendingEventsNeedingAck && !aWidget->Destroyed() &&
+      (mPendingTextChange.HasNotification() ||
+       mPendingSelectionChange.HasNotification() ||
+       mPendingCompositionUpdate.HasNotification())) {
+    FlushPendingNotifications(aWidget);
+  }
 }
 
 /*****************************************************************************
  * mozilla::ContentCache::TextRectArray
  *****************************************************************************/
 
 LayoutDeviceIntRect
 ContentCache::TextRectArray::GetRect(uint32_t aOffset) const
--- a/widget/ContentCache.h
+++ b/widget/ContentCache.h
@@ -9,28 +9,23 @@
 #define mozilla_ContentCache_h
 
 #include <stdint.h>
 
 #include "mozilla/Assertions.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/EventForwards.h"
 #include "mozilla/WritingModes.h"
+#include "nsIWidget.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "Units.h"
 
-class nsIWidget;
-
 namespace mozilla {
 
-namespace widget {
-struct IMENotification;
-}
-
 class ContentCacheInParent;
 
 /**
  * ContentCache stores various information of the child content.
  * This class has members which are necessary both in parent process and
  * content process.
  */
 
@@ -317,18 +312,22 @@ public:
   /**
    * OnSelectionEvent() should be called before sending selection event.
    */
   void OnSelectionEvent(const WidgetSelectionEvent& aSelectionEvent);
 
   /**
    * OnEventNeedingAckReceived() should be called when the child process
    * receives a sent event which needs acknowledging.
+   *
+   * WARNING: This may send notifications to IME.  That might cause destroying
+   *          TabParent or aWidget.  Therefore, the caller must not destroy
+   *          this instance during a call of this method.
    */
-  void OnEventNeedingAckReceived();
+  void OnEventNeedingAckReceived(nsIWidget* aWidget);
 
   /**
    * RequestToCommitComposition() requests to commit or cancel composition to
    * the widget.  If it's handled synchronously, this returns the number of
    * composition events after that.
    *
    * @param aWidget     The widget to be requested to commit or cancel
    *                    the composition.
@@ -339,23 +338,28 @@ public:
    * @return            The count of composition events ignored after a call of
    *                    WillRequestToCommitOrCancelComposition().
    */
   uint32_t RequestToCommitComposition(nsIWidget* aWidget,
                                       bool aCancel,
                                       nsAString& aLastString);
 
   /**
-   * InitNotification() initializes aNotification with stored data.
-   *
-   * @param aNotification       Must be NOTIFY_IME_OF_SELECTION_CHANGE.
+   * MaybeNotifyIME() may notify IME of the notification.  If child process
+   * hasn't been handled all sending events yet, this stores the notification
+   * and flush it later.
    */
-  void InitNotification(IMENotification& aNotification) const;
+  void MaybeNotifyIME(nsIWidget* aWidget,
+                      IMENotification& aNotification);
 
 private:
+  IMENotification mPendingSelectionChange;
+  IMENotification mPendingTextChange;
+  IMENotification mPendingCompositionUpdate;
+
   // This is commit string which is caused by our request.
   nsString mCommitStringByRequest;
   // Start offset of the composition string.
   uint32_t mCompositionStart;
   // Count of composition events during requesting commit or cancel the
   // composition.
   uint32_t mCompositionEventsDuringRequest;
   // mPendingEventsNeedingAck is increased before sending a composition event or
@@ -368,13 +372,14 @@ private:
 
   bool GetCaretRect(uint32_t aOffset, LayoutDeviceIntRect& aCaretRect) const;
   bool GetTextRect(uint32_t aOffset,
                    LayoutDeviceIntRect& aTextRect) const;
   bool GetUnionTextRects(uint32_t aOffset,
                          uint32_t aLength,
                          LayoutDeviceIntRect& aUnionTextRect) const;
 
+  void FlushPendingNotifications(nsIWidget* aWidget);
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ContentCache_h
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@@ -568,18 +568,21 @@ struct SizeConstraints {
 };
 
 // IMEMessage is shared by IMEStateManager and TextComposition.
 // Update values in GeckoEditable.java if you make changes here.
 // XXX Negative values are used in Android...
 typedef int8_t IMEMessageType;
 enum IMEMessage : IMEMessageType
 {
+  // This is used by IMENotification internally.  This means that the instance
+  // hasn't been initialized yet.
+  NOTIFY_IME_OF_NOTHING,
   // An editable content is getting focus
-  NOTIFY_IME_OF_FOCUS = 1,
+  NOTIFY_IME_OF_FOCUS,
   // An editable content is losing focus
   NOTIFY_IME_OF_BLUR,
   // Selection in the focused editable content is changed
   NOTIFY_IME_OF_SELECTION_CHANGE,
   // Text in the focused editable content is changed
   NOTIFY_IME_OF_TEXT_CHANGE,
   // Composition string has been updated
   NOTIFY_IME_OF_COMPOSITION_UPDATE,
@@ -593,17 +596,17 @@ enum IMEMessage : IMEMessageType
   // Request to cancel current composition to IME
   // (some platforms may not support)
   REQUEST_TO_CANCEL_COMPOSITION
 };
 
 struct IMENotification
 {
   IMENotification()
-    : mMessage(static_cast<IMEMessage>(-1))
+    : mMessage(NOTIFY_IME_OF_NOTHING)
   {}
 
   MOZ_IMPLICIT IMENotification(IMEMessage aMessage)
     : mMessage(aMessage)
   {
     switch (aMessage) {
       case NOTIFY_IME_OF_SELECTION_CHANGE:
         mSelectionChangeData.mOffset = UINT32_MAX;
@@ -626,16 +629,69 @@ struct IMENotification
         mMouseButtonEventData.mButton = -1;
         mMouseButtonEventData.mButtons = 0;
         mMouseButtonEventData.mModifiers = 0;
       default:
         break;
     }
   }
 
+  void Clear()
+  {
+    mMessage = NOTIFY_IME_OF_NOTHING;
+  }
+
+  bool HasNotification() const
+  {
+    return mMessage != NOTIFY_IME_OF_NOTHING;
+  }
+
+  void MergeWith(const IMENotification& aNotification)
+  {
+    switch (mMessage) {
+      case NOTIFY_IME_OF_NOTHING:
+        MOZ_ASSERT(aNotification.mMessage != NOTIFY_IME_OF_NOTHING);
+        *this = aNotification;
+        break;
+      case NOTIFY_IME_OF_SELECTION_CHANGE:
+        MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE);
+        mSelectionChangeData.mOffset =
+          aNotification.mSelectionChangeData.mOffset;
+        mSelectionChangeData.mLength =
+          aNotification.mSelectionChangeData.mLength;
+        mSelectionChangeData.mWritingMode =
+          aNotification.mSelectionChangeData.mWritingMode;
+        mSelectionChangeData.mReversed =
+          aNotification.mSelectionChangeData.mReversed;
+        mSelectionChangeData.mCausedByComposition =
+          mSelectionChangeData.mCausedByComposition &&
+            aNotification.mSelectionChangeData.mCausedByComposition;
+        break;
+      case NOTIFY_IME_OF_TEXT_CHANGE:
+        MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_TEXT_CHANGE);
+        // TODO: Needs to merge the ranges rather than overwriting.
+        mTextChangeData.mStartOffset =
+          aNotification.mTextChangeData.mStartOffset;
+        mTextChangeData.mOldEndOffset =
+          aNotification.mTextChangeData.mOldEndOffset;
+        mTextChangeData.mNewEndOffset =
+          aNotification.mTextChangeData.mNewEndOffset;
+        mTextChangeData.mCausedByComposition =
+          mTextChangeData.mCausedByComposition &&
+            aNotification.mTextChangeData.mCausedByComposition;
+        break;
+      case NOTIFY_IME_OF_COMPOSITION_UPDATE:
+        MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_COMPOSITION_UPDATE);
+        break;
+      default:
+        MOZ_CRASH("Merging notification isn't supported");
+        break;
+    }
+  }
+
   IMEMessage mMessage;
 
   // NOTIFY_IME_OF_SELECTION_CHANGE specific data
   struct SelectionChangeData
   {
     // Selection range.
     uint32_t mOffset;
     uint32_t mLength;