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
2017-04-15 01:35:58 +09:00 · 2017-04-15 01:35:58 +09:00 · 0789f7b595
--- 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
--- 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
--- 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
  {
--- 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
--- 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;
--- 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<TextEventDispatcherListener> 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<TextEventDispatcherListener> 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<TextEventDispatcherListener> 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<TextEventDispatcherListener> 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<TextEventDispatcherListener> 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<TextEventDispatcherListener> nativeListener =
+      mWidget->GetNativeTextEventDispatcherListener();
+    if (nativeListener) {
+      mIMENotificationRequests |= nativeListener->GetIMENotificationRequests();
    }
  }
 }
--- 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
--- 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<TextEventDispatcherListener> 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)
--- 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<nsIWidget>
  CreateChild(const LayoutDeviceIntRect& aRect,
--- 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.