Bug 1470786 - 1. Support async text changes from replacing text; r?esawin draft
authorJim Chen <nchen@mozilla.com>
Tue, 17 Jul 2018 11:22:34 -0400
changeset 819302 66f071146bcdad97d958250e53b62f5628e3cc8b
parent 819215 547144f5596c1a146b208d68d93950a6313080ca
child 819303 330b9ebdec531667423fde48348f7dfb664c11be
push id116499
push userbmo:nchen@mozilla.com
push dateTue, 17 Jul 2018 15:23:54 +0000
reviewersesawin
bugs1470786
milestone63.0a1
Bug 1470786 - 1. Support async text changes from replacing text; r?esawin Currently, we expect editing operations in GeckoEditableSupport::OnImeReplaceText to cause synchronous text change notifications. However, under e10s, text change notifications can be asynchornous. The new code keeps track of active OnImeReplaceText calls, and look for async text changes before replying to the calls. MozReview-Commit-ID: INM3JLmQebK
mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testInputConnection.java
widget/android/GeckoEditableSupport.cpp
widget/android/GeckoEditableSupport.h
--- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testInputConnection.java
+++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testInputConnection.java
@@ -449,18 +449,17 @@ public class testInputConnection extends
             });
 
             // Bug 1254629, crash when hiding input during input.
             ic.commitText("foo", 1);
             assertTextAndSelectionAt("Can commit text (hiding)", ic, "foo", 3);
 
             ic.commitText("!", 1);
             // The '!' key causes the input to hide in robocop_input.html,
-            // and there won't be a text/selection update as a result.
-            assertTextAndSelectionAt("Can handle hiding input", ic, "foo", 3);
+            assertTextAndSelectionAt("Can handle hiding input", ic, "foo!", 4);
 
             // Bug 1401737, Editable does not behave correctly after disconnecting from Gecko.
             getJS().syncCall("blur_hiding_input");
             processGeckoEvents();
             processInputConnectionEvents();
 
             ic.setComposingRegion(0, 3);
             ic.commitText("bar", 1);
--- a/widget/android/GeckoEditableSupport.cpp
+++ b/widget/android/GeckoEditableSupport.cpp
@@ -903,16 +903,21 @@ GeckoEditableSupport::FlushIMEChanges(Fl
         }
     }
 
     if (mIMESelectionChanged) {
         mIMESelectionChanged = false;
         mEditable->OnSelectionChange(selStart, selEnd);
         flushOnException();
     }
+
+    while (mIMEActiveReplaceTextCount) {
+        mIMEActiveReplaceTextCount--;
+        OnImeSynchronize();
+    }
 }
 
 void
 GeckoEditableSupport::FlushIMEText(FlushChangesFlag aFlags)
 {
     // Notify Java of the newly focused content
     mIMETextChanges.Clear();
     mIMESelectionChanged = true;
@@ -961,33 +966,48 @@ GeckoEditableSupport::OnImeSynchronize()
     }
     mEditable->NotifyIME(EditableListener::NOTIFY_IME_REPLY_EVENT);
 }
 
 void
 GeckoEditableSupport::OnImeReplaceText(int32_t aStart, int32_t aEnd,
                                        jni::String::Param aText)
 {
-    AutoIMESynchronize as(this);
+    mIMEActiveReplaceTextCount++;
+
+    if (!DoReplaceText(aStart, aEnd, aText)) {
+        // We did not process the event, so reply to it now.
+        mIMEActiveReplaceTextCount--;
+        OnImeSynchronize();
+    }
+}
+
+bool
+GeckoEditableSupport::DoReplaceText(int32_t aStart, int32_t aEnd,
+                                    jni::String::Param aText)
+{
+    // Return true if processed and we should reply to the OnImeReplaceText
+    // event later. Return false if _not_ processed and we should reply to the
+    // OnImeReplaceText event now.
 
     if (mIMEMaskEventsCount > 0) {
         // Not focused; still reply to events, but don't do anything else.
-        return;
+        return false;
     }
 
     if (mWindow) {
         mWindow->UserActivity();
     }
 
     /*
         Replace text in Gecko thread from aStart to aEnd with the string text.
     */
     nsCOMPtr<nsIWidget> widget = GetWidget();
-    NS_ENSURE_TRUE_VOID(mDispatcher && widget);
-    NS_ENSURE_SUCCESS_VOID(BeginInputTransaction(mDispatcher));
+    NS_ENSURE_TRUE(mDispatcher && widget, false);
+    NS_ENSURE_SUCCESS(BeginInputTransaction(mDispatcher), false);
 
     RefPtr<TextComposition> composition(GetComposition());
     MOZ_ASSERT(!composition || !composition->IsEditorHandlingEvent());
 
     nsString string(aText->ToString());
     const bool composing = !mIMERanges->IsEmpty();
     nsEventStatus status = nsEventStatus_eIgnore;
 
@@ -1023,27 +1043,27 @@ GeckoEditableSupport::OnImeReplaceText(i
                 } else {
                     mDispatcher->MaybeDispatchKeypressEvents(*event, status);
                 }
                 if (!mDispatcher || widget->Destroyed()) {
                     break;
                 }
             }
             mIMEKeyEvents.Clear();
