Bug 1172466 part.4 Don't notify IME during reflow r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 17 Jun 2015 10:03:57 +0900
changeset 280067 a633cd8ac3d9eb7ff9f7ebbf2993c123d96cfff9
parent 280066 6e481878f2b00025b998b9752a5714e4ccf996c0
child 280068 5e35c779d79193d52fd20d5d73b7ff9a05642ae4
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1172466
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 1172466 part.4 Don't notify IME during reflow r=smaug
dom/events/IMEContentObserver.cpp
dom/events/IMEContentObserver.h
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -196,16 +196,20 @@ IMEContentObserver::Init(nsIWidget* aWid
     if (!mRootContent) {
       return;
     }
   }
 
   mDocShell = aPresContext->GetDocShell();
 
   ObserveEditableNode();
+
+  // Some change events may wait to notify IME because this was being
+  // initialized.  It is the time to flush them.
+  FlushMergeableNotifications();
 }
 
 void
 IMEContentObserver::ObserveEditableNode()
 {
   MOZ_RELEASE_ASSERT(mEditor);
   MOZ_RELEASE_ASSERT(mSelection);
   MOZ_RELEASE_ASSERT(mRootContent);
@@ -989,69 +993,103 @@ IMEContentObserver::CancelEditAction()
 {
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
   FlushMergeableNotifications();
   return NS_OK;
 }
 
 void
-IMEContentObserver::MaybeNotifyIMEOfFocusSet()
+IMEContentObserver::PostFocusSetNotification()
 {
   mIsFocusEventPending = true;
-  FlushMergeableNotifications();
 }
 
 void
-IMEContentObserver::MaybeNotifyIMEOfTextChange(const TextChangeData& aData)
+IMEContentObserver::PostTextChangeNotification(const TextChangeData& aData)
 {
   StoreTextChangeData(aData);
   MOZ_ASSERT(mTextChangeData.mStored,
              "mTextChangeData must have text change data");
-  FlushMergeableNotifications();
 }
 
 void
-IMEContentObserver::MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition)
+IMEContentObserver::PostSelectionChangeNotification(bool aCausedByComposition)
 {
   if (!mIsSelectionChangeEventPending) {
     mSelectionChangeCausedOnlyByComposition = aCausedByComposition;
   } else {
     mSelectionChangeCausedOnlyByComposition =
       mSelectionChangeCausedOnlyByComposition && aCausedByComposition;
   }
   mIsSelectionChangeEventPending = true;
-  FlushMergeableNotifications();
 }
 
 void
-IMEContentObserver::MaybeNotifyIMEOfPositionChange()
+IMEContentObserver::PostPositionChangeNotification()
 {
   mIsPositionChangeEventPending = true;
-  FlushMergeableNotifications();
 }
 
-void
-IMEContentObserver::FlushMergeableNotifications()
+bool
+IMEContentObserver::IsReflowLocked() const
+{
+  nsPresContext* presContext = GetPresContext();
+  if (NS_WARN_IF(!presContext)) {
+    return false;
+  }
+  nsIPresShell* presShell = presContext->GetPresShell();
+  if (NS_WARN_IF(!presShell)) {
+    return false;
+  }
+  // During reflow, we shouldn't notify IME because IME may query content
+  // synchronously.  Then, it causes ContentEventHandler will try to flush
+  // pending notifications during reflow.
+  return presShell->IsReflowLocked();
+}
+
+bool
+IMEContentObserver::IsSafeToNotifyIME() const
 {
   // If this is already detached from the widget, this doesn't need to notify
   // anything.
   if (!mWidget) {
-    return;
+    return false;
   }
 
   // Don't notify IME of anything if it's not good time to do it.
   if (mSuppressNotifications) {
-    return;
+    return false;
+  }
+
+  if (!mESM || NS_WARN_IF(!GetPresContext())) {
+    return false;
+  }
+
+  // If it's in reflow, we should wait to finish the reflow.
+  // FYI: This should be called again from Reflow() or ReflowInterruptible().
+  if (IsReflowLocked()) {
+    return false;
   }
 
   // If we're in handling an edit action, this method will be called later.
   bool isInEditAction = false;
   if (mEditor && NS_SUCCEEDED(mEditor->GetIsInEditAction(&isInEditAction)) &&
       isInEditAction) {
+    return false;
+  }
+
+  return true;
+}
+
+void
+IMEContentObserver::FlushMergeableNotifications()
+{
+  if (!IsSafeToNotifyIME()) {
+    // So, if this is already called, this should do nothing.
     return;
   }
 
   // Notifying something may cause nested call of this method.  For example,
   // when somebody notified one of the notifications may dispatch query content
   // event. Then, it causes flushing layout which may cause another layout
   // change notification.
 
@@ -1579,56 +1617,78 @@ IMEContentObserver::AChangeEvent::CanNot
   }
 
   // If IME has focus, IMEContentObserver must hold the widget.
   MOZ_ASSERT(mIMEContentObserver->mWidget);
 
   return true;
 }
 
