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=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 26 Jul 2017 00:57:29 +0900
changeset 420347 258d81d739533da1d4753741119bfd18c2176483
parent 420346 51919d68802ec622a8c24a5e839e046f57f66405
child 420348 dc839a86967dafca35021b3744f6dfb39264179f
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1384027
milestone56.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 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=m_kato 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
@@ -192,38 +192,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
@@ -243,16 +248,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;
@@ -483,23 +509,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.