-            return;
+            return true;
         }
 
         if (aStart != aEnd) {
             // Perform a deletion first.
             WidgetContentCommandEvent event(
                     true, eContentCommandDelete, widget);
             event.mTime = PR_Now() / 1000;
             widget->DispatchEvent(&event, status);
             if (!mDispatcher || widget->Destroyed()) {
-                return;
+                return false;
             }
         }
     } else if (composition->String().Equals(string)) {
         /* If the new text is the same as the existing composition text,
          * the NS_COMPOSITION_CHANGE event does not generate a text
          * change notification. However, the Java side still expects
          * one, so we manually generate a notification. */
         IMETextChange dummyChange;
@@ -1051,37 +1071,38 @@ GeckoEditableSupport::OnImeReplaceText(i
         dummyChange.mOldEnd = dummyChange.mNewEnd = aEnd;
         AddIMETextChange(dummyChange);
     }
 
     if (sDispatchKeyEventsInCompositionForAnyApps ||
         mInputContext.mMayBeIMEUnaware) {
         SendIMEDummyKeyEvent(widget, eKeyDown);
         if (!mDispatcher || widget->Destroyed()) {
-            return;
+            return false;
         }
     }
 
     if (composing) {
         mDispatcher->SetPendingComposition(string, mIMERanges);
         mDispatcher->FlushPendingComposition(status);
         // Ensure IME ranges are empty.
         mIMERanges->Clear();
     } else if (!string.IsEmpty() || mDispatcher->IsComposing()) {
         mDispatcher->CommitComposition(status, &string);
     }
     if (!mDispatcher || widget->Destroyed()) {
-        return;
+        return false;
     }
 
     if (sDispatchKeyEventsInCompositionForAnyApps ||
         mInputContext.mMayBeIMEUnaware) {
         SendIMEDummyKeyEvent(widget, eKeyUp);
         // Widget may be destroyed after dispatching the above event.
     }
+    return true;
 }
 
 void
 GeckoEditableSupport::OnImeAddCompositionRange(
         int32_t aStart, int32_t aEnd, int32_t aRangeType, int32_t aRangeStyle,
         int32_t aRangeLineStyle, bool aRangeBoldLine, int32_t aRangeForeColor,
         int32_t aRangeBackColor, int32_t aRangeLineColor)
 {
@@ -1303,16 +1324,17 @@ GeckoEditableSupport::NotifyIME(TextEven
             ALOGIME("IME: NOTIFY_IME_OF_BLUR");
 
             mIMEFocusCount--;
             MOZ_ASSERT(mIMEFocusCount >= 0);
 
             RefPtr<GeckoEditableSupport> self(this);
             nsAppShell::PostEvent([this, self] {
                 if (!mIMEFocusCount) {
+                    mIMEActiveReplaceTextCount = 0;
                     mEditable->NotifyIME(EditableListener::NOTIFY_IME_OF_BLUR);
                     OnRemovedFrom(mDispatcher);
                 }
             });
 
             // Mask events because we lost focus. Unmask on the next focus.
             mIMEMaskEventsCount++;
             break;
--- a/widget/android/GeckoEditableSupport.h
+++ b/widget/android/GeckoEditableSupport.h
@@ -38,26 +38,16 @@ class GeckoEditableSupport final
            composition through update composition events
     */
 
     using EditableBase =
             java::GeckoEditableChild::Natives<GeckoEditableSupport>;
     using EditableClient = java::SessionTextInput::EditableClient;
     using EditableListener = java::SessionTextInput::EditableListener;
 
-    // RAII helper class that automatically sends an event reply through
-    // OnImeSynchronize, as required by events like OnImeReplaceText.
-    class AutoIMESynchronize
-    {
-        GeckoEditableSupport* const mGES;
-    public:
-        explicit AutoIMESynchronize(GeckoEditableSupport* ges) : mGES(ges) {}
-        ~AutoIMESynchronize() { mGES->OnImeSynchronize(); }
-    };
-
     struct IMETextChange final {
         int32_t mStart, mOldEnd, mNewEnd;
 
         IMETextChange() :
             mStart(-1), mOldEnd(-1), mNewEnd(-1) {}
 
         explicit IMETextChange(const IMENotification& aIMENotification)
             : mStart(aIMENotification.mTextChangeData.mStartOffset)
@@ -97,16 +87,17 @@ class GeckoEditableSupport final
     java::GeckoEditableChild::GlobalRef mEditable;
     bool mEditableAttached;
     InputContext mInputContext;
     AutoTArray<UniquePtr<mozilla::WidgetEvent>, 4> mIMEKeyEvents;
     AutoTArray<IMETextChange, 4> mIMETextChanges;
     RefPtr<TextRangeArray> mIMERanges;
     int32_t mIMEMaskEventsCount; // Mask events when > 0.
     int32_t mIMEFocusCount; // We are focused when > 0.
+    int32_t mIMEActiveReplaceTextCount; // We still need to reply when > 0.
     bool mIMESelectionChanged;
     bool mIMETextChangedDuringFlush;
     bool mIMEMonitorCursor;
 
     static bool sDispatchKeyEventsInCompositionForAnyApps;
 
     void ObservePrefs();
 
@@ -131,16 +122,17 @@ class GeckoEditableSupport final
             RemoveCompositionFlag aFlag = COMMIT_IME_COMPOSITION);
     void SendIMEDummyKeyEvent(nsIWidget* aWidget, EventMessage msg);
     void AddIMETextChange(const IMETextChange& aChange);
     void PostFlushIMEChanges();
     void FlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
     void FlushIMEText(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
     void AsyncNotifyIME(int32_t aNotification);
     void UpdateCompositionRects();
+    bool DoReplaceText(int32_t aStart, int32_t aEnd, jni::String::Param aText);
 
 public:
     template<typename Functor>
     static void OnNativeCall(Functor&& aCall)
     {
         struct IMEEvent : nsAppShell::LambdaEvent<Functor>
         {
             explicit IMEEvent(Functor&& l) : nsAppShell::LambdaEvent<Functor>(std::move(l)) {}
@@ -176,16 +168,17 @@ public:
                          java::GeckoEditableChild::Param aEditableChild)
         : mIsRemote(!aWindow)
         , mWindow(aPtr, aWindow)
         , mEditable(aEditableChild)
         , mEditableAttached(!mIsRemote)
         , mIMERanges(new TextRangeArray())
         , mIMEMaskEventsCount(1) // Mask IME events since there's no focus yet
         , mIMEFocusCount(0)
+        , mIMEActiveReplaceTextCount(0)
         , mIMESelectionChanged(false)
         , mIMETextChangedDuringFlush(false)
         , mIMEMonitorCursor(false)
     {
         ObservePrefs();
     }
 
     // Constructor for content process GeckoEditableChild.