Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 04 Sep 2017 17:37:40 +0900
changeset 658487 8fd74fec647296f6011f28841ffe1f9b3f35adf0
parent 658358 8e05298328da75f3056a9f1f9609938870d756a0
child 729678 a7af98a0f18ffa4089e64162d914d1e1406b2feb
push id77795
push usermasayuki@d-toybox.com
push dateMon, 04 Sep 2017 08:39:36 +0000
reviewersm_kato
bugs1396302
milestone57.0a1
Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget r?m_kato 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());
       }
     }
   }