Bug 1224994 part.8 Don't notify TSF of text changes while there is cached content r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 30 Jun 2016 15:04:02 +0900
changeset 303678 8866c7c9a21d08e36cf45c1e9e2da6b6ef4b098d
parent 303677 d0b6fa38f05d68fb62eb4c2a1490ee59e7aea20e
child 303679 ff08df084602201a7daaa124b2b01671271daa0a
push id79141
push usercbook@mozilla.com
push dateTue, 05 Jul 2016 14:07:42 +0000
treeherdermozilla-inbound@f08c54971dd1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1224994
milestone50.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 1224994 part.8 Don't notify TSF of text changes while there is cached content r=m_kato Same as selection change notification, text change notification shouldn't be notified to TSF while there is cachec content because neither TSF nor TIP may allow to change text by web applications during keeping storing cached content. This patch makes TSFTextStore stores and merges text changes until MaybeFlushPendingNotifications() is called and there is no cached content. MozReview-Commit-ID: 9fj0GREbX18
widget/windows/TSFTextStore.cpp
widget/windows/TSFTextStore.h
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -1849,16 +1849,22 @@ TSFTextStore::MaybeFlushPendingNotificat
     MOZ_LOG(sTextStoreLog, LogLevel::Debug,
            ("TSF: 0x%p   TSFTextStore::MaybeFlushPendingNotifications(), "
             "mContentForTSF is cleared", this));
   }
 
   // When there is no cached content, we can sync actual contents and TSF/TIP
   // expecting contents.
   if (!mContentForTSF.IsInitialized()) {
+    if (mPendingTextChangeData.IsValid()) {
+      MOZ_LOG(sTextStoreLog, LogLevel::Info,
+             ("TSF: 0x%p   TSFTextStore::MaybeFlushPendingNotifications(), "
+              "calling TSFTextStore::NotifyTSFOfTextChange()...", this));
+      NotifyTSFOfTextChange();
+    }
     if (mPendingSelectionChangeData.IsValid()) {
       MOZ_LOG(sTextStoreLog, LogLevel::Info,
              ("TSF: 0x%p   TSFTextStore::MaybeFlushPendingNotifications(), "
               "calling TSFTextStore::NotifyTSFOfSelectionChange()...", this));
       NotifyTSFOfSelectionChange();
     }
   }
 
