Bug 1218032 part.2 Make sending focus notification to IME async-aware r=smaug

dom/events/IMEContentObserver.cpp

@@ -251,26 +251,56 @@ IMEContentObserver::Init(nsIWidget* aWidget,
   }
 
   if (firstInitialization) {
+    // Now, try to send NOTIFY_IME_OF_FOCUS to IME via the widget.
     MaybeNotifyIMEOfFocusSet();
-
-    // While Init() notifies IME of focus, pending layout may be flushed
-    // because the notification may cause querying content.  Then, recursive
-    // call of Init() with the latest content may be occur.  In such case, we
-    // shouldn't keep first initialization.
-    if (GetState() != eState_Initializing) {
-      return;
-    }
-
-    // NOTIFY_IME_OF_FOCUS might cause recreating IMEContentObserver
-    // instance via IMEStateManager::UpdateIMEState().  So, this
-    // instance might already have been destroyed, check it.
-    if (!mRootContent) {
-      return;
-    }
+    // When this is called first time, IME has not received NOTIFY_IME_OF_FOCUS
+    // yet since NOTIFY_IME_OF_FOCUS will be sent to widget asynchronously.
+    // So, we need to do nothing here.  After NOTIFY_IME_OF_FOCUS has been
+    // sent, OnIMEReceivedFocus() will be called and content, selection and/or
+    // position changes will be observed
+    return;
   }
 
+  // When this is called after editor reframing (i.e., the root editable node
+  // is also recreated), IME has usually received NOTIFY_IME_OF_FOCUS.  In this
+  // case, we need to restart to observe content, selection and/or position
+  // changes in new root editable node.
   ObserveEditableNode();
 
+  if (!NeedsToNotifyIMEOfSomething()) {
+    return;
+  }
+
+  // Some change events may wait to notify IME because this was being
+  // initialized.  It is the time to flush them.
+  FlushMergeableNotifications();
+}
+
+void
+IMEContentObserver::OnIMEReceivedFocus()
+{
+  // While Init() notifies IME of focus, pending layout may be flushed
+  // because the notification may cause querying content.  Then, recursive
+  // call of Init() with the latest content may occur.  In such case, we
+  // shouldn't keep first initialization which notified IME of focus.
+  if (GetState() != eState_Initializing) {
+    return;
+  }
+
+  // NOTIFY_IME_OF_FOCUS might cause recreating IMEContentObserver
+  // instance via IMEStateManager::UpdateIMEState().  So, this
+  // instance might already have been destroyed, check it.
+  if (!mRootContent) {
+    return;
+  }
+
+  // Start to observe which is needed by IME when IME actually has focus.
+  ObserveEditableNode();
+
+  if (!NeedsToNotifyIMEOfSomething()) {
+    return;
+  }
+
   // Some change events may wait to notify IME because this was being
   // initialized.  It is the time to flush them.
   FlushMergeableNotifications();
@@ -404,6 +434,18 @@ IMEContentObserver::ObserveEditableNode()
   MOZ_RELEASE_ASSERT(mRootContent);
   MOZ_RELEASE_ASSERT(GetState() != eState_Observing);
 
+  // If this is called before sending NOTIFY_IME_OF_FOCUS (it's possible when
+  // the editor is reframed before sending NOTIFY_IME_OF_FOCUS asynchronously),
+  // the update preference of mWidget may be different from after the widget
+  // receives NOTIFY_IME_OF_FOCUS.   So, this should be called again by
+  // OnIMEReceivedFocus() which is called after sending NOTIFY_IME_OF_FOCUS.
+  if (!mIMEHasFocus) {
+    MOZ_ASSERT(!mWidget || mNeedsToNotifyIMEOfFocusSet ||
+               mSendingNotification == NOTIFY_IME_OF_FOCUS,
+               "Wow, OnIMEReceivedFocus() won't be called?");
+    return;
+  }
+
   mIsObserving = true;
   if (mEditor) {
     mEditor->AddEditorObserver(this);
@@ -1418,10 +1460,25 @@ IMEContentObserver::IMENotificationSender::Run()
   if (mIMEContentObserver->mNeedsToNotifyIMEOfFocusSet) {
     mIMEContentObserver->mNeedsToNotifyIMEOfFocusSet = false;
     SendFocusSet();
+    mIMEContentObserver->mIsFlushingPendingNotifications = false;
+    // If it's not safe to notify IME of focus, SendFocusSet() sets
+    // mNeedsToNotifyIMEOfFocusSet true again.  For guaranteeing to send the
+    // focus notification later,  we should put a new sender into the queue but
+    // this case must be rare.  Note that if mIMEContentObserver is already
+    // destroyed, mNeedsToNotifyIMEOfFocusSet is never set true again.
+    if (mIMEContentObserver->mNeedsToNotifyIMEOfFocusSet) {
+      MOZ_ASSERT(!mIMEContentObserver->mIMEHasFocus);
+      MOZ_LOG(sIMECOLog, LogLevel::Debug,
+        ("IMECO: 0x%p IMEContentObserver::IMENotificationSender::Run(), "
+         "posting AsyncMergeableNotificationsFlusher to current thread", this));
+      RefPtr<AsyncMergeableNotificationsFlusher> asyncFlusher =
+        new AsyncMergeableNotificationsFlusher(mIMEContentObserver);
+      NS_DispatchToCurrentThread(asyncFlusher);
+      return NS_OK;
+    }
     // This is the first notification to IME. So, we don't need to notify
     // anymore since IME starts to query content after it gets focus.
     mIMEContentObserver->ClearPendingNotifications();
-    mIMEContentObserver->mIsFlushingPendingNotifications = false;
     return NS_OK;
   }
 
@@ -1454,10 +1511,10 @@ IMEContentObserver::IMENotificationSender::Run()
     }
   }
 
