Bug 1203381 part.1 IMEContentObserver shouldn't clear mTextChangeData until immediately before sending a text change notification r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 16 Sep 2015 17:48:23 +0900
changeset 295370 bcfee08450fc95eceb46f2fbf60bec97d4c08b50
parent 295369 dab5c069d6d3428445d448b597aac238600cfc6d
child 295371 6b69a6cadbe1fca58a29edbf7db28975433a2abc
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1203381
milestone43.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 1203381 part.1 IMEContentObserver shouldn't clear mTextChangeData until immediately before sending a text change notification r=smaug
dom/events/IMEContentObserver.cpp
dom/events/IMEContentObserver.h
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -190,16 +190,17 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(IMECont
 
 IMEContentObserver::IMEContentObserver()
   : mESM(nullptr)
   , mSuppressNotifications(0)
   , mPreCharacterDataChangeLength(-1)
   , mIsObserving(false)
   , mIMEHasFocus(false)
   , mIsFocusEventPending(false)
+  , mIsTextChangeEventPending(false)
   , mIsSelectionChangeEventPending(false)
   , mIsPositionChangeEventPending(false)
   , mIsFlushingPendingNotifications(false)
 {
 #ifdef DEBUG
   mTextChangeData.Test();
 #endif
   if (!sIMECOLog) {
@@ -998,32 +999,26 @@ IMEContentObserver::PostFocusSetNotifica
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::PostFocusSetNotification()", this));
 
   mIsFocusEventPending = true;
 }
 
 void
-IMEContentObserver::PostTextChangeNotification(
-                      const TextChangeDataBase& aTextChangeData)
+IMEContentObserver::PostTextChangeNotification()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::PostTextChangeNotification("
-     "aTextChangeData=%s)",
-     this, TextChangeDataToString(aTextChangeData).get()));
+     "mTextChangeData=%s)",
+     this, TextChangeDataToString(mTextChangeData).get()));
 
-  mTextChangeData += aTextChangeData;
   MOZ_ASSERT(mTextChangeData.IsValid(),
              "mTextChangeData must have text change data");
-
-  MOZ_LOG(sIMECOLog, LogLevel::Debug,
-    ("IMECO: 0x%p IMEContentObserver::PostTextChangeNotification(), "
-     "mTextChangeData=%s)",
-     this, TextChangeDataToString(mTextChangeData).get()));
+  mIsTextChangeEventPending = true;
 }
 
 void
 IMEContentObserver::PostSelectionChangeNotification()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::PostSelectionChangeNotification(), "
      "mSelectionData={ mCausedByComposition=%s, mCausedBySelectionEvent=%s }",
@@ -1047,17 +1042,18 @@ void
 IMEContentObserver::MaybeNotifyIMEOfTextChange(
                       const TextChangeDataBase& aTextChangeData)
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::MaybeNotifyIMEOfTextChange("
      "aTextChangeData=%s)",
      this, TextChangeDataToString(aTextChangeData).get()));
 
-  PostTextChangeNotification(aTextChangeData);
+  mTextChangeData += aTextChangeData;
+  PostTextChangeNotification();
   FlushMergeableNotifications();
 }
 
 void
 IMEContentObserver::MaybeNotifyIMEOfSelectionChange(
                       bool aCausedByComposition,
                       bool aCausedBySelectionEvent)
 {
@@ -1214,21 +1210,22 @@ IMEContentObserver::FlushMergeableNotifi
     mIsFocusEventPending = false;
     nsContentUtils::AddScriptRunner(new FocusSetEvent(this));
     // This is the first notification to IME. So, we don't need to notify any
     // more since IME starts to query content after it gets focus.
     ClearPendingNotifications();
     return;
   }
 
-  if (mTextChangeData.IsValid()) {
+  if (mIsTextChangeEventPending) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::FlushMergeableNotifications(), "
        "creating TextChangeEvent...", this));
-    nsContentUtils::AddScriptRunner(new TextChangeEvent(this, mTextChangeData));
+    mIsTextChangeEventPending = false;
+    nsContentUtils::AddScriptRunner(new TextChangeEvent(this));
   }
 
   // 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) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::FlushMergeableNotifications(), "
@@ -1241,17 +1238,17 @@ IMEContentObserver::FlushMergeableNotifi
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::FlushMergeableNotifications(), "
        "creating PositionChangeEvent...", this));
     mIsPositionChangeEventPending = false;
     nsContentUtils::AddScriptRunner(new PositionChangeEvent(this));
   }
 
   // If notifications may cause new change, we should notify them now.
-  if (mTextChangeData.IsValid() ||
+  if (mIsTextChangeEventPending ||
       mIsSelectionChangeEventPending ||
       mIsPositionChangeEventPending) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::FlushMergeableNotifications(), "
        "posting AsyncMergeableNotificationsFlusher to current thread", this));
     nsRefPtr<AsyncMergeableNotificationsFlusher> asyncFlusher =
       new AsyncMergeableNotificationsFlusher(this);
     NS_DispatchToCurrentThread(asyncFlusher);
