Bug 1422230 - part 1: TextEventDispatcher should manage if dispatched composition events have been handled by remote content and TSFTextStore refer the state r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 12 Jan 2018 11:31:53 +0900
changeset 719985 d1e2e27dc01b3e7d4482663638958ee078933f07
parent 719984 dfa03f5a9d3058182aa8fde09bf6d47ed7a70c45
child 719986 fc14cdbc39b5f87d5ad808f4178fca649ed30ffe
push id95417
push usermasayuki@d-toybox.com
push dateSat, 13 Jan 2018 00:49:22 +0000
reviewersm_kato
bugs1422230
milestone59.0a1
Bug 1422230 - part 1: TextEventDispatcher should manage if dispatched composition events have been handled by remote content and TSFTextStore refer the state r?m_kato When composition events are handled by content actually, widget receives NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED notification. If focused content is in a remote process, this is notified only when all sending composition events are handled in the remote process. So, when widget receives the notification can there is no composition in IME, that means that nobody is composing composition at that time. This patch adds TextEventDispatcher::IsHandlingComposition() which returns false only when nobody has composition and makes TSFTextStore refer this method because TSFTextStore needs to know if focused content has composition in any cases. MozReview-Commit-ID: F1ZZgFJAArD
widget/TextEventDispatcher.cpp
widget/TextEventDispatcher.h
widget/windows/TSFTextStore.cpp
widget/windows/TSFTextStore.h
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -23,16 +23,17 @@ namespace widget {
 
 bool TextEventDispatcher::sDispatchKeyEventsDuringComposition = false;
 
 TextEventDispatcher::TextEventDispatcher(nsIWidget* aWidget)
   : mWidget(aWidget)
   , mDispatchingEvent(0)
   , mInputTransactionType(eNoInputTransaction)
   , mIsComposing(false)
+  , mIsHandlingComposition(false)
   , mHasFocus(false)
 {
   MOZ_RELEASE_ASSERT(mWidget, "aWidget must not be nullptr");
 
   static bool sInitialized = false;
   if (!sInitialized) {
     Preferences::AddBoolVarCache(
       &sDispatchKeyEventsDuringComposition,
@@ -160,26 +161,29 @@ TextEventDispatcher::BeginInputTransacti
   // method of this class.
   switch (aEvent->mMessage) {
     case eKeyDown:
     case eKeyPress:
     case eKeyUp:
       return NS_OK;
     case eCompositionStart:
       MOZ_ASSERT(!mIsComposing);
-      mIsComposing = true;
+      mIsComposing = mIsHandlingComposition = true;
       return NS_OK;
     case eCompositionChange:
       MOZ_ASSERT(mIsComposing);
-      mIsComposing = true;
+      MOZ_ASSERT(mIsHandlingComposition);
+      mIsComposing = mIsHandlingComposition = true;
       return NS_OK;
     case eCompositionCommit:
     case eCompositionCommitAsIs:
       MOZ_ASSERT(mIsComposing);
+      MOZ_ASSERT(mIsHandlingComposition);
       mIsComposing = false;
+      mIsHandlingComposition = true;
       return NS_OK;
     default:
       MOZ_ASSERT_UNREACHABLE("You forgot to handle the event");
       return NS_ERROR_UNEXPECTED;
   }
 }
 void
 TextEventDispatcher::EndInputTransaction(TextEventDispatcherListener* aListener)
@@ -307,17 +311,17 @@ TextEventDispatcher::StartComposition(ns
   }
 
   if (NS_WARN_IF(mIsComposing)) {
     return NS_ERROR_FAILURE;
   }
 
   // When you change some members from here, you may need same change in
   // BeginInputTransactionFor().
-  mIsComposing = true;
+  mIsComposing = mIsHandlingComposition = true;
   WidgetCompositionEvent compositionStartEvent(true, eCompositionStart,
                                                mWidget);
   InitEvent(compositionStartEvent);
   if (aEventTime) {
     compositionStartEvent.AssignEventTime(*aEventTime);
   }
   rv = DispatchEvent(mWidget, compositionStartEvent, aStatus);
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -421,19 +425,33 @@ TextEventDispatcher::CommitComposition(n
   return NS_OK;
 }
 
 nsresult
 TextEventDispatcher::NotifyIME(const IMENotification& aIMENotification)
 {
   nsresult rv = NS_ERROR_NOT_IMPLEMENTED;
 
-  if (aIMENotification.mMessage == NOTIFY_IME_OF_BLUR) {
-    mHasFocus = false;
-    ClearNotificationRequests();
+  switch (aIMENotification.mMessage) {
+    case NOTIFY_IME_OF_BLUR:
+      mHasFocus = false;
+      ClearNotificationRequests();
+      break;
+    case NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED:
+      // If content handles composition events when native IME doesn't have
+      // composition, that means that we completely finished handling
+      // composition(s).  Note that when focused content is in a remote
+      // process, this is sent when all dispatched composition events
+      // have been handled in the remote process.
+      if (!IsComposing()) {
+        mIsHandlingComposition = false;
+      }
+      break;
+    default:
+      break;
   }
 
 
   // First, send the notification to current input transaction's listener.
   nsCOMPtr<TextEventDispatcherListener> listener = do_QueryReferent(mListener);
   if (listener) {
     rv = listener->NotifyIME(this, aIMENotification);
   }
--- a/widget/TextEventDispatcher.h
+++ b/widget/TextEventDispatcher.h
@@ -97,21 +97,29 @@ public:
    *                                          should be called.
    *                NS_ERROR_NOT_AVAILABLE: The widget isn't available for
    *                                        composition.
    */
   nsresult GetState() const;
 
   /**
    * IsComposing() returns true after calling StartComposition() and before
-   * calling CommitComposition().
+   * calling CommitComposition().  In other words, native IME has composition
+   * when this returns true.
    */
   bool IsComposing() const { return mIsComposing; }
 
   /**
+   * IsHandlingComposition() returns true after calling StartComposition() and
+   * content has not handled eCompositionCommit(AsIs) event.  In other words,
+   * our content has composition when this returns true.
+   */
+  bool IsHandlingComposition() const { return mIsHandlingComposition; }
+
+  /**
    * IsInNativeInputTransaction() returns true if native IME handler began a
    * transaction and it's not finished yet.
    */
   bool IsInNativeInputTransaction() const
   {
     return mInputTransactionType == eNativeInputTransaction;
   }
 
@@ -431,16 +439,19 @@ private:
       default:
         MOZ_CRASH("Define the behavior of new InputTransactionType");
     }
   }
 
   // See IsComposing().
   bool mIsComposing;
 
+  // See IsHandlingComposition().
+  bool mIsHandlingComposition;
+
   // true while NOTIFY_IME_OF_FOCUS is received but NOTIFY_IME_OF_BLUR has not
   // received yet.  Otherwise, false.
   bool mHasFocus;
 
   // If this is true, keydown and keyup events are dispatched even when there
   // is a composition.
   static bool sDispatchKeyEventsDuringComposition;
 
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -2277,19 +2277,21 @@ TSFTextStore::FlushPendingActions()
            "dispatching compositionstart event...", this));
         WidgetEventTime eventTime = widget->CurrentMessageWidgetEventTime();
         nsEventStatus status;
         rv = mDispatcher->StartComposition(status, &eventTime);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           MOZ_LOG(sTextStoreLog, LogLevel::Error,
             ("0x%p   TSFTextStore::FlushPendingActions() "
              "FAILED to dispatch compositionstart event, "
-             "IsComposingInContent()=%s",
-             this, GetBoolName(!IsComposingInContent())));
-          mDeferClearingContentForTSF = !IsComposingInContent();
+             "IsHandlingComposition()=%s",
+             this, GetBoolName(IsHandlingComposition())));
+          // XXX Is this right? If there is a composition in content,
+          //     shouldn't we wait NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED?
+          mDeferClearingContentForTSF = !IsHandlingComposition();
         }
         if (!widget || widget->Destroyed()) {
           break;
         }
         break;
       }
       case PendingAction::COMPOSITION_UPDATE: {
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
@@ -2307,63 +2309,72 @@ TSFTextStore::FlushPendingActions()
         mDeferClearingContentForTSF = true;
 
         rv = mDispatcher->SetPendingComposition(action.mData,
                                                 action.mRanges);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           MOZ_LOG(sTextStoreLog, LogLevel::Error,
             ("0x%p   TSFTextStore::FlushPendingActions() "
              "FAILED to setting pending composition... "
-             "IsComposingInContent()=%s",
-             this, GetBoolName(IsComposingInContent())));
-          mDeferClearingContentForTSF = !IsComposingInContent();
+             "IsHandlingComposition()=%s",
+             this, GetBoolName(IsHandlingComposition())));
+          // XXX Is this right? If there is a composition in content,
+          //     shouldn't we wait NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED?
+          mDeferClearingContentForTSF = !IsHandlingComposition();
         } else {
           MOZ_LOG(sTextStoreLog, LogLevel::Debug,
             ("0x%p   TSFTextStore::FlushPendingActions() "
              "dispatching compositionchange event...", this));
           WidgetEventTime eventTime = widget->CurrentMessageWidgetEventTime();
           nsEventStatus status;
           rv = mDispatcher->FlushPendingComposition(status, &eventTime);
           if (NS_WARN_IF(NS_FAILED(rv))) {
             MOZ_LOG(sTextStoreLog, LogLevel::Error,
               ("0x%p   TSFTextStore::FlushPendingActions() "
                "FAILED to dispatch compositionchange event, "
-               "IsComposingInContent()=%s",
-               this, GetBoolName(IsComposingInContent())));
-            mDeferClearingContentForTSF = !IsComposingInContent();
+               "IsHandlingComposition()=%s",
+               this, GetBoolName(IsHandlingComposition())));
+            // XXX Is this right? If there is a composition in content,
+            //     shouldn't we wait NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED?
+            mDeferClearingContentForTSF = !IsHandlingComposition();
           }
           // Be aware, the mWidget might already have been destroyed.
         }
         break;
       }
       case PendingAction::COMPOSITION_END: {
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
           ("0x%p   TSFTextStore::FlushPendingActions() "
            "flushing COMPOSITION_END={ mData=\"%s\" }",
            this, GetEscapedUTF8String(action.mData).get()));
 
         // Dispatching eCompositionCommit causes a DOM text event, then,