+bool
+IMEContentObserver::AChangeEvent::IsSafeToNotifyIME() const
+{
+  if (NS_WARN_IF(!nsContentUtils::IsSafeToRunScript())) {
+    return false;
+  }
+  State state = mIMEContentObserver->GetState();
+  if (mChangeEventType == eChangeEventType_Focus) {
+    if (NS_WARN_IF(state != eState_Initializing && state != eState_Observing)) {
+      return false;
+    }
+  } else if (state != eState_Observing) {
+    return false;
+  }
+  return mIMEContentObserver->IsSafeToNotifyIME();
+}
+
 /******************************************************************************
  * mozilla::IMEContentObserver::FocusSetEvent
  ******************************************************************************/
 
 NS_IMETHODIMP
 IMEContentObserver::FocusSetEvent::Run()
 {
   if (!CanNotifyIME()) {
     // If IMEContentObserver has already gone, we don't need to notify IME of
     // focus.
     mIMEContentObserver->ClearPendingNotifications();
     return NS_OK;
   }
 
+  if (!IsSafeToNotifyIME()) {
+    mIMEContentObserver->PostFocusSetNotification();
+    return NS_OK;
+  }
+
   mIMEContentObserver->mIMEHasFocus = true;
   mIMEContentObserver->mWidget->NotifyIME(IMENotification(NOTIFY_IME_OF_FOCUS));
   return NS_OK;
 }
 
 /******************************************************************************
  * mozilla::IMEContentObserver::SelectionChangeEvent
  ******************************************************************************/
 
 NS_IMETHODIMP
 IMEContentObserver::SelectionChangeEvent::Run()
 {
   if (!CanNotifyIME()) {
     return NS_OK;
   }
 
-  nsPresContext* presContext = mIMEContentObserver->GetPresContext();
-  if (!presContext) {
+  if (!IsSafeToNotifyIME()) {
+    mIMEContentObserver->PostSelectionChangeNotification(mCausedByComposition);
     return NS_OK;
   }
 
   // XXX Cannot we cache some information for reducing the cost to compute
   //     selection offset and writing mode?
   WidgetQueryContentEvent selection(true, NS_QUERY_SELECTED_TEXT,
                                     mIMEContentObserver->mWidget);
-  ContentEventHandler handler(presContext);
+  ContentEventHandler handler(mIMEContentObserver->GetPresContext());
   handler.OnQuerySelectedText(&selection);
   if (NS_WARN_IF(!selection.mSucceeded)) {
     return NS_OK;
   }
 
   // The state may be changed since querying content causes flushing layout.
   if (!CanNotifyIME()) {
     return NS_OK;
@@ -1654,16 +1714,21 @@ IMEContentObserver::SelectionChangeEvent
 
 NS_IMETHODIMP
 IMEContentObserver::TextChangeEvent::Run()
 {
   if (!CanNotifyIME()) {
     return NS_OK;
   }
 
+  if (!IsSafeToNotifyIME()) {
+    mIMEContentObserver->PostTextChangeNotification(mData);
+    return NS_OK;
+  }
+
   IMENotification notification(NOTIFY_IME_OF_TEXT_CHANGE);
   notification.mTextChangeData.mStartOffset = mData.mStartOffset;
   notification.mTextChangeData.mOldEndOffset = mData.mRemovedEndOffset;
   notification.mTextChangeData.mNewEndOffset = mData.mAddedEndOffset;
   notification.mTextChangeData.mCausedByComposition =
     mData.mCausedOnlyByComposition;
   mIMEContentObserver->mWidget->NotifyIME(notification);
   return NS_OK;
@@ -1675,16 +1740,21 @@ IMEContentObserver::TextChangeEvent::Run
 
 NS_IMETHODIMP
 IMEContentObserver::PositionChangeEvent::Run()
 {
   if (!CanNotifyIME()) {
     return NS_OK;
   }
 
+  if (!IsSafeToNotifyIME()) {
+    mIMEContentObserver->PostPositionChangeNotification();
+    return NS_OK;
+  }
+
   mIMEContentObserver->mWidget->NotifyIME(
     IMENotification(NOTIFY_IME_OF_POSITION_CHANGE));
   return NS_OK;
 }
 
 /******************************************************************************
  * mozilla::IMEContentObserver::AsyncMergeableNotificationsFlusher
  ******************************************************************************/
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -156,20 +156,43 @@ private:
     eState_NotObserving,
     eState_Initializing,
     eState_StoppedObserving,
     eState_Observing
   };
   State GetState() const;
   bool IsObservingContent(nsPresContext* aPresContext,
                           nsIContent* aContent) const;
-  void MaybeNotifyIMEOfFocusSet();
-  void MaybeNotifyIMEOfTextChange(const TextChangeData& aTextChangeData);
-  void MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition);
-  void MaybeNotifyIMEOfPositionChange();
+  bool IsReflowLocked() const;
+  bool IsSafeToNotifyIME() const;
+
+  void PostFocusSetNotification();
+  void MaybeNotifyIMEOfFocusSet()
+  {
+    PostFocusSetNotification();
+    FlushMergeableNotifications();
+  }
+  void PostTextChangeNotification(const TextChangeData& aTextChangeData);
+  void MaybeNotifyIMEOfTextChange(const TextChangeData& aTextChangeData)
+  {
+    PostTextChangeNotification(aTextChangeData);
+    FlushMergeableNotifications();
+  }
+  void PostSelectionChangeNotification(bool aCausedByComposition);
+  void MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition)
+  {
+    PostSelectionChangeNotification(aCausedByComposition);
+    FlushMergeableNotifications();
+  }
+  void PostPositionChangeNotification();
+  void MaybeNotifyIMEOfPositionChange()
+  {
+    PostPositionChangeNotification();
+    FlushMergeableNotifications();
+  }
 
   void NotifyContentAdded(nsINode* aContainer, int32_t aStart, int32_t aEnd);
   void ObserveEditableNode();
   /**
    *  NotifyIMEOfBlur() notifies IME of blur.
    */
   void NotifyIMEOfBlur();
   /**
@@ -295,16 +318,21 @@ private:
 
     nsRefPtr<IMEContentObserver> mIMEContentObserver;
     ChangeEventType mChangeEventType;
 
     /**
      * CanNotifyIME() checks if mIMEContentObserver can and should notify IME.
      */
     bool CanNotifyIME() const;
+
+    /**
+     * IsSafeToNotifyIME() checks if it's safe to noitify IME.
+     */
+    bool IsSafeToNotifyIME() const;
   };
 
   class FocusSetEvent: public AChangeEvent
   {
   public:
     explicit FocusSetEvent(IMEContentObserver* aIMEContentObserver)
       : AChangeEvent(eChangeEventType_Focus, aIMEContentObserver)
     {