From 190b2c2740742b8261d3c62eaac461df4dee79c3 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 13 Oct 2020 23:29:29 +0000 Subject: [PATCH] Bug 1612802 - Make `IMEStateManager` stop notifying `PuppetWidget` of editor state changes while its `StopIMEStateManagement()` r=m_kato Currently, `BrowserParent` rejects any notifications which come after it blurs from IME focus. https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/dom/ipc/BrowserParent.cpp#2157,2169,2181,2192,2203,2216 But for saving unnecessary IPC messages, we should make `IMEStateManager` in a content process stop sending notifications which occurs during its `StopIMEStateManagement()` is called. Differential Revision: https://phabricator.services.mozilla.com/D93176 --- dom/events/IMEContentObserver.cpp | 3 +++ dom/events/IMEStateManager.cpp | 27 ++++++++++++++++++++++++--- dom/events/IMEStateManager.h | 13 +++++++++++++ dom/events/TextComposition.cpp | 2 +- widget/PuppetWidget.cpp | 9 +++++++++ 5 files changed, 50 insertions(+), 4 deletions(-) diff --git a/dom/events/IMEContentObserver.cpp b/dom/events/IMEContentObserver.cpp index 0b38a70b0203..7163913d2cf2 100644 --- a/dom/events/IMEContentObserver.cpp +++ b/dom/events/IMEContentObserver.cpp @@ -476,6 +476,9 @@ nsPresContext* IMEContentObserver::GetPresContext() const { void IMEContentObserver::Destroy() { // WARNING: When you change this method, you have to check Unlink() too. + // Note that don't send any notifications later from here. I.e., notify + // IMEStateManager of the blur synchronously because IMEStateManager needs to + // stop notifying the main process if this is requested by the main process. NotifyIMEOfBlur(); UnregisterObservers(); Clear(); diff --git a/dom/events/IMEStateManager.cpp b/dom/events/IMEStateManager.cpp index e79053bcfd79..576a9a54d71f 100644 --- a/dom/events/IMEStateManager.cpp +++ b/dom/events/IMEStateManager.cpp @@ -9,6 +9,7 @@ #include "mozilla/IMEStateManager.h" #include "mozilla/Attributes.h" +#include "mozilla/AutoRestore.h" #include "mozilla/EditorBase.h" #include "mozilla/EventListenerManager.h" #include "mozilla/EventStates.h" @@ -76,6 +77,7 @@ InputContext::Origin IMEStateManager::sOrigin = InputContext::ORIGIN_MAIN; InputContext IMEStateManager::sActiveChildInputContext; bool IMEStateManager::sInstalledMenuKeyboardListener = false; bool IMEStateManager::sIsGettingNewIMEState = false; +bool IMEStateManager::sCleaningUpForStoppingIMEStateManagement = false; // static void IMEStateManager::Init() { @@ -203,6 +205,15 @@ void IMEStateManager::StopIMEStateManagement() { // NOTE: Don't set input context from here since this has already lost // the rights to change input context. + // The requestee of this method in the main process must destroy its + // active IMEContentObserver for making existing composition end and + // make it be possible to start new composition in new focused process. + // Therefore, we shouldn't notify the main process of any changes which + // occurred after here. + AutoRestore restoreStoppingIMEStateManagementState( + sCleaningUpForStoppingIMEStateManagement); + sCleaningUpForStoppingIMEStateManagement = true; + if (sTextCompositions && sPresContext) { NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, sPresContext, nullptr); } @@ -1550,12 +1561,14 @@ nsresult IMEStateManager::NotifyIME(const IMENotification& aNotification, "aWidget=0x%p, aBrowserParent=0x%p), sFocusedIMEWidget=0x%p, " "BrowserParent::GetFocused()=0x%p, sFocusedIMEBrowserParent=0x%p, " "aBrowserParent == BrowserParent::GetFocused()=%s, " - "aBrowserParent == sFocusedIMEBrowserParent=%s", + "aBrowserParent == sFocusedIMEBrowserParent=%s, " + "CanSendNotificationToTheMainProcess()=%s", ToChar(aNotification.mMessage), aWidget, aBrowserParent, sFocusedIMEWidget, BrowserParent::GetFocused(), sFocusedIMEBrowserParent.get(), GetBoolName(aBrowserParent == BrowserParent::GetFocused()), - GetBoolName(aBrowserParent == sFocusedIMEBrowserParent))); + GetBoolName(aBrowserParent == sFocusedIMEBrowserParent), + GetBoolName(CanSendNotificationToTheMainProcess()))); if (NS_WARN_IF(!aWidget)) { MOZ_LOG(sISMLog, LogLevel::Error, @@ -1565,6 +1578,8 @@ nsresult IMEStateManager::NotifyIME(const IMENotification& aNotification, switch (aNotification.mMessage) { case NOTIFY_IME_OF_FOCUS: { + MOZ_ASSERT(CanSendNotificationToTheMainProcess()); + // If focus notification comes from a remote browser which already lost // focus, we shouldn't accept the focus notification. Then, following // notifications from the process will be ignored. @@ -1632,7 +1647,10 @@ nsresult IMEStateManager::NotifyIME(const IMENotification& aNotification, nsCOMPtr focusedIMEWidget(sFocusedIMEWidget); sFocusedIMEWidget = nullptr; sFocusedIMEBrowserParent = nullptr; - return focusedIMEWidget->NotifyIME(IMENotification(NOTIFY_IME_OF_BLUR)); + return CanSendNotificationToTheMainProcess() + ? focusedIMEWidget->NotifyIME( + IMENotification(NOTIFY_IME_OF_BLUR)) + : NS_OK; } case NOTIFY_IME_OF_SELECTION_CHANGE: case NOTIFY_IME_OF_TEXT_CHANGE: @@ -1660,6 +1678,9 @@ nsresult IMEStateManager::NotifyIME(const IMENotification& aNotification, "is ignored because it's not for current focused IME widget")); return NS_OK; } + if (!CanSendNotificationToTheMainProcess()) { + return NS_OK; + } nsCOMPtr widget(aWidget); return widget->NotifyIME(aNotification); } diff --git a/dom/events/IMEStateManager.h b/dom/events/IMEStateManager.h index f4756120d2ff..a947f0bf8b9b 100644 --- a/dom/events/IMEStateManager.h +++ b/dom/events/IMEStateManager.h @@ -78,6 +78,15 @@ class IMEStateManager { return sFocusedIMEBrowserParent == aBrowserParent; } + /** + * If CanSendNotificationToTheMainProcess() returns false (it should occur + * only in a content process), we shouldn't notify the main process of + * any focused editor changes since the content process was blurred. + */ + static bool CanSendNotificationToTheMainProcess() { + return !sCleaningUpForStoppingIMEStateManagement; + } + /** * Focus moved between browsers from aBlur to aFocus. (nullptr means the * chrome process.) @@ -369,6 +378,10 @@ class IMEStateManager { static bool sIsGettingNewIMEState; static bool sCheckForIMEUnawareWebApps; + // Set to true only if this is an instance in a content process and + // only while `IMEStateManager::StopIMEStateManagement()`. + static bool sCleaningUpForStoppingIMEStateManagement; + class MOZ_STACK_CLASS GettingNewIMEStateBlocker final { public: GettingNewIMEStateBlocker() diff --git a/dom/events/TextComposition.cpp b/dom/events/TextComposition.cpp index 311108e570ed..51f53cd29605 100644 --- a/dom/events/TextComposition.cpp +++ b/dom/events/TextComposition.cpp @@ -563,7 +563,7 @@ nsresult TextComposition::RequestToCommit(nsIWidget* aWidget, bool aDiscard) { RefPtr kungFuDeathGrip(this); const nsAutoString lastData(mLastData); - { + if (IMEStateManager::CanSendNotificationToTheMainProcess()) { AutoRestore saveRequestingCancel(mIsRequestingCancel); AutoRestore saveRequestingCommit(mIsRequestingCommit); if (aDiscard) { diff --git a/widget/PuppetWidget.cpp b/widget/PuppetWidget.cpp index 2891b469fb90..ca8209fdcede 100644 --- a/widget/PuppetWidget.cpp +++ b/widget/PuppetWidget.cpp @@ -743,6 +743,8 @@ NativeIMEContext PuppetWidget::GetNativeIMEContext() { nsresult PuppetWidget::NotifyIMEOfFocusChange( const IMENotification& aIMENotification) { + MOZ_ASSERT(IMEStateManager::CanSendNotificationToTheMainProcess()); + if (!mBrowserChild) { return NS_ERROR_FAILURE; } @@ -789,6 +791,8 @@ nsresult PuppetWidget::NotifyIMEOfFocusChange( nsresult PuppetWidget::NotifyIMEOfCompositionUpdate( const IMENotification& aIMENotification) { + MOZ_ASSERT(IMEStateManager::CanSendNotificationToTheMainProcess()); + if (NS_WARN_IF(!mBrowserChild)) { return NS_ERROR_FAILURE; } @@ -804,8 +808,10 @@ nsresult PuppetWidget::NotifyIMEOfCompositionUpdate( nsresult PuppetWidget::NotifyIMEOfTextChange( const IMENotification& aIMENotification) { + MOZ_ASSERT(IMEStateManager::CanSendNotificationToTheMainProcess()); MOZ_ASSERT(aIMENotification.mMessage == NOTIFY_IME_OF_TEXT_CHANGE, "Passed wrong notification"); + if (!mBrowserChild) { return NS_ERROR_FAILURE; } @@ -835,6 +841,7 @@ nsresult PuppetWidget::NotifyIMEOfTextChange( nsresult PuppetWidget::NotifyIMEOfSelectionChange( const IMENotification& aIMENotification) { + MOZ_ASSERT(IMEStateManager::CanSendNotificationToTheMainProcess()); MOZ_ASSERT(aIMENotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE, "Passed wrong notification"); if (!mBrowserChild) { @@ -862,6 +869,7 @@ nsresult PuppetWidget::NotifyIMEOfSelectionChange( nsresult PuppetWidget::NotifyIMEOfMouseButtonEvent( const IMENotification& aIMENotification) { + MOZ_ASSERT(IMEStateManager::CanSendNotificationToTheMainProcess()); if (!mBrowserChild) { return NS_ERROR_FAILURE; } @@ -883,6 +891,7 @@ nsresult PuppetWidget::NotifyIMEOfMouseButtonEvent( nsresult PuppetWidget::NotifyIMEOfPositionChange( const IMENotification& aIMENotification) { + MOZ_ASSERT(IMEStateManager::CanSendNotificationToTheMainProcess()); if (NS_WARN_IF(!mBrowserChild)) { return NS_ERROR_FAILURE; }