Bug 1376417 - part2: ContentCache should adjust composition start offset when querying content with relative offset r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 29 Jun 2017 18:31:09 +0900
changeset 366818 55ffef6341b7ec410a4d01f7e732b506ea34a336
parent 366817 4b627ad5d95a24eefe39d1e3df36f0d31b5b8659
child 366819 bbd783f7fd35e57b31e77d884031a43a1bb723f6
push id92071
push usercbook@mozilla.com
push dateFri, 30 Jun 2017 13:12:39 +0000
treeherdermozilla-inbound@d865e836f74a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1376417
milestone56.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 1376417 - part2: ContentCache should adjust composition start offset when querying content with relative offset r=m_kato When IME commits composition and restarts composition immediately, query event with relative offset doesn't work as expected until the commit is handled in the remote process. Therefore, this patch makes ContentCacheInParent store the last commit string length until it's handled in the content. Note that ContentCache doesn't need to store all commit string lengthes which have not been handled in the remote process yet because it's important and possible to return next character of commited composition only when there are not 2 or more pending compositions. Even if there are, i.e., the remote process is too busy, ContentCacheInParent doesn't have necessary character rect. MozReview-Commit-ID: 8gBr8kO4JcF
widget/ContentCache.cpp
widget/ContentCache.h
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -343,17 +343,20 @@ ContentCacheInChild::CacheTextRects(nsIW
     IMEStateManager::GetTextCompositionFor(aWidget);
   if (textComposition) {
     // mCompositionStart may be updated by some composition event handlers.
     // So, let's update it with the latest information.
     mCompositionStart = textComposition->NativeOffsetOfStartComposition();
     // Note that TextComposition::String() may not be modified here because
     // it's modified after all edit action listeners are performed but this
     // is called while some of them are performed.
-    uint32_t length = textComposition->LastData().Length();
+    // FYI: For supporting IME which commits composition and restart new
+    //      composition immediately, we should cache next character of current
+    //      composition too.
+    uint32_t length = textComposition->LastData().Length() + 1;
     mTextRectArray.mStart = mCompositionStart;
     if (NS_WARN_IF(!QueryCharRectArray(aWidget, mTextRectArray.mStart, length,
                                        mTextRectArray.mRects))) {
       MOZ_LOG(sContentCacheLog, LogLevel::Error,
         ("0x%p CacheTextRects(), FAILED, "
          "couldn't retrieve text rect array of the composition string", this));
     }
   }
@@ -540,47 +543,53 @@ ContentCacheInParent::AssignContent(cons
 
   // When the widget has composition, we should set mCompositionStart to
   // *current* composition start offset.  Note that, in strictly speaking,
   // widget should not use WidgetQueryContentEvent if there are some pending
   // compositions (i.e., when mPendingCompositionCount is 2 or more).
   mCompositionStartInChild = aOther.mCompositionStart;
   if (mWidgetHasComposition) {
     if (aOther.mCompositionStart != UINT32_MAX) {
-      mCompositionStart = aOther.mCompositionStart;
-    } else {
+      if (mCompositionStart != aOther.mCompositionStart) {
+        mCompositionStart = aOther.mCompositionStart;
+        mPendingCommitLength = 0;
+      }
+    } else if (mCompositionStart != mSelection.StartOffset()) {
       mCompositionStart = mSelection.StartOffset();
+      mPendingCommitLength = 0;
       NS_WARNING_ASSERTION(mCompositionStart != UINT32_MAX,
                            "mCompositionStart shouldn't be invalid offset when "
                            "the widget has composition");
     }
-  } else {
+  } else if (mCompositionStart != UINT32_MAX) {
     mCompositionStart = UINT32_MAX;
+    mPendingCommitLength = 0;
   }
 
   MOZ_LOG(sContentCacheLog, LogLevel::Info,
     ("0x%p AssignContent(aNotification=%s), "
      "Succeeded, mText.Length()=%u, mSelection={ mAnchor=%u, mFocus=%u, "
      "mWritingMode=%s, mAnchorCharRects[eNextCharRect]=%s, "
      "mAnchorCharRects[ePrevCharRect]=%s, mFocusCharRects[eNextCharRect]=%s, "
      "mFocusCharRects[ePrevCharRect]=%s, mRect=%s }, "
      "mFirstCharRect=%s, mCaret={ mOffset=%u, mRect=%s }, mTextRectArray={ "
      "mStart=%u, mRects.Length()=%" PRIuSIZE " }, mWidgetHasComposition=%s, "
-     "mPendingCompositionCount=%u, mCompositionStart=%u, mEditorRect=%s",
+     "mPendingCompositionCount=%u, mCompositionStart=%u, "
+     "mPendingCommitLength=%u, mEditorRect=%s",
      this, GetNotificationName(aNotification),
      mText.Length(), mSelection.mAnchor, mSelection.mFocus,
      GetWritingModeName(mSelection.mWritingMode).get(),
      GetRectText(mSelection.mAnchorCharRects[eNextCharRect]).get(),
      GetRectText(mSelection.mAnchorCharRects[ePrevCharRect]).get(),
      GetRectText(mSelection.mFocusCharRects[eNextCharRect]).get(),
      GetRectText(mSelection.mFocusCharRects[ePrevCharRect]).get(),
      GetRectText(mSelection.mRect).get(), GetRectText(mFirstCharRect).get(),
      mCaret.mOffset, GetRectText(mCaret.mRect).get(), mTextRectArray.mStart,
      mTextRectArray.mRects.Length(), GetBoolName(mWidgetHasComposition),
-     mPendingCompositionCount, mCompositionStart,
+     mPendingCompositionCount, mCompositionStart, mPendingCommitLength,
      GetRectText(mEditorRect).get()));
 }
 
 bool
 ContentCacheInParent::HandleQueryContentEvent(WidgetQueryContentEvent& aEvent,
                                               nsIWidget* aWidget) const
 {
   MOZ_ASSERT(aWidget);
@@ -620,41 +629,49 @@ ContentCacheInParent::HandleQueryContent
           ("0x%p HandleQueryContentEvent(), FAILED due to "
            "aEvent.mInput.MakeOffsetAbsolute(0) failure, aEvent={ mMessage=%s, "
            "mInput={ mOffset=%" PRId64 ", mLength=%" PRIu32 " } }",
            this, ToChar(aEvent.mMessage), aEvent.mInput.mOffset,
            aEvent.mInput.mLength));
         return false;
       }
     } else if (mWidgetHasComposition) {
-      if (NS_WARN_IF(!aEvent.mInput.MakeOffsetAbsolute(mCompositionStart))) {
+      if (NS_WARN_IF(!aEvent.mInput.MakeOffsetAbsolute(
+                                      mCompositionStart +
+                                        mPendingCommitLength))) {
         MOZ_LOG(sContentCacheLog, LogLevel::Error,
           ("0x%p HandleQueryContentEvent(), FAILED due to "
-           "aEvent.mInput.MakeOffsetAbsolute(mCompositionStart) failure, "
-           "mCompositionStart=%d, aEvent={ mMessage=%s, "
-           "mInput={ mOffset=%" PRId64 ", mLength=%" PRIu32 " } }",
-           this, mCompositionStart, ToChar(aEvent.mMessage),
-           aEvent.mInput.mOffset, aEvent.mInput.mLength));
+           "aEvent.mInput.MakeOffsetAbsolute(mCompositionStart + "
+           "mPendingCommitLength) failure, "
+           "mCompositionStart=%" PRIu32 ", mPendingCommitLength=%" PRIu32 ", "
+           "aEvent={ mMessage=%s, mInput={ mOffset=%" PRId64
+           ", mLength=%" PRIu32 " } }",
+           this, mCompositionStart, mPendingCommitLength,
+           ToChar(aEvent.mMessage), aEvent.mInput.mOffset,
+           aEvent.mInput.mLength));
         return false;
       }
     } else if (NS_WARN_IF(!mSelection.IsValid())) {
       MOZ_LOG(sContentCacheLog, LogLevel::Error,
         ("0x%p HandleQueryContentEvent(), FAILED due to mSelection is invalid",
          this));
       return false;
     } else if (NS_WARN_IF(!aEvent.mInput.MakeOffsetAbsolute(
-                                           mSelection.StartOffset()))) {
+                                           mSelection.StartOffset() +
+                                             mPendingCommitLength))) {
       MOZ_LOG(sContentCacheLog, LogLevel::Error,
         ("0x%p HandleQueryContentEvent(), FAILED due to "
-         "aEvent.mInput.MakeOffsetAbsolute(mSelection.StartOffset()) "
-         "failure, mSelection={ StartOffset()=%d, Length()=%d }, "
-         "aEvent={ mMessage=%s, mInput={ mOffset=%" PRId64 ", mLength=%" PRIu32 " } }",
+         "aEvent.mInput.MakeOffsetAbsolute(mSelection.StartOffset() + "
+         "mPendingCommitLength) failure, "
+         "mSelection={ StartOffset()=%d, Length()=%d }, "
+         "mPendingCommitLength=%" PRIu32 ", aEvent={ mMessage=%s, "
+         "mInput={ mOffset=%" PRId64 ", mLength=%" PRIu32 " } }",
          this, mSelection.StartOffset(), mSelection.Length(),
-         ToChar(aEvent.mMessage), aEvent.mInput.mOffset,
-         aEvent.mInput.mLength));
+         mPendingCommitLength, ToChar(aEvent.mMessage),
+         aEvent.mInput.mOffset, aEvent.mInput.mLength));
       return false;
     }
   }
 
   switch (aEvent.mMessage) {
     case eQuerySelectedText:
       MOZ_LOG(sContentCacheLog, LogLevel::Info,
         ("0x%p HandleQueryContentEvent("
@@ -1108,16 +1125,19 @@ ContentCacheInParent::OnCompositionEvent
     MOZ_RELEASE_ASSERT(mPendingCompositionCount < UINT8_MAX);
     mPendingCompositionCount++;
   }
 
   mWidgetHasComposition = !aEvent.CausesDOMCompositionEndEvent();
 
   if (!mWidgetHasComposition) {
     mCompositionStart = UINT32_MAX;
+    if (mPendingCompositionCount == 1) {
+      mPendingCommitLength = aEvent.mData.Length();
+    }
   }
 
   // During REQUEST_TO_COMMIT_COMPOSITION or REQUEST_TO_CANCEL_COMPOSITION,
   // widget usually sends a eCompositionChange and/or eCompositionCommit event
   // to finalize or clear the composition, respectively.  In this time,
   // we need to intercept all composition events here and pass the commit
   // string for returning to the remote process as a result of
   // RequestIMEToCommitComposition().  Then, eCommitComposition event will
@@ -1170,16 +1190,21 @@ ContentCacheInParent::OnEventNeedingAckH
     ("0x%p OnEventNeedingAckHandled(aWidget=0x%p, "
      "aMessage=%s), mPendingEventsNeedingAck=%u, mPendingCompositionCount=%" PRIu8,
      this, aWidget, ToChar(aMessage), mPendingEventsNeedingAck, mPendingCompositionCount));
 
   if (WidgetCompositionEvent::IsFollowedByCompositionEnd(aMessage) ||
       aMessage == eCompositionCommitRequestHandled) {
     MOZ_RELEASE_ASSERT(mPendingCompositionCount > 0);
     mPendingCompositionCount--;
+    // Forget pending commit string length if it's handled in the remote
+    // process.  Note that this doesn't care too old composition's commit
+    // string because in such case, we cannot return proper information
+    // to IME synchornously.
+    mPendingCommitLength = 0;
   }
 
   MOZ_RELEASE_ASSERT(mPendingEventsNeedingAck > 0);
   if (--mPendingEventsNeedingAck) {
     return;
   }
 
   FlushPendingNotifications(aWidget);
--- a/widget/ContentCache.h
+++ b/widget/ContentCache.h
@@ -412,16 +412,23 @@ private:
   nsAString* mCommitStringByRequest;
   // mPendingEventsNeedingAck is increased before sending a composition event or
   // a selection event and decreased after they are received in the child
   // process.
   uint32_t mPendingEventsNeedingAck;
   // mCompositionStartInChild stores current composition start offset in the
   // remote process.
   uint32_t mCompositionStartInChild;
+  // mPendingCommitLength is commit string length of the first pending
+  // composition.  This is used by relative offset query events when querying
+  // new composition start offset.
+  // Note that when mPendingCompositionCount is not 0, i.e., there are 2 or
+  // more pending compositions, this cache won't be used because in such case,
+  // anyway ContentCacheInParent cannot return proper character rect.
+  uint32_t mPendingCommitLength;
   // mPendingCompositionCount is number of compositions which started in widget
   // but not yet handled in the child process.
   uint8_t mPendingCompositionCount;
   // mWidgetHasComposition is true when the widget in this process thinks that
   // IME has composition.  So, this is set to true when eCompositionStart is
   // dispatched and set to false when eCompositionCommit(AsIs) is dispatched.
   bool mWidgetHasComposition;