From 0789f7b5955c7c94b0a355fac8b3ef1a334020d3 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Sat, 15 Apr 2017 01:35:58 +0900 Subject: [PATCH] Bug 1217700 part.1 nsIWidget should return reference to IMENotificationRequests r=m_kato IMEContentObserver may need to change notifications to send when TextInputProcessor begins input transaction. In current design, IMEContentObserver needs to retrieve IMENotificationRequests at every change. However, if nsIWidget returns a reference to its IMENotificationRequests, IMEContentObserver can call it only once. For that purpose, this patch changes nsIWidget::GetIMENotificationRequests() to nsIWidget::IMENotificationRequestsRef() and make it return |const IMENotificationRequests&|. However, if the lifetime of the instance of IMENotificationRequest is shorter than the widget instance's, it's dangerous. Therefore, it always returns TextEventDispatcher::mIMENotificationRequests. TextEventDispatcher's lifetime is longer than the widget. Therefore, this guarantees the lifetime. On the other hand, widget needs to update TextEventDispatcher::mIMENotificationRequests before calls of nsIWidget::IMENotificationRequestsRef(). Therefore, this patch makes TextEventDispatcher update proper IMENotificationRequests when it gets focus or starts new input transaction and clear mIMENotificationRequests when it loses focus. Note that TextEventDispatcher gets proper requests both from native text event dispatcher listener (typically, implemented by native IME handler class) and TextInputProcessor when TextInputProcessor has input transaction because even if TextInputProcessor overrides native IME, native IME still needs to know the content changes since they may get new input transaction after that. However, there may not be native IME handler in content process. If it runs in Android, PuppetWidget may have native IME handler because widget directly handles IME in e10s mode for Android. Otherwise, native IME handler is in its parent process. So, if TextInputHandler has input transaction in content process, PuppetWidget needs to behave as native event handler. Therefore, this patch makes PuppetWidget inherit TextEventDispatcherListener and implements PuppetWidget::IMENotificationRequestsRef(). MozReview-Commit-ID: 2SW3moONTOX --HG-- extra : rebase_source : d2634ada6c33dbf7a966fadb68608411ee24bfab --- dom/events/IMEContentObserver.cpp | 2 +- dom/ipc/TabParent.cpp | 5 +- widget/IMEData.h | 7 +++ widget/PuppetWidget.cpp | 74 +++++++++++++++---------- widget/PuppetWidget.h | 28 +++++++++- widget/TextEventDispatcher.cpp | 92 +++++++++++++++++++++++++------ widget/TextEventDispatcher.h | 28 +++++++++- widget/nsBaseWidget.cpp | 21 +++---- widget/nsBaseWidget.h | 1 - widget/nsIWidget.h | 7 ++- 10 files changed, 195 insertions(+), 70 deletions(-) diff --git a/dom/events/IMEContentObserver.cpp b/dom/events/IMEContentObserver.cpp index 1b500ef50e92..8b2dffa3dd9e 100644 --- a/dom/events/IMEContentObserver.cpp +++ b/dom/events/IMEContentObserver.cpp @@ -433,7 +433,7 @@ IMEContentObserver::ObserveEditableNode() mEditor->AddEditorObserver(this); } - mIMENotificationRequests = mWidget->GetIMENotificationRequests(); + mIMENotificationRequests = mWidget->IMENotificationRequestsRef(); if (!WasInitializedWithPlugin()) { // Add selection change listener only when this starts to observe // non-plugin content since we cannot detect selection changes in diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 9b253738a834..1416a7165d4a 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -1742,7 +1742,7 @@ TabParent::RecvNotifyIMEFocus(const ContentCache& aContentCache, IMEStateManager::NotifyIME(aIMENotification, widget, true); if (aIMENotification.mMessage == NOTIFY_IME_OF_FOCUS) { - *aRequests = widget->GetIMENotificationRequests(); + *aRequests = widget->IMENotificationRequestsRef(); } return IPC_OK(); } @@ -1757,7 +1757,8 @@ TabParent::RecvNotifyIMETextChange(const ContentCache& aContentCache, } #ifdef DEBUG - IMENotificationRequests requests = widget->GetIMENotificationRequests(); + const IMENotificationRequests& requests = + widget->IMENotificationRequestsRef(); NS_ASSERTION(requests.WantTextChange(), "Don't call Send/RecvNotifyIMETextChange without NOTIFY_TEXT_CHANGE"); #endif diff --git a/widget/IMEData.h b/widget/IMEData.h index 9277442917ac..5df5fabc6bfc 100644 --- a/widget/IMEData.h +++ b/widget/IMEData.h @@ -9,6 +9,8 @@ #include "nsPoint.h" #include "nsRect.h" #include "nsStringGlue.h" +#include "nsXULAppAPI.h" +#include "Units.h" class nsIWidget; @@ -66,6 +68,11 @@ struct IMENotificationRequests final { return IMENotificationRequests(aOther.mWantUpdates | mWantUpdates); } + IMENotificationRequests& operator|=(const IMENotificationRequests& aOther) + { + mWantUpdates |= aOther.mWantUpdates; + return *this; + } bool WantTextChange() const { diff --git a/widget/PuppetWidget.cpp b/widget/PuppetWidget.cpp index 155a6087b64d..4634b328d076 100644 --- a/widget/PuppetWidget.cpp +++ b/widget/PuppetWidget.cpp @@ -19,6 +19,7 @@ #include "mozilla/layers/WebRenderLayerManager.h" #include "mozilla/Preferences.h" #include "mozilla/TextComposition.h" +#include "mozilla/TextEventDispatcher.h" #include "mozilla/TextEvents.h" #include "mozilla/Unused.h" #include "BasicLayers.h" @@ -80,7 +81,8 @@ const size_t PuppetWidget::kMaxDimension = 4000; static bool gRemoteDesktopBehaviorEnabled = false; static bool gRemoteDesktopBehaviorInitialized = false; -NS_IMPL_ISUPPORTS_INHERITED0(PuppetWidget, nsBaseWidget) +NS_IMPL_ISUPPORTS_INHERITED(PuppetWidget, nsBaseWidget + , TextEventDispatcherListener) PuppetWidget::PuppetWidget(TabChild* aTabChild) : mTabChild(aTabChild) @@ -866,33 +868,6 @@ PuppetWidget::NotifyIMEOfCompositionUpdate( return NS_OK; } -IMENotificationRequests -PuppetWidget::GetIMENotificationRequests() -{ - if (mNativeTextEventDispatcherListener) { - // Use mNativeTextEventDispatcherListener for retrieving IME notification - // requests because non-native IME may have transaction. - return mNativeTextEventDispatcherListener->GetIMENotificationRequests(); - } - - // e10s requires IME content cache in in the TabParent for handling query - // content event only with the parent process. Therefore, this process - // needs to receive a lot of information from the focused editor to sent - // the latest content to the parent process. - if (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN) { - // But if a plugin has focus, we cannot receive text nor selection change - // in the plugin. Therefore, PuppetWidget needs to receive only position - // change event for updating the editor rect cache. - return IMENotificationRequests( - mIMENotificationRequestsOfParent.mWantUpdates | - IMENotificationRequests::NOTIFY_POSITION_CHANGE); - } - return IMENotificationRequests( - mIMENotificationRequestsOfParent.mWantUpdates | - IMENotificationRequests::NOTIFY_TEXT_CHANGE | - IMENotificationRequests::NOTIFY_POSITION_CHANGE); -} - nsresult PuppetWidget::NotifyIMEOfTextChange(const IMENotification& aIMENotification) { @@ -1547,5 +1522,48 @@ PuppetWidget::OnWindowedPluginKeyEvent(const NativeEventData& aKeyEventData, return NS_SUCCESS_EVENT_HANDLED_ASYNCHRONOUSLY; } +// TextEventDispatcherListener + +NS_IMETHODIMP +PuppetWidget::NotifyIME(TextEventDispatcher* aTextEventDispatcher, + const IMENotification& aNotification) +{ + MOZ_ASSERT(aTextEventDispatcher == mTextEventDispatcher); + return NS_ERROR_NOT_IMPLEMENTED; +} + +NS_IMETHODIMP_(IMENotificationRequests) +PuppetWidget::GetIMENotificationRequests() +{ + if (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN) { + // If a plugin has focus, we cannot receive text nor selection change + // in the plugin. Therefore, PuppetWidget needs to receive only position + // change event for updating the editor rect cache. + return IMENotificationRequests( + mIMENotificationRequestsOfParent.mWantUpdates | + IMENotificationRequests::NOTIFY_POSITION_CHANGE); + } + return IMENotificationRequests( + mIMENotificationRequestsOfParent.mWantUpdates | + IMENotificationRequests::NOTIFY_TEXT_CHANGE | + IMENotificationRequests::NOTIFY_POSITION_CHANGE); +} + +NS_IMETHODIMP_(void) +PuppetWidget::OnRemovedFrom(TextEventDispatcher* aTextEventDispatcher) +{ + MOZ_ASSERT(aTextEventDispatcher == mTextEventDispatcher); +} + +NS_IMETHODIMP_(void) +PuppetWidget::WillDispatchKeyboardEvent( + TextEventDispatcher* aTextEventDispatcher, + WidgetKeyboardEvent& aKeyboardEvent, + uint32_t aIndexOfKeypress, + void* aData) +{ + MOZ_ASSERT(aTextEventDispatcher == mTextEventDispatcher); +} + } // namespace widget } // namespace mozilla diff --git a/widget/PuppetWidget.h b/widget/PuppetWidget.h index 3f80dd705848..cf8ea8163f6e 100644 --- a/widget/PuppetWidget.h +++ b/widget/PuppetWidget.h @@ -39,11 +39,18 @@ namespace widget { struct AutoCacheNativeKeyCommands; class PuppetWidget : public nsBaseWidget + , public TextEventDispatcherListener { + typedef mozilla::CSSRect CSSRect; typedef mozilla::dom::TabChild TabChild; typedef mozilla::gfx::DrawTarget DrawTarget; + + // Avoiding to make compiler confused between mozilla::widget and nsIWidget. + typedef mozilla::widget::TextEventDispatcher TextEventDispatcher; + typedef mozilla::widget::TextEventDispatcherListener + TextEventDispatcherListener; + typedef nsBaseWidget Base; - typedef mozilla::CSSRect CSSRect; // The width and height of the "widget" are clamped to this. static const size_t kMaxDimension; @@ -183,9 +190,11 @@ public: const InputContextAction& aAction) override; virtual InputContext GetInputContext() override; virtual NativeIMEContext GetNativeIMEContext() override; - virtual IMENotificationRequests GetIMENotificationRequests() override; TextEventDispatcherListener* GetNativeTextEventDispatcherListener() override - { return mNativeTextEventDispatcherListener; } + { + return mNativeTextEventDispatcherListener ? + mNativeTextEventDispatcherListener.get() : this; + } void SetNativeTextEventDispatcherListener(TextEventDispatcherListener* aListener) { mNativeTextEventDispatcherListener = aListener; } @@ -291,6 +300,19 @@ public: const bool aIsVertical, const LayoutDeviceIntPoint& aPoint) override; + // TextEventDispatcherListener + using nsBaseWidget::NotifyIME; + NS_IMETHOD NotifyIME(TextEventDispatcher* aTextEventDispatcher, + const IMENotification& aNotification) override; + NS_IMETHOD_(IMENotificationRequests) GetIMENotificationRequests() override; + NS_IMETHOD_(void) OnRemovedFrom( + TextEventDispatcher* aTextEventDispatcher) override; + NS_IMETHOD_(void) WillDispatchKeyboardEvent( + TextEventDispatcher* aTextEventDispatcher, + WidgetKeyboardEvent& aKeyboardEvent, + uint32_t aIndexOfKeypress, + void* aData) override; + protected: virtual nsresult NotifyIMEInternal( const IMENotification& aIMENotification) override; diff --git a/widget/TextEventDispatcher.cpp b/widget/TextEventDispatcher.cpp index b7f519130d7d..432b11566012 100644 --- a/widget/TextEventDispatcher.cpp +++ b/widget/TextEventDispatcher.cpp @@ -27,6 +27,7 @@ TextEventDispatcher::TextEventDispatcher(nsIWidget* aWidget) , mDispatchingEvent(0) , mInputTransactionType(eNoInputTransaction) , mIsComposing(false) + , mHasFocus(false) { MOZ_RELEASE_ASSERT(mWidget, "aWidget must not be nullptr"); @@ -38,6 +39,8 @@ TextEventDispatcher::TextEventDispatcher(nsIWidget* aWidget) false); sInitialized = true; } + + ClearNotificationRequests(); } nsresult @@ -83,6 +86,7 @@ TextEventDispatcher::BeginInputTransactionInternal( nsCOMPtr listener = do_QueryReferent(mListener); if (listener) { if (listener == aListener && mInputTransactionType == aType) { + UpdateNotificationRequests(); return NS_OK; } // If this has composition or is dispatching an event, any other listener @@ -98,6 +102,7 @@ TextEventDispatcher::BeginInputTransactionInternal( if (listener && listener != aListener) { listener->OnRemovedFrom(this); } + UpdateNotificationRequests(); return NS_OK; } @@ -121,12 +126,15 @@ TextEventDispatcher::EndInputTransaction(TextEventDispatcherListener* aListener) mListener = nullptr; listener->OnRemovedFrom(this); + UpdateNotificationRequests(); } void TextEventDispatcher::OnDestroyWidget() { mWidget = nullptr; + mHasFocus = false; + ClearNotificationRequests(); mPendingComposition.Clear(); nsCOMPtr listener = do_QueryReferent(mListener); mListener = nullptr; @@ -338,13 +346,19 @@ TextEventDispatcher::NotifyIME(const IMENotification& aIMENotification) { nsresult rv = NS_ERROR_NOT_IMPLEMENTED; + if (aIMENotification.mMessage == NOTIFY_IME_OF_BLUR) { + mHasFocus = false; + ClearNotificationRequests(); + } + + // First, send the notification to current input transaction's listener. nsCOMPtr listener = do_QueryReferent(mListener); if (listener) { rv = listener->NotifyIME(this, aIMENotification); } - if (mInputTransactionType == eNativeInputTransaction || !mWidget) { + if (!mWidget) { return rv; } @@ -360,24 +374,66 @@ TextEventDispatcher::NotifyIME(const IMENotification& aIMENotification) // focus move). nsCOMPtr nativeListener = mWidget->GetNativeTextEventDispatcherListener(); - if (!nativeListener) { - return rv; + if (listener != nativeListener && nativeListener) { + switch (aIMENotification.mMessage) { + case REQUEST_TO_COMMIT_COMPOSITION: + case REQUEST_TO_CANCEL_COMPOSITION: + // It's not necessary to notify native IME of requests. + break; + default: { + // Even if current input transaction's listener returns NS_OK or + // something, we need to notify native IME of notifications because + // when user typing after TIP does something, the changed information + // is necessary for them. + nsresult rv2 = + nativeListener->NotifyIME(this, aIMENotification); + // But return the result from current listener except when the + // notification isn't handled. + if (rv == NS_ERROR_NOT_IMPLEMENTED) { + rv = rv2; + } + break; + } + } } - switch (aIMENotification.mMessage) { - case REQUEST_TO_COMMIT_COMPOSITION: - case REQUEST_TO_CANCEL_COMPOSITION: - // It's not necessary to notify native IME of requests. - return rv; - default: { - // Even if current input transaction's listener returns NS_OK or - // something, we need to notify native IME of notifications because - // when user typing after TIP does something, the changed information - // is necessary for them. - nsresult rv2 = - nativeListener->NotifyIME(this, aIMENotification); - // But return the result from current listener except when the - // notification isn't handled. - return rv == NS_ERROR_NOT_IMPLEMENTED ? rv2 : rv; + + if (aIMENotification.mMessage == NOTIFY_IME_OF_FOCUS) { + mHasFocus = true; + UpdateNotificationRequests(); + } + + return rv; +} + +void +TextEventDispatcher::ClearNotificationRequests() +{ + mIMENotificationRequests = IMENotificationRequests(); +} + +void +TextEventDispatcher::UpdateNotificationRequests() +{ + ClearNotificationRequests(); + + // If it doesn't has focus, no notifications are available. + if (!mHasFocus || !mWidget) { + return; + } + + // If there is a listener, its requests are necessary. + nsCOMPtr listener = do_QueryReferent(mListener); + if (listener) { + mIMENotificationRequests = listener->GetIMENotificationRequests(); + } + + // Even if this is in non-native input transaction, native IME needs + // requests. So, add native IME requests too. + if (!IsInNativeInputTransaction()) { + nsCOMPtr nativeListener = + mWidget->GetNativeTextEventDispatcherListener(); + if (nativeListener) { + mIMENotificationRequests |= nativeListener->GetIMENotificationRequests(); } } } diff --git a/widget/TextEventDispatcher.h b/widget/TextEventDispatcher.h index 37a4e72b2f90..96382c8dd40b 100644 --- a/widget/TextEventDispatcher.h +++ b/widget/TextEventDispatcher.h @@ -12,14 +12,13 @@ #include "mozilla/EventForwards.h" #include "mozilla/TextEventDispatcherListener.h" #include "mozilla/TextRange.h" +#include "mozilla/widget/IMEData.h" class nsIWidget; namespace mozilla { namespace widget { -struct IMENotification; - /** * TextEventDispatcher is a helper class for dispatching widget events defined * in TextEvents.h. Currently, this is a helper for dispatching @@ -75,6 +74,11 @@ public: nsIWidget* GetWidget() const { return mWidget; } + const IMENotificationRequests& IMENotificationRequestsRef() const + { + return mIMENotificationRequests; + } + /** * GetState() returns current state of this class. * @@ -307,6 +311,9 @@ private: // check if a method to uninstall the listener is called by valid instance. // So, using weak reference is the best way in this case. nsWeakPtr mListener; + // mIMENotificationRequests should store current IME's notification requests. + // So, this may be invalid when IME doesn't have focus. + IMENotificationRequests mIMENotificationRequests; // mPendingComposition stores new composition string temporarily. // These values will be used for dispatching eCompositionChange event @@ -412,6 +419,10 @@ private: // See IsComposing(). bool mIsComposing; + // true while NOTIFY_IME_OF_FOCUS is received but NOTIFY_IME_OF_BLUR has not + // received yet. Otherwise, false. + bool mHasFocus; + // If this is true, keydown and keyup events are dispatched even when there // is a composition. static bool sDispatchKeyEventsDuringComposition; @@ -495,6 +506,19 @@ private: void* aData, uint32_t aIndexOfKeypress = 0, bool aNeedsCallback = false); + + /** + * ClearNotificationRequests() clears mIMENotificationRequests. + */ + void ClearNotificationRequests(); + + /** + * UpdateNotificationRequests() updates mIMENotificationRequests with + * current state. If the instance doesn't have focus, this clears + * mIMENotificationRequests. Otherwise, updates it with both requests of + * current listener and native listener. + */ + void UpdateNotificationRequests(); }; } // namespace widget diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index 074b804f6f58..7ef9d9713c5b 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -1723,7 +1723,7 @@ nsBaseWidget::NotifyWindowMoved(int32_t aX, int32_t aY) mWidgetListener->WindowMoved(this, aX, aY); } - if (mIMEHasFocus && GetIMENotificationRequests().WantPositionChanged()) { + if (mIMEHasFocus && IMENotificationRequestsRef().WantPositionChanged()) { NotifyIME(IMENotification(IMEMessage::NOTIFY_IME_OF_POSITION_CHANGE)); } } @@ -1799,18 +1799,6 @@ nsBaseWidget::NotifyIME(const IMENotification& aIMENotification) } } -IMENotificationRequests -nsBaseWidget::GetIMENotificationRequests() -{ - RefPtr listener = - GetNativeTextEventDispatcherListener(); - if (!listener) { - // Default is to not send additional change notifications to NotifyIME. - return IMENotificationRequests(); - } - return listener->GetIMENotificationRequests(); -} - void nsBaseWidget::EnsureTextEventDispatcher() { @@ -2306,6 +2294,13 @@ nsIWidget::GetNativeIMEContext() return NativeIMEContext(this); } +const IMENotificationRequests& +nsIWidget::IMENotificationRequestsRef() +{ + TextEventDispatcher* dispatcher = GetTextEventDispatcher(); + return dispatcher->IMENotificationRequestsRef(); +} + nsresult nsIWidget::OnWindowedPluginKeyEvent(const NativeEventData& aKeyEventData, nsIKeyEventInPluginCallback* aCallback) diff --git a/widget/nsBaseWidget.h b/widget/nsBaseWidget.h index e6c4511962b6..ff296d2638b6 100644 --- a/widget/nsBaseWidget.h +++ b/widget/nsBaseWidget.h @@ -286,7 +286,6 @@ public: void* aCallbackData) override { return false; } bool ComputeShouldAccelerate(); virtual bool WidgetTypeSupportsAcceleration() { return true; } - virtual IMENotificationRequests GetIMENotificationRequests() override; virtual MOZ_MUST_USE nsresult OnDefaultButtonLoaded(const LayoutDeviceIntRect& aButtonRect) override { return NS_ERROR_NOT_IMPLEMENTED; } virtual already_AddRefed CreateChild(const LayoutDeviceIntRect& aRect, diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h index 5fa81464db4e..3d294f01c7af 100644 --- a/widget/nsIWidget.h +++ b/widget/nsIWidget.h @@ -1818,9 +1818,12 @@ public: void* aCallbackData) = 0; /* - * Retrieves preference for IME updates + * Retrieves a reference to notification requests of IME. Note that the + * reference is valid while the nsIWidget instance is alive. So, if you + * need to store the reference for a long time, you need to grab the widget + * instance too. */ - virtual IMENotificationRequests GetIMENotificationRequests() = 0; + const IMENotificationRequests& IMENotificationRequestsRef(); /* * Call this method when a dialog is opened which has a default button.