From 0fa4a64aa3605c805514a03cb92ba71263a6a456 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 26 Jul 2017 00:57:29 +0900 Subject: [PATCH] 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 --HG-- extra : rebase_source : c3de0df442420979414b47d8d20c7988c49b205b --- dom/events/IMEStateManager.cpp | 40 ++++++++++++++++++++++++++++++---- dom/events/IMEStateManager.h | 6 +++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/dom/events/IMEStateManager.cpp b/dom/events/IMEStateManager.cpp index 9b2944496468..70e1bb78a536 100644 --- a/dom/events/IMEStateManager.cpp +++ b/dom/events/IMEStateManager.cpp @@ -197,9 +197,14 @@ IMEStateManager::Shutdown() 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)); @@ -207,8 +212,7 @@ IMEStateManager::OnTabParentDestroying(TabParent* 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 @@ -219,6 +223,7 @@ IMEStateManager::WidgetDestroyed(nsIWidget* aWidget) sWidget = nullptr; } if (sFocusedIMEWidget == aWidget) { + NotifyIMEOfBlurForChildProcess(); sFocusedIMEWidget = nullptr; } if (sActiveInputContextWidget == aWidget) { @@ -246,6 +251,27 @@ IMEStateManager::StopIMEStateManagement() 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, @@ -488,13 +514,19 @@ IMEStateManager::OnChangeFocusInternal(nsPresContext* aPresContext, // 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(); } } diff --git a/dom/events/IMEStateManager.h b/dom/events/IMEStateManager.h index a11b8a8ce0a1..89a60c5b7e4d 100644 --- a/dom/events/IMEStateManager.h +++ b/dom/events/IMEStateManager.h @@ -274,6 +274,12 @@ protected: 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);