From e16aa473be820042f4dd1e72c4678e38a1f8b55d Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 10 Nov 2015 11:49:04 +0900 Subject: [PATCH] Bug 1218032 part.2 Make sending focus notification to IME async-aware r=smaug --- dom/events/IMEContentObserver.cpp | 113 +++++++++++++++++++++++++----- dom/events/IMEContentObserver.h | 3 + 2 files changed, 97 insertions(+), 19 deletions(-) diff --git a/dom/events/IMEContentObserver.cpp b/dom/events/IMEContentObserver.cpp index 79c5c4e9a93c..5f72cd59cda6 100644 --- a/dom/events/IMEContentObserver.cpp +++ b/dom/events/IMEContentObserver.cpp @@ -251,26 +251,56 @@ IMEContentObserver::Init(nsIWidget* aWidget, } if (firstInitialization) { + // Now, try to send NOTIFY_IME_OF_FOCUS to IME via the widget. MaybeNotifyIMEOfFocusSet(); - - // While Init() notifies IME of focus, pending layout may be flushed - // because the notification may cause querying content. Then, recursive - // call of Init() with the latest content may be occur. In such case, we - // shouldn't keep first initialization. - if (GetState() != eState_Initializing) { - return; - } - - // NOTIFY_IME_OF_FOCUS might cause recreating IMEContentObserver - // instance via IMEStateManager::UpdateIMEState(). So, this - // instance might already have been destroyed, check it. - if (!mRootContent) { - return; - } + // When this is called first time, IME has not received NOTIFY_IME_OF_FOCUS + // yet since NOTIFY_IME_OF_FOCUS will be sent to widget asynchronously. + // So, we need to do nothing here. After NOTIFY_IME_OF_FOCUS has been + // sent, OnIMEReceivedFocus() will be called and content, selection and/or + // position changes will be observed + return; } + // When this is called after editor reframing (i.e., the root editable node + // is also recreated), IME has usually received NOTIFY_IME_OF_FOCUS. In this + // case, we need to restart to observe content, selection and/or position + // changes in new root editable node. ObserveEditableNode(); + if (!NeedsToNotifyIMEOfSomething()) { + return; + } + + // Some change events may wait to notify IME because this was being + // initialized. It is the time to flush them. + FlushMergeableNotifications(); +} + +void +IMEContentObserver::OnIMEReceivedFocus() +{ + // While Init() notifies IME of focus, pending layout may be flushed + // because the notification may cause querying content. Then, recursive + // call of Init() with the latest content may occur. In such case, we + // shouldn't keep first initialization which notified IME of focus. + if (GetState() != eState_Initializing) { + return; + } + + // NOTIFY_IME_OF_FOCUS might cause recreating IMEContentObserver + // instance via IMEStateManager::UpdateIMEState(). So, this + // instance might already have been destroyed, check it. + if (!mRootContent) { + return; + } + + // Start to observe which is needed by IME when IME actually has focus. + ObserveEditableNode(); + + if (!NeedsToNotifyIMEOfSomething()) { + return; + } + // Some change events may wait to notify IME because this was being // initialized. It is the time to flush them. FlushMergeableNotifications(); @@ -404,6 +434,18 @@ IMEContentObserver::ObserveEditableNode() MOZ_RELEASE_ASSERT(mRootContent); MOZ_RELEASE_ASSERT(GetState() != eState_Observing); + // If this is called before sending NOTIFY_IME_OF_FOCUS (it's possible when + // the editor is reframed before sending NOTIFY_IME_OF_FOCUS asynchronously), + // the update preference of mWidget may be different from after the widget + // receives NOTIFY_IME_OF_FOCUS. So, this should be called again by + // OnIMEReceivedFocus() which is called after sending NOTIFY_IME_OF_FOCUS. + if (!mIMEHasFocus) { + MOZ_ASSERT(!mWidget || mNeedsToNotifyIMEOfFocusSet || + mSendingNotification == NOTIFY_IME_OF_FOCUS, + "Wow, OnIMEReceivedFocus() won't be called?"); + return; + } + mIsObserving = true; if (mEditor) { mEditor->AddEditorObserver(this); @@ -1418,10 +1460,25 @@ IMEContentObserver::IMENotificationSender::Run() if (mIMEContentObserver->mNeedsToNotifyIMEOfFocusSet) { mIMEContentObserver->mNeedsToNotifyIMEOfFocusSet = false; SendFocusSet(); + mIMEContentObserver->mIsFlushingPendingNotifications = false; + // If it's not safe to notify IME of focus, SendFocusSet() sets + // mNeedsToNotifyIMEOfFocusSet true again. For guaranteeing to send the + // focus notification later, we should put a new sender into the queue but + // this case must be rare. Note that if mIMEContentObserver is already + // destroyed, mNeedsToNotifyIMEOfFocusSet is never set true again. + if (mIMEContentObserver->mNeedsToNotifyIMEOfFocusSet) { + MOZ_ASSERT(!mIMEContentObserver->mIMEHasFocus); + MOZ_LOG(sIMECOLog, LogLevel::Debug, + ("IMECO: 0x%p IMEContentObserver::IMENotificationSender::Run(), " + "posting AsyncMergeableNotificationsFlusher to current thread", this)); + RefPtr asyncFlusher = + new AsyncMergeableNotificationsFlusher(mIMEContentObserver); + NS_DispatchToCurrentThread(asyncFlusher); + return NS_OK; + } // This is the first notification to IME. So, we don't need to notify // anymore since IME starts to query content after it gets focus. mIMEContentObserver->ClearPendingNotifications(); - mIMEContentObserver->mIsFlushingPendingNotifications = false; return NS_OK; } @@ -1454,10 +1511,10 @@ IMEContentObserver::IMENotificationSender::Run() } } + mIMEContentObserver->mIsFlushingPendingNotifications = false; + // If notifications caused some new change, we should notify them now. - mIMEContentObserver->mIsFlushingPendingNotifications = - mIMEContentObserver->NeedsToNotifyIMEOfSomething(); - if (mIMEContentObserver->mIsFlushingPendingNotifications) { + if (mIMEContentObserver->NeedsToNotifyIMEOfSomething()) { MOZ_LOG(sIMECOLog, LogLevel::Debug, ("IMECO: 0x%p IMEContentObserver::IMENotificationSender::Run(), " "posting AsyncMergeableNotificationsFlusher to current thread", this)); @@ -1505,6 +1562,12 @@ IMEContentObserver::IMENotificationSender::SendFocusSet() mIMEContentObserver->mWidget); mIMEContentObserver->mSendingNotification = NOTIFY_IME_OF_NOTHING; + // nsIMEUpdatePreference referred by ObserveEditableNode() may be different + // before or after widget receives NOTIFY_IME_OF_FOCUS. Therefore, we need + // to guarantee to call ObserveEditableNode() after sending + // NOTIFY_IME_OF_FOCUS. + mIMEContentObserver->OnIMEReceivedFocus(); + MOZ_LOG(sIMECOLog, LogLevel::Debug, ("IMECO: 0x%p IMEContentObserver::IMENotificationSender::" "SendFocusSet(), sent NOTIFY_IME_OF_FOCUS", this)); @@ -1675,6 +1738,18 @@ IMEContentObserver::IMENotificationSender::SendPositionChange() NS_IMETHODIMP IMEContentObserver::AsyncMergeableNotificationsFlusher::Run() { + // When this is performed, another sender may already have been performed + // with nsContentUtils::AddScriptRunner(). + if (!mIMEContentObserver->NeedsToNotifyIMEOfSomething()) { + NS_ASSERTION(!mIMEContentObserver->mIsFlushingPendingNotifications, + "It shouldn't be flushing pending notifications now"); + MOZ_LOG(sIMECOLog, LogLevel::Debug, + ("IMECO: 0x%p IMEContentObserver::AsyncMergeableNotificationsFlusher::" + "Run(), does nothing, due to already flushed the pending notifications", + this)); + return NS_OK; + } + if (!CanNotifyIME(eChangeEventType_FlushPendingEvents)) { MOZ_LOG(sIMECOLog, LogLevel::Debug, ("IMECO: 0x%p IMEContentObserver::AsyncMergeableNotificationsFlusher::" diff --git a/dom/events/IMEContentObserver.h b/dom/events/IMEContentObserver.h index 785b73d7c1ca..46ee0bb0b6c5 100644 --- a/dom/events/IMEContentObserver.h +++ b/dom/events/IMEContentObserver.h @@ -117,6 +117,7 @@ private: nsIEditor* aEditor); bool InitWithPlugin(nsPresContext* aPresContext, nsIContent* aContent); bool IsInitializedWithPlugin() const { return !mEditor; } + void OnIMEReceivedFocus(); void Clear(); bool IsObservingContent(nsPresContext* aPresContext, nsIContent* aContent) const; @@ -264,6 +265,8 @@ private: // mIsFlushingPendingNotifications is true between // FlushMergeableNotifications() creates IMENotificationSender and // IMENotificationSender sent all pending notifications. + // Note this may false even after an AsyncMergeableNotificationsFlusher is + // queued into the event loop. Don't be confused by these difference. bool mIsFlushingPendingNotifications; // mIsHandlingQueryContentEvent is true when IMEContentObserver is handling // WidgetQueryContentEvent with ContentEventHandler.