Bug 1224994 part.6 Don't clear TSFTextStore::mContentForTSF until active composition is committed r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 29 Jun 2016 18:24:10 +0900
changeset 383445 29a6f23f88f735bf12d806611af7bcf6cfdc33f7
parent 383444 394d089cc6767e310d6ba6c2add43678b2548e59
child 383446 ab1697b41df8ed55a457662030db442889ee1636
push id22031
push usermasayuki@d-toybox.com
push dateMon, 04 Jul 2016 06:55:01 +0000
reviewersm_kato
bugs1224994
milestone50.0a1
Bug 1224994 part.6 Don't clear TSFTextStore::mContentForTSF until active composition is committed r?m_kato This patch stop clearing mContentForTSF at unlocking the document because we should keep it until active composition is committed. If so, TSF/TIP won't be confused by content changes by JS. So, this is important for a11y of TIP users in some complicated websites like GoogleDocs, Facebook, etc. Note that this patch doesn't work well without following patches. We need to stop notifying TSF of selection changes and text changed while mContentForTSF is valid. MozReview-Commit-ID: 9QOGZxdYU3I
widget/windows/TSFTextStore.cpp
widget/windows/TSFTextStore.h
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -1671,18 +1671,20 @@ TSFTextStore::FlushPendingActions()
                ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
                 "dispatching compositionstart event...", this));
         WidgetEventTime eventTime = mWidget->CurrentMessageWidgetEventTime();
         nsEventStatus status;
         rv = mDispatcher->StartComposition(status, &eventTime);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           MOZ_LOG(sTextStoreLog, LogLevel::Error,
             ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
-             "FAILED to dispatch compositionstart event.", this));
-          mDeferClearingContentForTSF = false;
+             "FAILED to dispatch compositionstart event, "
+             "IsComposingInContent()=%s",
+             this, GetBoolName(!IsComposingInContent())));
+          mDeferClearingContentForTSF = !IsComposingInContent();
         }
         if (!mWidget || mWidget->Destroyed()) {
           break;
         }
         break;
       }
       case PendingAction::COMPOSITION_UPDATE: {
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
@@ -1698,30 +1700,34 @@ TSFTextStore::FlushPendingActions()
         // composition update.
         mDeferClearingContentForTSF = true;
 
         rv = mDispatcher->SetPendingComposition(action.mData,
                                                 action.mRanges);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           MOZ_LOG(sTextStoreLog, LogLevel::Error,
             ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
-             "FAILED to setting pending composition...", this));
-          mDeferClearingContentForTSF = false;
+             "FAILED to setting pending composition... "
+             "IsComposingInContent()=%s",
+             this, GetBoolName(IsComposingInContent())));
+          mDeferClearingContentForTSF = !IsComposingInContent();
         } else {
           MOZ_LOG(sTextStoreLog, LogLevel::Debug,
             ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
              "dispatching compositionchange event...", this));
           WidgetEventTime eventTime = mWidget->CurrentMessageWidgetEventTime();
           nsEventStatus status;
           rv = mDispatcher->FlushPendingComposition(status, &eventTime);
           if (NS_WARN_IF(NS_FAILED(rv))) {
             MOZ_LOG(sTextStoreLog, LogLevel::Error,
               ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
-               "FAILED to dispatch compositionchange event.", this));
-            mDeferClearingContentForTSF = false;
+               "FAILED to dispatch compositionchange event, "
+               "IsComposingInContent()=%s",
+               this, GetBoolName(IsComposingInContent())));
+            mDeferClearingContentForTSF = !IsComposingInContent();
           }
           // Be aware, the mWidget might already have been destroyed.
         }
         break;
       }
       case PendingAction::COMPOSITION_END: {
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
                ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
@@ -1738,18 +1744,20 @@ TSFTextStore::FlushPendingActions()
                ("TSF: 0x%p   TSFTextStore::FlushPendingActions(), "
                 "dispatching compositioncommit event...", this));
         WidgetEventTime eventTime = mWidget->CurrentMessageWidgetEventTime();
         nsEventStatus status;
         rv = mDispatcher->CommitComposition(status, &action.mData, &eventTime);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           MOZ_LOG(sTextStoreLog, LogLevel::Error,
             ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
-             "FAILED to dispatch compositioncommit event.", this));
-          mDeferClearingContentForTSF = false;
+             "FAILED to dispatch compositioncommit event, "
+             "IsComposingInContent()=%s",
+             this, GetBoolName(IsComposingInContent())));
+          mDeferClearingContentForTSF = !IsComposingInContent();
         }
         break;
       }
       case PendingAction::SET_SELECTION: {
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
                ("TSF: 0x%p   TSFTextStore::FlushPendingActions() "
                 "flushing SET_SELECTION={ mSelectionStart=%d, "
                 "mSelectionLength=%d, mSelectionReversed=%s }, "
@@ -2020,20 +2028,21 @@ TSFTextStore::ContentForTSFRef()
       MOZ_LOG(sTextStoreLog, LogLevel::Error,
              ("TSF: 0x%p   TSFTextStore::ContentForTSFRef(), FAILED, due to "
               "GetCurrentText() failure", this));
       mContentForTSF.Clear();
       return mContentForTSF;
     }
 
     mContentForTSF.Init(text);
-    // Basically, the locked content should be cleared after the document is
+    // Basically, the cached content which is expected by TSF/TIP should be
+    // cleared after active composition is committed or the document lock is
     // unlocked.  However, in e10s mode, content will be modified
     // asynchronously.  In such case, mDeferClearingContentForTSF may be
-    // true even after the document is unlocked.
+    // true until whole dispatched events are handled by the focused editor.
     mDeferClearingContentForTSF = false;
   }
 
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
          ("TSF: 0x%p   TSFTextStore::ContentForTSFRef(): "
           "mContentForTSF={ mText=\"%s\" (Length()=%u), "
           "mLastCompositionString=\"%s\" (Length()=%u), "
           "mMinTextModifiedOffset=%u }",
@@ -4995,19 +5004,22 @@ TSFTextStore::OnUpdateCompositionInterna
      "mDestroyed=%s, mDeferNotifyingTSF=%s",
      this, GetBoolName(mDestroyed), GetBoolName(mDeferNotifyingTSF)));
 
   // There are nothing to do after destroyed.
   if (mDestroyed) {
     return NS_OK;
   }
 
