Bug 1384027 - part3: Don't send blur notification to IME from IMEStateManager::OnChangeFocusInternal() if no window becomes active and IME wants to keep composition during deactive r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 26 Jul 2017 00:57:29 +0900
changeset 615826 19f938edcc70581d1888129f1290eca9ae7f07d2
parent 615825 a39d959c1bb0a929bc4968289ccc086115ce8054
child 639288 53e6f49f2bf3cec15e8f50e0c96443bd12537ace
push id70483
push usermasayuki@d-toybox.com
push dateWed, 26 Jul 2017 10:53:21 +0000
reviewerssmaug
bugs1384027
milestone56.0a1
Bug 1384027 - part3: Don't send blur notification to IME from IMEStateManager::OnChangeFocusInternal() if no window becomes active and IME wants to keep composition during deactive r?smaug Currently, IMEStateManager::OnChangeFocusInternal() sends blur notification to IME when a remote process has IME focus and focus is moving from the process. However, if IME wants to keep composition even during deactive and nobody will gets focus (i.e., all windows becomes deactive), IMEStateManager shouldn't send the blur notification because it causes committing composition. Therefore, it should send blur notification only when focus is moving to a PresContext (that means that not all windows becomes deactive) or IME doesn't want to keep composition during deactive. Then, even if another window becomes active next time, IMEStateManager can send "stop IME state management" message to the composing remote process and the remote process can commit composition normally. Additionally, this patch ensures to send blur notification when IME focused TabParent or widget is being destroyed. This fixes new memory leak bug of this patch (sFocusedIMETabParent keeps grabbing the instance until shutting down in some mochitests). MozReview-Commit-ID: KYiFGo970a8
dom/events/IMEStateManager.cpp
dom/events/IMEStateManager.h
--- a/dom/events/IMEStateManager.cpp
+++ b/dom/events/IMEStateManager.cpp
@@ -193,38 +193,43 @@ IMEStateManager::Shutdown()
   delete sTextCompositions;
   sTextCompositions = nullptr;
 }
 
 // static
 void
 IMEStateManager::OnTabParentDestroying(TabParent* aTabParent)
 {
+  if (sFocusedIMETabParent == aTabParent) {
+    NotifyIMEOfBlurForChildProcess();
+  }
+
   if (sActiveTabParent != aTabParent) {
     return;
   }
+
   MOZ_LOG(sISMLog, LogLevel::Info,
     ("OnTabParentDestroying(aTabParent=0x%p), "
      "The active TabParent is being destroyed", aTabParent));
 
   // The active remote process might have crashed.
   sActiveTabParent = nullptr;
 
-  // TODO: Need to cancel composition without TextComposition and make
-  //       disable IME.
+  // XXX: Need to disable IME?
 }
 
 // static
 void
 IMEStateManager::WidgetDestroyed(nsIWidget* aWidget)
 {
   if (sWidget == aWidget) {
     sWidget = nullptr;
   }
   if (sFocusedIMEWidget == aWidget) {
+    NotifyIMEOfBlurForChildProcess();
     sFocusedIMEWidget = nullptr;
   }
   if (sActiveInputContextWidget == aWidget) {
     sActiveInputContextWidget = nullptr;
   }
 }
 
 // static
@@ -244,16 +249,37 @@ IMEStateManager::StopIMEStateManagement(
   sPresContext = nullptr;
   sContent = nullptr;
   sActiveTabParent = nullptr;
   DestroyIMEContentObserver();
 }
 
 // static
 void
+IMEStateManager::NotifyIMEOfBlurForChildProcess()
+{
+  MOZ_LOG(sISMLog, LogLevel::Debug,
+    ("NotifyIMEOfBlurForChildProcess(), sFocusedIMETabParent=0x%p, "
+     "sFocusedIMEWidget=0x%p",
+     sFocusedIMETabParent.get(), sFocusedIMEWidget));
+
+  if (!sFocusedIMETabParent) {
+    MOZ_ASSERT(!sFocusedIMEWidget);
+    return;
+  }
+
+  MOZ_ASSERT(sFocusedIMEWidget);
+  NotifyIME(NOTIFY_IME_OF_BLUR, sFocusedIMEWidget, sFocusedIMETabParent);
+
+  MOZ_ASSERT(!sFocusedIMETabParent);
+  MOZ_ASSERT(!sFocusedIMEWidget);
+}
+
+// static
+void
 IMEStateManager::MaybeStartOffsetUpdatedInChild(nsIWidget* aWidget,
                                                 uint32_t aStartOffset)
 {
   if (NS_WARN_IF(!sTextCompositions)) {
     MOZ_LOG(sISMLog, LogLevel::Warning,
       ("MaybeStartOffsetUpdatedInChild(aWidget=0x%p, aStartOffset=%u), "
        "called when there is no composition", aWidget, aStartOffset));
     return;
@@ -484,23 +510,29 @@ IMEStateManager::OnChangeFocusInternal(n
   } else {
     // If there is no active IMEContentObserver, it means that focused content
     // may be in another process.
 
     // If focus is moving from current focused remote process to different
     // process while the process has IME focus too, we need to notify IME of
     // blur here because it may be too late the blur notification to reach
     // this process especially when closing active window.
-    if (sFocusedIMETabParent &&
+    // However, don't send blur if we're being deactivated and IME wants to
+    // keep composition during deactive because notifying blur will commit
+    // or cancel composition.
+    if (sFocusedIMETabParent && sFocusedIMEWidget &&
+        (aPresContext ||
+         !sFocusedIMEWidget->IMENotificationRequestsRef().
+           WantDuringDeactive()) &&
         !IsSameProcess(sFocusedIMETabParent, newTabParent)) {
       MOZ_LOG(sISMLog, LogLevel::Info,
         ("  OnChangeFocusInternal(), notifying IME of blur of previous focused "
          "remote process because it may be too late actual notification to "
          "reach this process"));
-      NotifyIME(NOTIFY_IME_OF_BLUR, sFocusedIMEWidget, sFocusedIMETabParent);
+      NotifyIMEOfBlurForChildProcess();
     }
   }
 
   if (!aPresContext) {
     MOZ_LOG(sISMLog, LogLevel::Debug,
       ("  OnChangeFocusInternal(), "
        "no nsPresContext is being activated"));
     return NS_OK;
--- a/dom/events/IMEStateManager.h
+++ b/dom/events/IMEStateManager.h
@@ -269,16 +269,22 @@ protected:
                               const InputContextAction& aAction);
   static IMEState GetNewIMEState(nsPresContext* aPresContext,
                                  nsIContent* aContent);
 
   static void EnsureTextCompositionArray();
   static void CreateIMEContentObserver(EditorBase* aEditorBase);
   static void DestroyIMEContentObserver();
 
+  /**
+   * NotifyIMEOfBlurForChildProcess() tries to send blur notification when
+   * a remote process has IME focus.  Otherwise, do nothing.
+   */
+  static void NotifyIMEOfBlurForChildProcess();
+
   static bool IsEditable(nsINode* node);
 
   static bool IsIMEObserverNeeded(const IMEState& aState);
 
   static nsIContent* GetRootContent(nsPresContext* aPresContext);
 
   /**
    * CanHandleWith() returns false if aPresContext is nullptr or it's destroyed.