Backed out changeset 7e6f51f7fca4 (bug 1470786) for causing bug 1505161; a=backout
authorJim Chen <nchen@mozilla.com>
Fri, 09 Nov 2018 14:53:26 -0500
changeset 498430 12e947ab2054
parent 498429 43799c2555d4
child 498431 189d2cf9ca93
push id10156
push usernchen@mozilla.com
push dateFri, 09 Nov 2018 19:54:41 +0000
treeherdermozilla-beta@12e947ab2054 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1470786, 1505161
milestone64.0
backs out7e6f51f7fca4
Backed out changeset 7e6f51f7fca4 (bug 1470786) for causing bug 1505161; a=backout
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,17 +449,18 @@ 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,
-            assertTextAndSelectionAt("Can handle hiding input", ic, "foo!", 4);
+            // and there won't be a text/selection update as a result.
+            assertTextAndSelectionAt("Can handle hiding input", ic, "foo", 3);
 
             // 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,21 +903,16 @@ 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;
@@ -966,48 +961,33 @@ GeckoEditableSupport::OnImeSynchronize()
     }
     mEditable->NotifyIME(EditableListener::NOTIFY_IME_REPLY_EVENT);
 }
 
 void
 GeckoEditableSupport::OnImeReplaceText(int32_t aStart, int32_t aEnd,
                                        jni::String::Param aText)
 {
-    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.
+    AutoIMESynchronize as(this);
 
     if (mIMEMaskEventsCount > 0) {
         // Not focused; still reply to events, but don't do anything else.
-        return false;
+        return;
     }
 
     if (mWindow) {
         mWindow->UserActivity();
     }
 
     /*
         Replace text in Gecko thread from aStart to aEnd with the string text.
     */
     nsCOMPtr<nsIWidget> widget = GetWidget();
-    NS_ENSURE_TRUE(mDispatcher && widget, false);
-    NS_ENSURE_SUCCESS(BeginInputTransaction(mDispatcher), false);
+    NS_ENSURE_TRUE_VOID(mDispatcher && widget);
+    NS_ENSURE_SUCCESS_VOID(BeginInputTransaction(mDispatcher));
 
     RefPtr<TextComposition> composition(GetComposition());
     MOZ_ASSERT(!composition || !composition->IsEditorHandlingEvent());
 
     nsString string(aText->ToString());
     const bool composing = !mIMERanges->IsEmpty();
     nsEventStatus status = nsEventStatus_eIgnore;
 
@@ -1044,27 +1024,27 @@ GeckoEditableSupport::DoReplaceText(int3
                 } else {
                     mDispatcher->MaybeDispatchKeypressEvents(*event, status);
                 }
                 if (!mDispatcher || widget->Destroyed()) {
                     break;
                 }
             }
             mIMEKeyEvents.Clear();
-            return true;
+            return;
         }
 
         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 false;
+                return;
             }
         }
     } 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;
@@ -1072,38 +1052,37 @@ GeckoEditableSupport::DoReplaceText(int3
         dummyChange.mOldEnd = dummyChange.mNewEnd = aEnd;
         AddIMETextChange(dummyChange);
     }
 
     if (sDispatchKeyEventsInCompositionForAnyApps ||
         mInputContext.mMayBeIMEUnaware) {
         SendIMEDummyKeyEvent(widget, eKeyDown);
         if (!mDispatcher || widget->Destroyed()) {
-            return false;
+            return;
         }
     }
 
     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 false;
+        return;
     }
 
     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)
 {
@@ -1325,17 +1304,16 @@ 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,16 +38,26 @@ 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)
@@ -87,17 +97,16 @@ 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();
 
@@ -122,17 +131,16 @@ 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)) {}
@@ -168,17 +176,16 @@ 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.