Bug 1470786 - 1. Support async text changes from replacing text; r=esawin a=RyanVM GECKOVIEW_64_RELBRANCH
authorJim Chen <nchen@mozilla.com>
Tue, 17 Jul 2018 11:22:34 -0400
branchGECKOVIEW_64_RELBRANCH
changeset 501459 6891b272d3fd
parent 501458 f95d41399777
child 501469 f891c440a238
push id1871
push userjcristau@mozilla.com
push dateWed, 05 Dec 2018 21:37:07 +0000
treeherdermozilla-release@6891b272d3fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin, RyanVM
bugs1470786
milestone64.0
Bug 1470786 - 1. Support async text changes from replacing text; r=esawin a=RyanVM 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;
 
@@ -1024,27 +1044,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;
@@ -1052,37 +1072,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)
 {
@@ -1304,16 +1325,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.