From db82a0196769e8c248940b587897507cbb449980 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Sat, 11 Jul 2015 10:53:55 +0900 Subject: [PATCH] Bug 1176954 part.3 Don't send selection change, text change nor composition update notification to IME from TabParent until all events sent to the child process is received by it r=smaug --- dom/ipc/TabParent.cpp | 18 +++++--- widget/ContentCache.cpp | 95 ++++++++++++++++++++++++++++++++++++----- widget/ContentCache.h | 27 +++++++----- widget/nsIWidget.h | 60 +++++++++++++++++++++++++- 4 files changed, 171 insertions(+), 29 deletions(-) diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 9a0d315f833..de9e7b893f6 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -1996,7 +1996,7 @@ TabParent::RecvNotifyIMETextChange(const ContentCache& aContentCache, notification.mTextChangeData.mCausedByComposition = aCausedByComposition; mContentCache.AssignContent(aContentCache, ¬ification); - IMEStateManager::NotifyIME(notification, widget, true); + mContentCache.MaybeNotifyIME(widget, notification); return true; } @@ -2011,8 +2011,7 @@ TabParent::RecvNotifyIMESelectedCompositionRect( IMENotification notification(NOTIFY_IME_OF_COMPOSITION_UPDATE); mContentCache.AssignContent(aContentCache, ¬ification); - - IMEStateManager::NotifyIME(notification, widget, true); + mContentCache.MaybeNotifyIME(widget, notification); return true; } @@ -2032,10 +2031,9 @@ TabParent::RecvNotifyIMESelection(const ContentCache& aContentCache, if (updatePreference.WantSelectionChange() && (updatePreference.WantChangesCausedByComposition() || !aCausedByComposition)) { - mContentCache.InitNotification(notification); notification.mSelectionChangeData.mCausedByComposition = aCausedByComposition; - IMEStateManager::NotifyIME(notification, widget, true); + mContentCache.MaybeNotifyIME(widget, notification); } return true; } @@ -2092,7 +2090,15 @@ TabParent::RecvOnEventNeedingAckReceived() { // This is called when the child process receives WidgetCompositionEvent or // WidgetSelectionEvent. - mContentCache.OnEventNeedingAckReceived(); + nsCOMPtr widget = GetWidget(); + if (!widget) { + return true; + } + + // While calling OnEventNeedingAckReceived(), TabParent *might* be destroyed + // since it may send notifications to IME. + nsRefPtr kungFuDeathGrip(this); + mContentCache.OnEventNeedingAckReceived(widget); return true; } diff --git a/widget/ContentCache.cpp b/widget/ContentCache.cpp index fcd8920646c..30cfc08f827 100644 --- a/widget/ContentCache.cpp +++ b/widget/ContentCache.cpp @@ -914,15 +914,22 @@ ContentCacheInParent::OnSelectionEvent( } void -ContentCacheInParent::OnEventNeedingAckReceived() +ContentCacheInParent::OnEventNeedingAckReceived(nsIWidget* aWidget) { + // This is called when the child process receives WidgetCompositionEvent or + // WidgetSelectionEvent. + MOZ_LOG(sContentCacheLog, LogLevel::Info, - ("ContentCacheInParent: 0x%p OnEventNeedingAckReceived(), " + ("ContentCacheInParent: 0x%p OnEventNeedingAckReceived(aWidget=0x%p), " "mPendingEventsNeedingAck=%u", - this, mPendingEventsNeedingAck)); + this, aWidget, mPendingEventsNeedingAck)); MOZ_RELEASE_ASSERT(mPendingEventsNeedingAck > 0); - mPendingEventsNeedingAck--; + if (--mPendingEventsNeedingAck) { + return; + } + + FlushPendingNotifications(aWidget); } uint32_t @@ -951,15 +958,83 @@ ContentCacheInParent::RequestToCommitComposition(nsIWidget* aWidget, } void -ContentCacheInParent::InitNotification(IMENotification& aNotification) const +ContentCacheInParent::MaybeNotifyIME(nsIWidget* aWidget, + IMENotification& aNotification) { - if (NS_WARN_IF(aNotification.mMessage != NOTIFY_IME_OF_SELECTION_CHANGE)) { + if (aNotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE) { + aNotification.mSelectionChangeData.mOffset = mSelection.StartOffset(); + aNotification.mSelectionChangeData.mLength = mSelection.Length(); + aNotification.mSelectionChangeData.mReversed = mSelection.Reversed(); + aNotification.mSelectionChangeData.SetWritingMode(mSelection.mWritingMode); + } + + if (!mPendingEventsNeedingAck) { + IMEStateManager::NotifyIME(aNotification, aWidget, true); return; } - aNotification.mSelectionChangeData.mOffset = mSelection.StartOffset(); - aNotification.mSelectionChangeData.mLength = mSelection.Length(); - aNotification.mSelectionChangeData.mReversed = mSelection.Reversed(); - aNotification.mSelectionChangeData.SetWritingMode(mSelection.mWritingMode); + + switch (aNotification.mMessage) { + case NOTIFY_IME_OF_SELECTION_CHANGE: + mPendingSelectionChange.MergeWith(aNotification); + break; + case NOTIFY_IME_OF_TEXT_CHANGE: + mPendingTextChange.MergeWith(aNotification); + break; + case NOTIFY_IME_OF_COMPOSITION_UPDATE: + mPendingCompositionUpdate.MergeWith(aNotification); + break; + default: + MOZ_CRASH("Unsupported notification"); + break; + } +} + +void +ContentCacheInParent::FlushPendingNotifications(nsIWidget* aWidget) +{ + MOZ_ASSERT(!mPendingEventsNeedingAck); + + // New notifications which are notified during flushing pending notifications + // should be merged again. + mPendingEventsNeedingAck++; + + nsCOMPtr kungFuDeathGrip(aWidget); + + // First, text change notification should be sent because selection change + // notification notifies IME of current selection range in the latest content. + // So, IME may need the latest content before that. + if (mPendingTextChange.HasNotification()) { + IMENotification notification(mPendingTextChange); + if (!aWidget->Destroyed()) { + mPendingTextChange.Clear(); + IMEStateManager::NotifyIME(notification, aWidget, true); + } + } + + if (mPendingSelectionChange.HasNotification()) { + IMENotification notification(mPendingSelectionChange); + if (!aWidget->Destroyed()) { + mPendingSelectionChange.Clear(); + IMEStateManager::NotifyIME(notification, aWidget, true); + } + } + + // Finally, send composition update notification because it notifies IME of + // finishing handling whole sending events. + if (mPendingCompositionUpdate.HasNotification()) { + IMENotification notification(mPendingCompositionUpdate); + if (!aWidget->Destroyed()) { + mPendingCompositionUpdate.Clear(); + IMEStateManager::NotifyIME(notification, aWidget, true); + } + } + + if (!--mPendingEventsNeedingAck && !aWidget->Destroyed() && + (mPendingTextChange.HasNotification() || + mPendingSelectionChange.HasNotification() || + mPendingCompositionUpdate.HasNotification())) { + FlushPendingNotifications(aWidget); + } } /***************************************************************************** diff --git a/widget/ContentCache.h b/widget/ContentCache.h index 891235aaf0b..679882c494d 100644 --- a/widget/ContentCache.h +++ b/widget/ContentCache.h @@ -14,18 +14,13 @@ #include "mozilla/CheckedInt.h" #include "mozilla/EventForwards.h" #include "mozilla/WritingModes.h" +#include "nsIWidget.h" #include "nsString.h" #include "nsTArray.h" #include "Units.h" -class nsIWidget; - namespace mozilla { -namespace widget { -struct IMENotification; -} - class ContentCacheInParent; /** @@ -322,8 +317,12 @@ public: /** * OnEventNeedingAckReceived() should be called when the child process * receives a sent event which needs acknowledging. + * + * WARNING: This may send notifications to IME. That might cause destroying + * TabParent or aWidget. Therefore, the caller must not destroy + * this instance during a call of this method. */ - void OnEventNeedingAckReceived(); + void OnEventNeedingAckReceived(nsIWidget* aWidget); /** * RequestToCommitComposition() requests to commit or cancel composition to @@ -344,13 +343,18 @@ public: nsAString& aLastString); /** - * InitNotification() initializes aNotification with stored data. - * - * @param aNotification Must be NOTIFY_IME_OF_SELECTION_CHANGE. + * MaybeNotifyIME() may notify IME of the notification. If child process + * hasn't been handled all sending events yet, this stores the notification + * and flush it later. */ - void InitNotification(IMENotification& aNotification) const; + void MaybeNotifyIME(nsIWidget* aWidget, + IMENotification& aNotification); private: + IMENotification mPendingSelectionChange; + IMENotification mPendingTextChange; + IMENotification mPendingCompositionUpdate; + // This is commit string which is caused by our request. nsString mCommitStringByRequest; // Start offset of the composition string. @@ -373,6 +377,7 @@ private: uint32_t aLength, LayoutDeviceIntRect& aUnionTextRect) const; + void FlushPendingNotifications(nsIWidget* aWidget); }; } // namespace mozilla diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h index 565bd053042..161e6cc6702 100644 --- a/widget/nsIWidget.h +++ b/widget/nsIWidget.h @@ -573,8 +573,11 @@ struct SizeConstraints { typedef int8_t IMEMessageType; enum IMEMessage : IMEMessageType { + // This is used by IMENotification internally. This means that the instance + // hasn't been initialized yet. + NOTIFY_IME_OF_NOTHING, // An editable content is getting focus - NOTIFY_IME_OF_FOCUS = 1, + NOTIFY_IME_OF_FOCUS, // An editable content is losing focus NOTIFY_IME_OF_BLUR, // Selection in the focused editable content is changed @@ -598,7 +601,7 @@ enum IMEMessage : IMEMessageType struct IMENotification { IMENotification() - : mMessage(static_cast(-1)) + : mMessage(NOTIFY_IME_OF_NOTHING) {} MOZ_IMPLICIT IMENotification(IMEMessage aMessage) @@ -631,6 +634,59 @@ struct IMENotification } } + void Clear() + { + mMessage = NOTIFY_IME_OF_NOTHING; + } + + bool HasNotification() const + { + return mMessage != NOTIFY_IME_OF_NOTHING; + } + + void MergeWith(const IMENotification& aNotification) + { + switch (mMessage) { + case NOTIFY_IME_OF_NOTHING: + MOZ_ASSERT(aNotification.mMessage != NOTIFY_IME_OF_NOTHING); + *this = aNotification; + break; + case NOTIFY_IME_OF_SELECTION_CHANGE: + MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE); + mSelectionChangeData.mOffset = + aNotification.mSelectionChangeData.mOffset; + mSelectionChangeData.mLength = + aNotification.mSelectionChangeData.mLength; + mSelectionChangeData.mWritingMode = + aNotification.mSelectionChangeData.mWritingMode; + mSelectionChangeData.mReversed = + aNotification.mSelectionChangeData.mReversed; + mSelectionChangeData.mCausedByComposition = + mSelectionChangeData.mCausedByComposition && + aNotification.mSelectionChangeData.mCausedByComposition; + break; + case NOTIFY_IME_OF_TEXT_CHANGE: + MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_TEXT_CHANGE); + // TODO: Needs to merge the ranges rather than overwriting. + mTextChangeData.mStartOffset = + aNotification.mTextChangeData.mStartOffset; + mTextChangeData.mOldEndOffset = + aNotification.mTextChangeData.mOldEndOffset; + mTextChangeData.mNewEndOffset = + aNotification.mTextChangeData.mNewEndOffset; + mTextChangeData.mCausedByComposition = + mTextChangeData.mCausedByComposition && + aNotification.mTextChangeData.mCausedByComposition; + break; + case NOTIFY_IME_OF_COMPOSITION_UPDATE: + MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_COMPOSITION_UPDATE); + break; + default: + MOZ_CRASH("Merging notification isn't supported"); + break; + } + } + IMEMessage mMessage; // NOTIFY_IME_OF_SELECTION_CHANGE specific data