@@ -4577,18 +4583,17 @@ TSFTextStore::GetIMEUpdatePreference()
     }
   }
   return nsIMEUpdatePreference();
 }
 
 nsresult
 TSFTextStore::OnTextChangeInternal(const IMENotification& aIMENotification)
 {
-  const IMENotification::TextChangeDataBase& textChangeData =
-    aIMENotification.mTextChangeData;
+  const TextChangeDataBase& textChangeData = aIMENotification.mTextChangeData;
 
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
          ("TSF: 0x%p   TSFTextStore::OnTextChangeInternal(aIMENotification={ "
           "mMessage=0x%08X, mTextChangeData={ mStartOffset=%lu, "
           "mRemovedEndOffset=%lu, mAddedEndOffset=%lu, "
           "mCausedOnlyByComposition=%s, "
           "mIncludingChangesDuringComposition=%s, "
           "mIncludingChangesWithoutComposition=%s }), "
@@ -4607,96 +4612,87 @@ TSFTextStore::OnTextChangeInternal(const
           GetBoolName(mComposition.IsComposing())));
 
   if (mDestroyed) {
     // If this instance is already destroyed, we shouldn't notify TSF of any
     // changes.
     return NS_OK;
   }
 
-  if (textChangeData.mCausedOnlyByComposition) {
-    // Ignore text change notifications caused only by composition since it's
-    // already been handled internally.
-    return NS_OK;
-  }
-
-  if (mComposition.IsComposing() &&
-      !textChangeData.mIncludingChangesDuringComposition) {
-    // Ignore text changes when they don't include changes caused not by
-    // composition at the latest composition because changes before current
-    // composition start shouldn't cause forcibly committing composition.
-    // In the future, we should notify TSF of such delayed text changes
-    // after current composition is active (In such case,
-    // mIncludingChangesWithoutComposition is true).
-    return NS_OK;
-  }
-
   mDeferNotifyingTSF = false;
 
-  if (IsReadLocked()) {
-    // XXX If text change occurs during the document is locked, it must be
-    //     modified by Javascript.  In such case, we should notify merged
-    //     text changes after it's unlocked.
-    return NS_OK;
-  }
-
-  mSelection.MarkDirty();
-
+  // Different from selection change, we don't modify anything with text
+  // change data.  Therefore, if neither TSF not TIP wants text change
+  // notifications, we don't need to store the changes.
   if (!mSink || !(mSinkMask & TS_AS_TEXT_CHANGE)) {
     return NS_OK;
   }
 
-  if (aIMENotification.mTextChangeData.IsInInt32Range()) {
-    TS_TEXTCHANGE textChange;
-    textChange.acpStart = static_cast<LONG>(textChangeData.mStartOffset);
-    textChange.acpOldEnd = static_cast<LONG>(textChangeData.mRemovedEndOffset);
-    textChange.acpNewEnd = static_cast<LONG>(textChangeData.mAddedEndOffset);
-    NotifyTSFOfTextChange(textChange);
-  } else {
-    MOZ_LOG(sTextStoreLog, LogLevel::Error,
-           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfTextChange() FAILED due to "
-            "offset is too big for calling "
-            "ITextStoreACPSink::OnTextChange()...",
-            this));
-  }
+  // Merge any text change data even if it's caused by composition.
+  mPendingTextChangeData.MergeWith(textChangeData);
 
   MaybeFlushPendingNotifications();
 
   return NS_OK;
 }
 
 void
