Bug 1236640 - Make selection change part of the IME change transaction; r=esawin a=sylvestre
authorJim Chen <nchen@mozilla.com>
Thu, 14 Jan 2016 13:50:10 -0500
changeset 298400 088009bfaae4662be59ef8fdfe0a23397295ad4e
parent 298399 746c9a0348f607456e2d7db191351cc2073ded39
child 298401 31eb8acb454fc5da5ed361456563eb14ee095730
push id8937
push userkwierso@gmail.com
push dateTue, 19 Jan 2016 22:45:39 +0000
treeherdermozilla-aurora@fe6c0f654c3f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin, sylvestre
bugs1236640
milestone45.0a2
Bug 1236640 - Make selection change part of the IME change transaction; r=esawin a=sylvestre We notify IME text changes in transactions. We should make selection change notification part of that transaction.
widget/android/nsWindow.cpp
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -321,17 +321,17 @@ private:
 
     void SendIMEDummyKeyEvents();
     void AddIMETextChange(const IMETextChange& aChange);
 
     enum FlushChangesFlag {
         FLUSH_FLAG_NONE,
         FLUSH_FLAG_RETRY
     };
-    void PostFlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
+    void PostFlushIMEChanges();
     void FlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
 
 public:
     bool NotifyIME(const IMENotification& aIMENotification);
     void SetInputContext(const InputContext& aContext,
                          const InputContextAction& aAction);
     InputContext GetInputContext();
 
@@ -2045,30 +2045,29 @@ nsWindow::Natives::AddIMETextChange(cons
         mIMETextChanges.RemoveElementAt(srcIndex);
         // Any ranges that we skip over between src and dst are not mergeable
         // so we can safely continue the merge starting at dst
         srcIndex = dstIndex;
     }
 }
 
 void
-nsWindow::Natives::PostFlushIMEChanges(FlushChangesFlag aFlags)
+nsWindow::Natives::PostFlushIMEChanges()
 {
-    if (aFlags != FLUSH_FLAG_RETRY &&
-            (!mIMETextChanges.IsEmpty() || mIMESelectionChanged)) {
+    if (!mIMETextChanges.IsEmpty() || mIMESelectionChanged) {
         // Already posted
         return;
     }
 
     // Keep a strong reference to the window to keep 'this' alive.
     RefPtr<nsWindow> window(&this->window);
 
-    nsAppShell::gAppShell->PostEvent([this, window, aFlags] {
+    nsAppShell::gAppShell->PostEvent([this, window] {
         if (!window->Destroyed()) {
-            FlushIMEChanges(aFlags);
+            FlushIMEChanges();
         }
     });
 }
 
 void
 nsWindow::Natives::FlushIMEChanges(FlushChangesFlag aFlags)
 {
     // Only send change notifications if we are *not* masking events,
@@ -2093,16 +2092,33 @@ nsWindow::Natives::FlushIMEChanges(Flush
     };
     nsAutoTArray<TextRecord, 4> textTransaction;
     if (mIMETextChanges.Length() > textTransaction.Capacity()) {
         textTransaction.SetCapacity(mIMETextChanges.Length());
     }
 
     mIMETextChangedDuringFlush = false;
 
+    auto shouldAbort = [=] () -> bool {
+        if (!mIMETextChangedDuringFlush) {
+            return false;
+        }
+        // A query event could have triggered more text changes to come in, as
+        // indicated by our flag. If that happens, try flushing IME changes
+        // again.
+        if (aFlags != FLUSH_FLAG_RETRY) {
+            FlushIMEChanges(FLUSH_FLAG_RETRY);
+        } else {
+            // Don't retry if already retrying, to avoid infinite loops.
+            __android_log_print(ANDROID_LOG_WARN, "GeckoViewSupport",
+                    "Already retrying IME flush");
+        }
+        return true;
+    };
+
     for (const IMETextChange &change : mIMETextChanges) {
         if (change.mStart == change.mOldEnd &&
                 change.mStart == change.mNewEnd) {
             continue;
         }
 
         WidgetQueryContentEvent event(true, eQueryTextContent, &window);
 
@@ -2110,51 +2126,55 @@ nsWindow::Natives::FlushIMEChanges(Flush
             window.InitEvent(event, nullptr);
             event.InitForQueryTextContent(change.mStart,
                                           change.mNewEnd - change.mStart);
             window.DispatchEvent(&event);
             NS_ENSURE_TRUE_VOID(event.mSucceeded);
             NS_ENSURE_TRUE_VOID(event.mReply.mContentsRoot == imeRoot.get());
         }
 
-        if (mIMETextChangedDuringFlush) {
-            // The query event above could have triggered more text changes to
-            // come in, as indicated by our flag. If that happens, try flushing
-            // IME changes again later.
-            if (!NS_WARN_IF(aFlags == FLUSH_FLAG_RETRY)) {
-                // Don't retry if already retrying, to avoid infinite loops.
-                PostFlushIMEChanges(FLUSH_FLAG_RETRY);
-            }
+        if (shouldAbort()) {
             return;
         }
 
         textTransaction.AppendElement(
                 TextRecord{event.mReply.mString, change.mStart,
                            change.mOldEnd, change.mNewEnd});
     }
 
+    int32_t selStart;
+    int32_t selEnd;
+
+    if (mIMESelectionChanged) {
+        WidgetQueryContentEvent event(true, eQuerySelectedText, &window);
+        window.InitEvent(event, nullptr);
+        window.DispatchEvent(&event);
+
+        NS_ENSURE_TRUE_VOID(event.mSucceeded);
+        NS_ENSURE_TRUE_VOID(event.mReply.mContentsRoot == imeRoot.get());
+
+        if (shouldAbort()) {
+            return;
+        }
+
+        selStart = int32_t(event.GetSelectionStart());
+        selEnd = int32_t(event.GetSelectionEnd());
+    }
+
+    // Commit the text change and selection change transaction.
     mIMETextChanges.Clear();
 
     for (const TextRecord& record : textTransaction) {
         mEditable->OnTextChange(record.text, record.start,
                                 record.oldEnd, record.newEnd);
     }
 
     if (mIMESelectionChanged) {
         mIMESelectionChanged = false;
-
-        WidgetQueryContentEvent event(true, eQuerySelectedText, &window);
-        window.InitEvent(event, nullptr);
-        window.DispatchEvent(&event);
-
-        NS_ENSURE_TRUE_VOID(event.mSucceeded);
-        NS_ENSURE_TRUE_VOID(event.mReply.mContentsRoot == imeRoot.get());
-
-        mEditable->OnSelectionChange(int32_t(event.GetSelectionStart()),
-                                     int32_t(event.GetSelectionEnd()));
+        mEditable->OnSelectionChange(selStart, selEnd);
     }
 }
 
 bool
 nsWindow::Natives::NotifyIME(const IMENotification& aIMENotification)
 {
     MOZ_ASSERT(mEditable);