Bug 1510527 - Active composition count may be mismatched when updating composition. r=esawin
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Fri, 18 Jan 2019 03:18:24 +0000
changeset 511489 8a464d751ed487778109a90475332f4130372b56
parent 511488 d1b5339852740fc2e559b66668204fc9202daf15
child 511490 ddb55e312c8a2971091d704a52aec679c85b264d
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin
bugs1510527
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1510527 - Active composition count may be mismatched when updating composition. r=esawin To avoid FlushIMEChanges per updating IME composition, we calculate composition count in DoReplaceText. But when using GV+e10s, this calculation is sometimes invalid since NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED event isn't received per PendingComposition. Because, IMEStateManager will merge this completed events due to optimization of IME event. Also, DoUpdateComposition calls SetPendingComposition, but it doesn't touch mIMEActiveCompositionCount, So when using some IME, this value is minus or forever non-zero on some IMEs. So we shouldn't use atomic count. When receiving NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED, we should reset it and allow IMEFlushChanges since Gecko has already handled all IME composition events in event queues. Differential Revision: https://phabricator.services.mozilla.com/D14668
widget/android/GeckoEditableSupport.cpp
widget/android/GeckoEditableSupport.h
--- a/widget/android/GeckoEditableSupport.cpp
+++ b/widget/android/GeckoEditableSupport.cpp
@@ -1046,16 +1046,19 @@ void GeckoEditableSupport::OnImeReplaceT
     mIMEDelaySynchronizeReply = true;
   }
 
   OnImeSynchronize();
 }
 
 bool GeckoEditableSupport::DoReplaceText(int32_t aStart, int32_t aEnd,
                                          jni::String::Param aText) {
+  ALOGIME("IME: IME_REPLACE_TEXT: text=\"%s\"",
+          NS_ConvertUTF16toUTF8(aText->ToString()).get());
+
   // 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 false;
   }
@@ -1292,16 +1295,17 @@ bool GeckoEditableSupport::DoUpdateCompo
 #endif  // DEBUG_ANDROID_IME
 
   if (NS_WARN_IF(NS_FAILED(BeginInputTransaction(mDispatcher)))) {
     mIMERanges->Clear();
     return false;
   }
   mDispatcher->SetPendingComposition(string, mIMERanges);
   mDispatcher->FlushPendingComposition(status);
+  mIMEActiveCompositionCount++;
   mIMERanges->Clear();
   return true;
 }
 
 void GeckoEditableSupport::OnImeRequestCursorUpdates(int aRequestMode) {
   if (aRequestMode == EditableClient::ONE_SHOT) {
     UpdateCompositionRects();
     return;
@@ -1429,37 +1433,55 @@ nsresult GeckoEditableSupport::NotifyIME
       mIMESelectionChanged = true;
       AddIMETextChange(IMETextChange(aNotification));
       break;
     }
 
     case NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED: {
       ALOGIME("IME: NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED");
 
-      // We often only get one event-handled notification after a pair of
-      // update-composition then replace-text calls. Therefore, only count
-      // the number of composition events for replace-text calls to reduce
-      // the chance of mismatch.
-      if (!(--mIMEActiveCompositionCount) && mIMEDelaySynchronizeReply) {
-        FlushIMEChanges();
-      }
-
-      // Hardware keyboard support requires each string rect.
-      if (mIMEMonitorCursor) {
-        UpdateCompositionRects();
+      // NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED isn't sent per IME call.
+      // Receiving this event means that Gecko has already handled all IME
+      // composing events in queue.
+      //
+      if (mIsRemote) {
+        OnNotifyIMEOfCompositionEventHandled();
+      } else {
+        // Also, when receiving this event, mIMEDelaySynchronizeReply won't
+        // update yet on non-e10s case since IME event is posted before updating
+        // it. So we have to delay handling of this event.
+        RefPtr<GeckoEditableSupport> self(this);
+        nsAppShell::PostEvent([this, self] {
+          OnNotifyIMEOfCompositionEventHandled();
+        });
       }
       break;
     }
 
     default:
       break;
   }
   return NS_OK;
 }
 
+void GeckoEditableSupport::OnNotifyIMEOfCompositionEventHandled()
+{
+  // NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED may be merged with multiple events,
+  // so reset count.
+  mIMEActiveCompositionCount = 0;
+  if (mIMEDelaySynchronizeReply) {
+    FlushIMEChanges();
+  }
+
+  // Hardware keyboard support requires each string rect.
+  if (mIMEMonitorCursor) {
+    UpdateCompositionRects();
+  }
+}
+
 void GeckoEditableSupport::OnRemovedFrom(
     TextEventDispatcher* aTextEventDispatcher) {
   mDispatcher = nullptr;
 
   if (mIsRemote && mEditable->HasEditableParent()) {
     // When we're remote, detach every time.
     OnDetach(NS_NewRunnableFunction(
         "GeckoEditableSupport::OnRemovedFrom",
--- a/widget/android/GeckoEditableSupport.h
+++ b/widget/android/GeckoEditableSupport.h
@@ -118,16 +118,17 @@ class GeckoEditableSupport final
   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);
   bool DoUpdateComposition(int32_t aStart, int32_t aEnd, int32_t aFlags);
+  void OnNotifyIMEOfCompositionEventHandled();
   void NotifyIMEContext(const InputContext& aContext,
                         const InputContextAction& aAction);
 
  public:
   template <typename Functor>
   static void OnNativeCall(Functor&& aCall) {
     struct IMEEvent : nsAppShell::LambdaEvent<Functor> {
       explicit IMEEvent(Functor&& l)