-  // Now, all sent composition events are handled by the content even in
-  // e10s mode.
-  mDeferClearingContentForTSF = false;
+  // If composition is completely finished both in TSF/TIP and the focused
+  // editor which may be in a remote process, we can clear the cache until
+  // starting next composition.
+  if (!mComposition.IsComposing() && !IsComposingInContent()) {
+    mDeferClearingContentForTSF = false;
+  }
   mDeferNotifyingTSF = false;
   MaybeFlushPendingNotifications();
   return NS_OK;
 }
 
 nsresult
 TSFTextStore::OnMouseButtonEventInternal(
                 const IMENotification& aIMENotification)
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -788,29 +788,34 @@ protected:
     enum : uint32_t
     {
       NOT_MODIFIED = UINT32_MAX
     };
     uint32_t mMinTextModifiedOffset;
 
     bool mInitialized;
   };
-  // mContentForTSF starts to cache content of the document at first query of
-  // the content during a document lock.  The information is expected by TSF
+  // mContentForTSF is cache of content.  The information is expected by TSF
   // and TIP.  Therefore, this is useful for answering the query from TSF or
-  // TIP.  This is abandoned after document is unlocked and dispatched events
-  // are handled.  This is initialized by ContentForTSFRef() automatically.
-  // So, don't access this member directly except at calling Clear(),
-  // IsInitialized(), IsLayoutChangedAfter() or IsLayoutChanged().
+  // TIP.
+  // This is initialized by ContentForTSFRef() automatically (therefore, don't
+  // access this member directly except at calling Clear(), IsInitialized(),
+  // IsLayoutChangeAfter() or IsLayoutChanged()).
+  // This is cleared when:
+  //  - When there is no composition, the document is unlocked.
+  //  - When there is a composition, all dispatched events are handled by
+  //    the focused editor which may be in a remote process.
+  // So, if two compositions are created very quickly, this cache may not be
+  // cleared between eCompositionCommit(AsIs) and eCompositionStart.
   Content mContentForTSF;
 
   Content& ContentForTSFRef();
 
-  // While the documet is locked, this returns the text stored by
-  // mContentForTSF.  Otherwise, return the current text content.
+  // While mContentForTSF is valid, this returns the text stored by it.
+  // Otherwise, return the current text content retrieved by eQueryTextContent.
   bool GetCurrentText(nsAString& aTextContent);
 
   class MouseTracker final
   {
   public:
     static const DWORD kInvalidCookie = static_cast<DWORD>(-1);
 
     MouseTracker();