-TSFTextStore::NotifyTSFOfTextChange(const TS_TEXTCHANGE& aTextChange)
+TSFTextStore::NotifyTSFOfTextChange()
 {
   MOZ_ASSERT(!mDestroyed);
-
-  // XXX We need to cache the text change ranges and notify TSF of that
-  //     the document is unlocked.
-  if (NS_WARN_IF(IsReadLocked())) {
+  MOZ_ASSERT(!IsReadLocked());
+  MOZ_ASSERT(!mComposition.IsComposing());
+  MOZ_ASSERT(mPendingTextChangeData.IsValid());
+
+  // If the text changes are caused only by composition, we don't need to
+  // notify TSF of the text changes.
+  if (mPendingTextChangeData.mCausedOnlyByComposition) {
+    mPendingTextChangeData.Clear();
     return;
   }
 
-  // Some TIPs are confused by text change notification during composition.
-  // Especially, some of them stop working for composition in our process.
-  // For preventing it, let's commit the composition.
-  if (mComposition.IsComposing()) {
-    MOZ_LOG(sTextStoreLog, LogLevel::Info,
-           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfTextChange(), "
-            "committing the composition for avoiding making TIP confused...",
+  // First, forget cached selection.
+  mSelection.MarkDirty();
+
+  // For making it safer, we should check if there is a valid sink to receive
+  // text change notification.
+  if (NS_WARN_IF(!mSink) || NS_WARN_IF(!(mSinkMask & TS_AS_TEXT_CHANGE))) {
+    MOZ_LOG(sTextStoreLog, LogLevel::Error,
+           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfTextChange() FAILED due to "
+            "mSink is not ready to call ITextStoreACPSink::OnTextChange()...",
             this));
-    CommitCompositionInternal(false);
+    mPendingTextChangeData.Clear();
     return;
   }
 
+  if (NS_WARN_IF(!mPendingTextChangeData.IsInInt32Range())) {
+    MOZ_LOG(sTextStoreLog, LogLevel::Error,
+           ("TSF: 0x%p   TSFTextStore::NotifyTSFOfTextChange() FAILED due to "
+            "offset is too big for calling "
+            "ITextStoreACPSink::OnTextChange()...",
+            this));
+    mPendingTextChangeData.Clear();
+    return;
+   }
+
+  TS_TEXTCHANGE textChange;
+  textChange.acpStart =
+    static_cast<LONG>(mPendingTextChangeData.mStartOffset);
+  textChange.acpOldEnd =
+    static_cast<LONG>(mPendingTextChangeData.mRemovedEndOffset);
+  textChange.acpNewEnd =
+    static_cast<LONG>(mPendingTextChangeData.mAddedEndOffset);
+  mPendingTextChangeData.Clear();
+
   MOZ_LOG(sTextStoreLog, LogLevel::Info,
          ("TSF: 0x%p   TSFTextStore::NotifyTSFOfTextChange(), calling "
           "ITextStoreACPSink::OnTextChange(0, { acpStart=%ld, acpOldEnd=%ld, "
-          "acpNewEnd=%ld })...", this, aTextChange.acpStart,
-          aTextChange.acpOldEnd, aTextChange.acpNewEnd));
-  mSink->OnTextChange(0, &aTextChange);
+          "acpNewEnd=%ld })...", this, textChange.acpStart,
+          textChange.acpOldEnd, textChange.acpNewEnd));
+  mSink->OnTextChange(0, &textChange);
 }
 
 nsresult
 TSFTextStore::OnSelectionChangeInternal(const IMENotification& aIMENotification)
 {
   const SelectionChangeDataBase& selectionChangeData =
     aIMENotification.mSelectionChangeData;
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
@@ -4723,16 +4719,19 @@ TSFTextStore::OnSelectionChangeInternal(
     // changes.
     return NS_OK;
   }
 
   mDeferNotifyingTSF = false;
 
   // Assign the new selection change data to the pending selection change data
   // because only the latest selection data is necessary.
+  // Note that this is necessary to update mSelection.  Therefore, even if
+  // neither TSF nor TIP wants selection change notifications, we need to
+  // store the selection information.
   mPendingSelectionChangeData.Assign(selectionChangeData);
 
   // Flush remaining pending notifications here if it's possible.
   MaybeFlushPendingNotifications();
 
   return NS_OK;
 }
 
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -51,16 +51,18 @@ struct MSGResult;
 
 class TSFTextStore final : public ITextStoreACP
                          , public ITfContextOwnerCompositionSink
                          , public ITfMouseTrackerACP
 {
 private:
   typedef IMENotification::SelectionChangeDataBase SelectionChangeDataBase;
   typedef IMENotification::SelectionChangeData SelectionChangeData;
+  typedef IMENotification::TextChangeDataBase TextChangeDataBase;
+  typedef IMENotification::TextChangeData TextChangeData;
 
 public: /*IUnknown*/
   STDMETHODIMP          QueryInterface(REFIID, void**);
 
   NS_INLINE_DECL_IUNKNOWN_REFCOUNTING(TSFTextStore)
 
 public: /*ITextStoreACP*/
   STDMETHODIMP AdviseSink(REFIID, IUnknown*, DWORD);
@@ -316,17 +318,22 @@ protected:
   nsresult OnLayoutChangeInternal();
   nsresult OnUpdateCompositionInternal();
 
   // mPendingSelectionChangeData stores selection change data until notifying
   // TSF of selection change.  If two or more selection changes occur, this
   // stores the latest selection change data because only it is necessary.
   SelectionChangeData mPendingSelectionChangeData;
 
-  void     NotifyTSFOfTextChange(const TS_TEXTCHANGE& aTextChange);
+  // mPendingTextChangeData stores one or more text change data until notifying
+  // TSF of text change.  If two or more text changes occur, this merges
+  // every text change data.
+  TextChangeData mPendingTextChangeData;
+
+  void     NotifyTSFOfTextChange();
   void     NotifyTSFOfSelectionChange();
   bool     NotifyTSFOfLayoutChange();
   void     NotifyTSFOfLayoutChangeAgain();
 
   HRESULT  HandleRequestAttrs(DWORD aFlags,
                               ULONG aFilterCount,
                               const TS_ATTRID* aFilterAttrs);
   void     SetInputScope(const nsString& aHTMLInputType,