Bug 1376407 - part1: IMContextWrapper should cache selected string in…

…stead of length of selection r=m_kato

IMContextWrapper::EnsureToCacheSelection() always queries actual selection when the caller needs selected string.  However, this may be expensive and this is bad behavior for the following patch because it wants to emulate selection range until receiving next selection change notification.

Therefore, this patch makes IMContextWrapper::Selection store selected string instead of just its length like other native IME handlers

Additionally, this patch renames IMContextWrapper::mSelectedString to IMContextWrapper::mSelectedStringRemovedByComposition for making the difference between it and the new string in Selection clearer.

MozReview-Commit-ID: 3bygvW7sKf4

widget/gtk/IMContextWrapper.cpp

@@ -587,7 +587,7 @@ IMContextWrapper::OnFocusChangeInGecko(bool aFocus)
          ToChar(mIsIMFocused)));
 
     // We shouldn't carry over the removed string to another editor.
-    mSelectedString.Truncate();
+    mSelectedStringRemovedByComposition.Truncate();
     mSelection.Clear();
 }
 
@@ -986,7 +986,8 @@ IMContextWrapper::OnSelectionChange(nsWindow* aCaller,
         } else {
             // Modify the selection start offset with new offset.
             mCompositionStart = mSelection.mOffset;
-            // XXX We should modify mSelectedString? But how?
+            // XXX We should modify mSelectedStringRemovedByComposition?
+            // But how?
             MOZ_LOG(gGtkIMLog, LogLevel::Debug,
                 ("0x%p   OnSelectionChange(), ignored, mCompositionStart "
                  "is updated to %u, the selection change doesn't cause "
@@ -1454,7 +1455,8 @@ IMContextWrapper::DispatchCompositionChangeEvent(
     // Store the selected string which will be removed by following
     // compositionchange event.
     if (mCompositionState == eCompositionState_CompositionStartDispatched) {
-        if (NS_WARN_IF(!EnsureToCacheSelection(&mSelectedString))) {
+        if (NS_WARN_IF(!EnsureToCacheSelection(
+                            &mSelectedStringRemovedByComposition))) {
             // XXX How should we behave in this case??
         } else {
             // XXX We should assume, for now, any web applications don't change
@@ -1926,10 +1928,10 @@ IMContextWrapper::SetCursorPosition(GtkIMContext* aContext)
     MOZ_LOG(gGtkIMLog, LogLevel::Info,
         ("0x%p SetCursorPosition(aContext=0x%p), "
          "mCompositionTargetRange={ mOffset=%u, mLength=%u }"
-         "mSelection={ mOffset=%u, mLength=%u, mWritingMode=%s }",
+         "mSelection={ mOffset=%u, Length()=%u, mWritingMode=%s }",
          this, aContext, mCompositionTargetRange.mOffset,
          mCompositionTargetRange.mLength,
-         mSelection.mOffset, mSelection.mLength,
+         mSelection.mOffset, mSelection.Length(),
          GetWritingModeName(mSelection.mWritingMode).get()));
 
     bool useCaret = false;
@@ -2023,7 +2025,7 @@ IMContextWrapper::GetCurrentParagraph(nsAString& aText,
     nsEventStatus status;
 
     uint32_t selOffset = mCompositionStart;
-    uint32_t selLength = mSelectedString.Length();
+    uint32_t selLength = mSelectedStringRemovedByComposition.Length();
 
     // If focused editor doesn't have composition string, we should use
     // current selection.
@@ -2038,7 +2040,7 @@ IMContextWrapper::GetCurrentParagraph(nsAString& aText,
         }
 
         selOffset = mSelection.mOffset;
-        selLength = mSelection.mLength;
+        selLength = mSelection.Length();
     }
 
     MOZ_LOG(gGtkIMLog, LogLevel::Debug,
@@ -2077,9 +2079,10 @@ IMContextWrapper::GetCurrentParagraph(nsAString& aText,
     // GtkEntry doesn't remove selected string until committing, however,
     // our editor does it.  We should emulate the behavior for IME.
     if (EditorHasCompositionString() &&
-        mDispatchedCompositionString != mSelectedString) {
+        mDispatchedCompositionString != mSelectedStringRemovedByComposition) {
         textContent.Replace(mCompositionStart,
-            mDispatchedCompositionString.Length(), mSelectedString);
+            mDispatchedCompositionString.Length(),
+            mSelectedStringRemovedByComposition);
     }
 
     // Get only the focused paragraph, by looking for newlines
@@ -2136,7 +2139,8 @@ IMContextWrapper::DeleteText(GtkIMContext* aContext,
     bool editorHadCompositionString = EditorHasCompositionString();
     if (wasComposing) {
         selOffset = mCompositionStart;
-        if (!DispatchCompositionCommitEvent(aContext, &mSelectedString)) {
+        if (!DispatchCompositionCommitEvent(aContext,
+                 &mSelectedStringRemovedByComposition)) {
             MOZ_LOG(gGtkIMLog, LogLevel::Error,
                 ("0x%p   DeleteText(), FAILED, quitting from DeletText",
                  this));
@@ -2284,8 +2288,10 @@ IMContextWrapper::EnsureToCacheSelection(nsAString* aSelectedString)
         aSelectedString->Truncate();
     }
 
-    if (mSelection.IsValid() &&
-        (!mSelection.Collapsed() || !aSelectedString)) {
+    if (mSelection.IsValid()) {
+       if (aSelectedString) {
+           *aSelectedString = mSelection.mString;
+       }
        return true;
     }
 
@@ -2325,8 +2331,8 @@ IMContextWrapper::EnsureToCacheSelection(nsAString* aSelectedString)
 
     MOZ_LOG(gGtkIMLog, LogLevel::Debug,
         ("0x%p EnsureToCacheSelection(), Succeeded, mSelection="
-         "{ mOffset=%u, mLength=%u, mWritingMode=%s }",
-         this, mSelection.mOffset, mSelection.mLength,
+         "{ mOffset=%u, Length()=%u, mWritingMode=%s }",
+         this, mSelection.mOffset, mSelection.Length(),
          GetWritingModeName(mSelection.mWritingMode).get()));
     return true;
 }
@@ -2339,8 +2345,8 @@ void
 IMContextWrapper::Selection::Assign(const IMENotification& aIMENotification)
 {
     MOZ_ASSERT(aIMENotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE);
+    mString = aIMENotification.mSelectionChangeData.String();
     mOffset = aIMENotification.mSelectionChangeData.mOffset;
-    mLength = aIMENotification.mSelectionChangeData.Length();
     mWritingMode = aIMENotification.mSelectionChangeData.GetWritingMode();
 }
 
@@ -2349,8 +2355,8 @@ IMContextWrapper::Selection::Assign(const WidgetQueryContentEvent& aEvent)
 {
     MOZ_ASSERT(aEvent.mMessage == eQuerySelectedText);
     MOZ_ASSERT(aEvent.mSucceeded);
+    mString = aEvent.mReply.mString.Length();
     mOffset = aEvent.mReply.mOffset;
-    mLength = aEvent.mReply.mString.Length();
     mWritingMode = aEvent.GetWritingMode();
 }
 

widget/gtk/IMContextWrapper.h

@@ -131,9 +131,9 @@ class IMContextWrapper final : public TextEventDispatcherListener
     // was dispatched by compositionupdate event.
     nsString mDispatchedCompositionString;
 
-    // mSelectedString is the selected string which was removed by first
-    // compositionchange event.
-    nsString mSelectedString;
+    // mSelectedStringRemovedByComposition is the selected string which was
+    // removed by first compositionchange event.
+    nsString mSelectedStringRemovedByComposition;
 
     // OnKeyEvent() temporarily sets mProcessingKeyEvent to the given native
     // event.
@@ -216,35 +216,35 @@ class IMContextWrapper final : public TextEventDispatcherListener
 
     struct Selection final
     {
+        nsString mString;
         uint32_t mOffset;
-        uint32_t mLength;
         WritingMode mWritingMode;
 
         Selection()
             : mOffset(UINT32_MAX)
-            , mLength(UINT32_MAX)
         {
         }
 
         void Clear()
         {
+            mString.Truncate();
             mOffset = UINT32_MAX;
-            mLength = UINT32_MAX;
             mWritingMode = WritingMode();
         }
 
         void Assign(const IMENotification& aIMENotification);
         void Assign(const WidgetQueryContentEvent& aSelectedTextEvent);
 
         bool IsValid() const { return mOffset != UINT32_MAX; }
-        bool Collapsed() const { return !mLength; }
+        bool Collapsed() const { return mString.IsEmpty(); }
+        uint32_t Length() const { return mString.Length(); }
         uint32_t EndOffset() const
         {
             if (NS_WARN_IF(!IsValid())) {
                 return UINT32_MAX;
             }
             CheckedInt<uint32_t> endOffset =
-                CheckedInt<uint32_t>(mOffset) + mLength;
+                CheckedInt<uint32_t>(mOffset) + mString.Length();
             if (NS_WARN_IF(!endOffset.isValid())) {
                 return UINT32_MAX;
             }