Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget. r=m_kato, a=lizzard
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 04 Sep 2017 17:37:40 +0900
changeset 421602 a805179e01649318edd4fb9fe029203241ee25a6
parent 421601 7a6699a52aec8acfd69754f439b6fd0605d827c3
child 421603 77d3e7d3c94b3f54cad68887166741823dff01f3
push id7724
push userryanvm@gmail.com
push dateThu, 07 Sep 2017 19:01:32 +0000
treeherdermozilla-beta@5df6d9e6c8ae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, lizzard
bugs1396302
milestone56.0
Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget. r=m_kato, a=lizzard In some cases, sFocusedIMEWidget may be nullptr but oldWidget still has composition since IMEStateManager doesn't guarantee that NOTIFY_IME_OF_BLUR is sent after REQUEST_TO_COMMIT_COMPOSITION nor REQUEST_TO_CANCEL_COMPOSITION. Therefore, when it tries to clean up old widget's composition, it should refer the old widget's IME notification requests. MozReview-Commit-ID: 8kZvJbHfs5z
dom/events/IMEStateManager.cpp
--- a/dom/events/IMEStateManager.cpp
+++ b/dom/events/IMEStateManager.cpp
@@ -261,16 +261,27 @@ IMEStateManager::NotifyIMEOfBlurForChild
      sFocusedIMETabParent.get(), sFocusedIMEWidget));
 
   if (!sFocusedIMETabParent) {
     MOZ_ASSERT(!sFocusedIMEWidget);
     return;
   }
 
   MOZ_ASSERT(sFocusedIMEWidget);
+
+  if (MOZ_LOG_TEST(sISMLog, LogLevel::Debug) && sTextCompositions) {
+    RefPtr<TextComposition> composition =
+      sTextCompositions->GetCompositionFor(sFocusedIMEWidget);
+    if (composition) {
+      MOZ_LOG(sISMLog, LogLevel::Debug,
+        ("  NotifyIMEOfBlurForChildProcess(), sFocusedIMEWidget still has "
+         "composition"));
+    }
+  }
+
   NotifyIME(NOTIFY_IME_OF_BLUR, sFocusedIMEWidget, sFocusedIMETabParent);
 
   MOZ_ASSERT(!sFocusedIMETabParent);
   MOZ_ASSERT(!sFocusedIMEWidget);
 }
 
 // static
 void
@@ -481,19 +492,22 @@ IMEStateManager::OnChangeFocusInternal(n
   // a native IME context is shared on all editors on some widgets or all
   // widgets (it depends on platforms).
   if (oldWidget && focusActuallyChanging && sTextCompositions) {
     RefPtr<TextComposition> composition =
       sTextCompositions->GetCompositionFor(oldWidget);
     if (composition) {
       // However, don't commit the composition if we're being inactivated
       // but the composition should be kept even during deactive.
+      // Note that oldWidget and sFocusedIMEWidget may be different here (in
+      // such case, sFocusedIMEWidget is perhaps nullptr).  For example, IME
+      // may receive only blur notification but still has composition.
+      // We need to clean up only the oldWidget's composition state here.
       if (aPresContext ||
-          !sFocusedIMEWidget->IMENotificationRequestsRef().
-           WantDuringDeactive()) {
+          !oldWidget->IMENotificationRequestsRef().WantDuringDeactive()) {
         MOZ_LOG(sISMLog, LogLevel::Info,
           ("  OnChangeFocusInternal(), requesting to commit composition to "
            "the (previous) focused widget"));
         NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, oldWidget,
                   composition->GetTabParent());
       }
     }
   }