@@ -1440,27 +1437,29 @@ IMEContentObserver::TextChangeEvent::Run
        "due to impossible to notify IME of text change", this));
     return NS_OK;
   }
 
   if (!IsSafeToNotifyIME()) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p   IMEContentObserver::TextChangeEvent::Run(), retrying to "
        "send NOTIFY_IME_OF_TEXT_CHANGE...", this));
-    mIMEContentObserver->PostTextChangeNotification(mTextChangeData);
+    mIMEContentObserver->PostTextChangeNotification();
     return NS_OK;
   }
 
   MOZ_LOG(sIMECOLog, LogLevel::Info,
     ("IMECO: 0x%p IMEContentObserver::TextChangeEvent::Run(), "
-     "sending NOTIFY_IME_OF_TEXT_CHANGE... mTextChangeData=%s",
-     this, TextChangeDataToString(mTextChangeData).get()));
+     "sending NOTIFY_IME_OF_TEXT_CHANGE... mIMEContentObserver={ "
+     "mTextChangeData=%s }",
+     this, TextChangeDataToString(mIMEContentObserver->mTextChangeData).get()));
 
   IMENotification notification(NOTIFY_IME_OF_TEXT_CHANGE);
-  notification.SetData(mTextChangeData);
+  notification.SetData(mIMEContentObserver->mTextChangeData);
+  mIMEContentObserver->mTextChangeData.Clear();
   IMEStateManager::NotifyIME(notification, mIMEContentObserver->mWidget);
 
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::TextChangeEvent::Run(), "
      "sent NOTIFY_IME_OF_TEXT_CHANGE", this));
   return NS_OK;
 }
 
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -114,17 +114,17 @@ private:
   State GetState() const;
   bool IsObservingContent(nsPresContext* aPresContext,
                           nsIContent* aContent) const;
   bool IsReflowLocked() const;
   bool IsSafeToNotifyIME() const;
 
   void PostFocusSetNotification();
   void MaybeNotifyIMEOfFocusSet();
-  void PostTextChangeNotification(const TextChangeDataBase& aTextChangeData);
+  void PostTextChangeNotification();
   void MaybeNotifyIMEOfTextChange(const TextChangeDataBase& aTextChangeData);
   void PostSelectionChangeNotification();
   void MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition,
                                        bool aCausedBySelectionEvent);
   void PostPositionChangeNotification();
   void MaybeNotifyIMEOfPositionChange();
 
   void NotifyContentAdded(nsINode* aContainer, int32_t aStart, int32_t aEnd);
@@ -136,16 +136,17 @@ private:
   /**
    *  UnregisterObservers() unregisters all listeners and observers.
    */
   void UnregisterObservers();
   void FlushMergeableNotifications();
   void ClearPendingNotifications()
   {
     mIsFocusEventPending = false;
+    mIsTextChangeEventPending = false;
     mIsSelectionChangeEventPending = false;
     mIsPositionChangeEventPending = false;
     mTextChangeData.Clear();
   }
 
   /**
    * UpdateSelectionCache() updates mSelectionData with the latest selection.
    * This should be called only when IsSafeToNotifyIME() returns true.
@@ -232,16 +233,17 @@ private:
   nsIMEUpdatePreference mUpdatePreference;
   uint32_t mPreAttrChangeLength;
   uint32_t mSuppressNotifications;
   int64_t mPreCharacterDataChangeLength;
 
   bool mIsObserving;
   bool mIMEHasFocus;
   bool mIsFocusEventPending;
+  bool mIsTextChangeEventPending;
   bool mIsSelectionChangeEventPending;
   bool mIsPositionChangeEventPending;
   bool mIsFlushingPendingNotifications;
 
 
   /**
    * Helper classes to notify IME.
    */
@@ -298,29 +300,22 @@ private:
     {
     }
     NS_IMETHOD Run() override;
   };
 
   class TextChangeEvent : public AChangeEvent
   {
   public:
-    TextChangeEvent(IMEContentObserver* aIMEContentObserver,
-                    TextChangeDataBase& aTextChangeData)
+    explicit TextChangeEvent(IMEContentObserver* aIMEContentObserver)
       : AChangeEvent(eChangeEventType_Text, aIMEContentObserver)
-      , mTextChangeData(aTextChangeData)
     {
-      MOZ_ASSERT(mTextChangeData.IsValid());
-      // Reset aTextChangeData because this now consumes the data.
-      aTextChangeData.Clear();
+      MOZ_ASSERT(aIMEContentObserver->mTextChangeData.IsValid());
     }
     NS_IMETHOD Run() override;
-
-  private:
-    TextChangeDataBase mTextChangeData;
   };
 
   class PositionChangeEvent final : public AChangeEvent
   {
   public:
     explicit PositionChangeEvent(IMEContentObserver* aIMEContentObserver)
       : AChangeEvent(eChangeEventType_Position, aIMEContentObserver)
     {