-        // the IME will be notified of NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED.
-        // In this case, we should not clear mContentForTSFuntil we notify
-        // the IME of the composition update.
+        // the IME will be notified of NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED
+        // when focused content actually handles the event.  For example,
+        // when focused content is in a remote process, it's sent when
+        // all dispatched composition events have been handled in the remote
+        // process.  So, until then, we don't have newer content information.
+        // Therefore, we need to put off to clear mContentForTSF.
         mDeferClearingContentForTSF = true;
 
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
           ("0x%p   TSFTextStore::FlushPendingActions(), "
            "dispatching compositioncommit event...", this));
         WidgetEventTime eventTime = widget->CurrentMessageWidgetEventTime();
         nsEventStatus status;
         rv = mDispatcher->CommitComposition(status, &action.mData, &eventTime);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           MOZ_LOG(sTextStoreLog, LogLevel::Error,
             ("0x%p   TSFTextStore::FlushPendingActions() "
              "FAILED to dispatch compositioncommit event, "
-             "IsComposingInContent()=%s",
-             this, GetBoolName(IsComposingInContent())));
-          mDeferClearingContentForTSF = !IsComposingInContent();
+             "IsHandlingComposition()=%s",
+             this, GetBoolName(IsHandlingComposition())));
+          // XXX Is this right? If there is a composition in content,
+          //     shouldn't we wait NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED?
+          mDeferClearingContentForTSF = !IsHandlingComposition();
         }
         break;
       }
       case PendingAction::SET_SELECTION: {
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
           ("0x%p   TSFTextStore::FlushPendingActions() "
            "flushing SET_SELECTION={ mSelectionStart=%d, "
            "mSelectionLength=%d, mSelectionReversed=%s }, "
@@ -2627,28 +2638,16 @@ TSFTextStore::DoNotReturnErrorFromGetSel
   // That was introduced in Anniversary Update (build 14393, see bug 1312302)
   // TODO: We should avoid to run this hack on fixed builds.  When we get
   //       exact build number, we should get back here.
   static bool sTSFMayCrashIfGetSelectionReturnsError =
     IsWindows10BuildOrLater(14393);
   return sTSFMayCrashIfGetSelectionReturnsError;
 }
 
-bool
-TSFTextStore::IsComposingInContent() const
-{
-  if (!mDispatcher) {
-    return false;
-  }
-  if (!mDispatcher->IsInNativeInputTransaction()) {
-    return false;
-  }
-  return mDispatcher->IsComposing();
-}
-
 TSFTextStore::Content&
 TSFTextStore::ContentForTSFRef()
 {
   // This should be called when the document is locked or the content hasn't
   // been abandoned yet.
   if (NS_WARN_IF(!IsReadLocked() && !mContentForTSF.IsInitialized())) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
       ("0x%p   TSFTextStore::ContentForTSFRef(), FAILED, due to "
@@ -5887,19 +5886,19 @@ TSFTextStore::OnUpdateCompositionInterna
      this, GetBoolName(mDestroyed), GetBoolName(mDeferNotifyingTSF)));
 
   // There are nothing to do after destroyed.
   if (mDestroyed) {
     return NS_OK;
   }
 
   // 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()) {
+  // editor which may be in a remote process, we can clear the cache and don't
+  // have it until starting next composition.
+  if (!mComposition.IsComposing() && !IsHandlingComposition()) {
     mDeferClearingContentForTSF = false;
   }
   mDeferNotifyingTSF = false;
   MaybeFlushPendingNotifications();
   return NS_OK;
 }
 
 nsresult
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -452,22 +452,23 @@ protected:
   // DOM events since the DOM events' handlers may modify the locked document.
   // However, even while the document is locked, TSF may queries us.
   // For that, TSFTextStore modifies mComposition even while the document is
   // locked.  With mComposition, query methods can returns the text content
   // information.
   Composition mComposition;
 
   /**
-   * IsComposingInContent() returns true if there is a composition in the
-   * focused editor and it's caused by native IME (either TIP of TSF or IME of
-   * IMM).  I.e., returns true between eCompositionStart and
-   * eCompositionCommit(AsIs).
+   * IsHandlingComposition() returns true if there is a composition in the
+   * focused editor.
    */
-  bool IsComposingInContent() const;
+  bool IsHandlingComposition() const
+  {
+    return mDispatcher && mDispatcher->IsHandlingComposition();
+  }
 
   class Selection
   {
   public:
     Selection() : mDirty(true) {}
 
     bool IsDirty() const { return mDirty; };
     void MarkDirty() { mDirty = true; }