+  mIMEContentObserver->mIsFlushingPendingNotifications = false;
+
   // If notifications caused some new change, we should notify them now.
-  mIMEContentObserver->mIsFlushingPendingNotifications =
-    mIMEContentObserver->NeedsToNotifyIMEOfSomething();
-  if (mIMEContentObserver->mIsFlushingPendingNotifications) {
+  if (mIMEContentObserver->NeedsToNotifyIMEOfSomething()) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::IMENotificationSender::Run(), "
        "posting AsyncMergeableNotificationsFlusher to current thread", this));
@@ -1505,6 +1562,12 @@ IMEContentObserver::IMENotificationSender::SendFocusSet()
                              mIMEContentObserver->mWidget);
   mIMEContentObserver->mSendingNotification = NOTIFY_IME_OF_NOTHING;
 
+  // nsIMEUpdatePreference referred by ObserveEditableNode() may be different
+  // before or after widget receives NOTIFY_IME_OF_FOCUS.  Therefore, we need
+  // to guarantee to call ObserveEditableNode() after sending
+  // NOTIFY_IME_OF_FOCUS.
+  mIMEContentObserver->OnIMEReceivedFocus();
+
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::IMENotificationSender::"
      "SendFocusSet(), sent NOTIFY_IME_OF_FOCUS", this));
@@ -1675,6 +1738,18 @@ IMEContentObserver::IMENotificationSender::SendPositionChange()
 NS_IMETHODIMP
 IMEContentObserver::AsyncMergeableNotificationsFlusher::Run()
 {
+  // When this is performed, another sender may already have been performed
+  // with nsContentUtils::AddScriptRunner().
+  if (!mIMEContentObserver->NeedsToNotifyIMEOfSomething()) {
+    NS_ASSERTION(!mIMEContentObserver->mIsFlushingPendingNotifications,
+      "It shouldn't be flushing pending notifications now");
+    MOZ_LOG(sIMECOLog, LogLevel::Debug,
+      ("IMECO: 0x%p IMEContentObserver::AsyncMergeableNotificationsFlusher::"
+       "Run(), does nothing, due to already flushed the pending notifications",
+       this));
+    return NS_OK;
+  }
+
   if (!CanNotifyIME(eChangeEventType_FlushPendingEvents)) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::AsyncMergeableNotificationsFlusher::"

dom/events/IMEContentObserver.h

@@ -117,6 +117,7 @@ private:
                       nsIEditor* aEditor);
   bool InitWithPlugin(nsPresContext* aPresContext, nsIContent* aContent);
   bool IsInitializedWithPlugin() const { return !mEditor; }
+  void OnIMEReceivedFocus();
   void Clear();
   bool IsObservingContent(nsPresContext* aPresContext,
                           nsIContent* aContent) const;
@@ -264,6 +265,8 @@ private:
   // mIsFlushingPendingNotifications is true between
   // FlushMergeableNotifications() creates IMENotificationSender and
   // IMENotificationSender sent all pending notifications.
+  // Note this may false even after an AsyncMergeableNotificationsFlusher is
+  // queued into the event loop.  Don't be confused by these difference.
   bool mIsFlushingPendingNotifications;
   // mIsHandlingQueryContentEvent is true when IMEContentObserver is handling
   // WidgetQueryContentEvent with ContentEventHandler.