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 615864 9c6db60f63b9fdc3c9950c38c38048b1ec249b92
parent 615863 54a42c19bd2b46893a57bf8ba522e43da1ef6497
child 615933 34d102bac0db840ecf7bc681c72b50a41991374b
push id70509
push usermasayuki@d-toybox.com
push dateWed, 26 Jul 2017 13:53:45 +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.