Bug 1384027 - part1: PuppetWidget should have TextEventDispatcher like nsIWidget instance in the parent process r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 25 Jul 2017 23:27:31 +0900
changeset 615862 574d64a99a6c08073bd28b3dcfe97a716b03615b
parent 615823 d47f5bab381cfc107b2d8c1bb558e68422afb8b3
child 615863 54a42c19bd2b46893a57bf8ba522e43da1ef6497
push id70509
push usermasayuki@d-toybox.com
push dateWed, 26 Jul 2017 13:53:45 +0000
reviewerssmaug
bugs1384027
milestone56.0a1
Bug 1384027 - part1: PuppetWidget should have TextEventDispatcher like nsIWidget instance in the parent process r?smaug In the parent process, every nsIWidget instance like nsWindow has TextEventDispatcher instance after dispatching a keyboard event or a composition event. Then, TextEventDispatcher manages whether there is composition and handles NotifyIME. However, PuppetWidget doesn't have it unless it synthesizes keyboard events or composition events for tests. This causes PuppetWidget implementing nsIWidget::NotifyIME() with nsBaseWidget::NotifyIMEInternal() which is a virtual method only implemented by PuppetWidget. For consistent implementation around here, we should move NotifyIMEInternal() implementation to TextEventDispatcherListener::NotifyIME() which is called by TextEventDispatcher::NotifyIME(). Then, PuppetWidget can handle NotifyIME() easier. This patch creates TextEventDispatcher::BeginInputTransactionFor() which takes pointer to a dispatching event and pointer to PuppetWidget. It emulates each corresponding event dispatcher method for managing composing state and begins input transaction for the dispatching event. Unfortunately, this implementation is ugly due to duplicated code. However, this is enough for now. When we need to make TextEventDispatcher manage more states, we should add methods which are shared by both BeginInputTransactionFor() and event dispatcher method. MozReview-Commit-ID: GeP028luZjR
widget/PuppetWidget.cpp
widget/TextEventDispatcher.cpp
widget/TextEventDispatcher.h
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -356,16 +356,42 @@ PuppetWidget::DispatchEvent(WidgetGUIEve
         "When there is composition caused by old native IME context, "
         "composition events caused by different native IME context are not "
         "allowed");
     }
 #endif // #ifdef DEBUG
     mNativeIMEContext = compositionEvent->mNativeIMEContext;
   }
 
+  // If the event is a composition event or a keyboard event, it should be
+  // dispatched with TextEventDispatcher if we could do that with current
+  // design.  However, we cannot do that without big changes and the behavior
+  // is not so complicated for now.  Therefore, we should just notify it
+  // of dispatching events and TextEventDispatcher should emulate the state
+  // with events here.
+  if (aEvent->mClass == eCompositionEventClass ||
+      aEvent->mClass == eKeyboardEventClass) {
+    TextEventDispatcher* dispatcher = GetTextEventDispatcher();
+    // However, if the event is being dispatched by the text event dispatcher
+    // or, there is native text event dispatcher listener, that means that
+    // native text input event handler is in this process like on Android,
+    // and the event is not synthesized for tests, the event is coming from
+    // the TextEventDispatcher.  In these cases, we shouldn't notify
+    // TextEventDispatcher of dispatching the event.
+    if (!dispatcher->IsDispatchingEvent() &&
+        !(mNativeTextEventDispatcherListener &&
+          !aEvent->mFlags.mIsSynthesizedForTests)) {
+      DebugOnly<nsresult> rv =
+        dispatcher->BeginInputTransactionFor(aEvent, this);
+      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+        "The text event dispatcher should always succeed to start input "
+        "transaction for the event");
+    }
+  }
+
   aStatus = nsEventStatus_eIgnore;
 
   if (GetCurrentWidgetListener()) {
     aStatus =
       GetCurrentWidgetListener()->HandleEvent(aEvent, mUseAttachedEvents);
   }
 
   return NS_OK;
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -7,16 +7,17 @@
 #include "mozilla/TextEvents.h"
 #include "mozilla/TextEventDispatcher.h"
 #include "nsIDocShell.h"
 #include "nsIFrame.h"
 #include "nsIPresShell.h"
 #include "nsIWidget.h"
 #include "nsPIDOMWindow.h"
 #include "nsView.h"
+#include "PuppetWidget.h"
 
 namespace mozilla {
 namespace widget {
 
 /******************************************************************************
  * TextEventDispatcher
  *****************************************************************************/
 
@@ -101,16 +102,90 @@ TextEventDispatcher::BeginInputTransacti
   mInputTransactionType = aType;
   if (listener && listener != aListener) {
     listener->OnRemovedFrom(this);
   }
   UpdateNotificationRequests();
   return NS_OK;
 }
 
+nsresult
+TextEventDispatcher::BeginInputTransactionFor(const WidgetGUIEvent* aEvent,
+                                              PuppetWidget* aPuppetWidget)
+{
+  MOZ_ASSERT(XRE_IsContentProcess());
+  MOZ_ASSERT(!IsDispatchingEvent());
+
+  switch (aEvent->mMessage) {
+    case eKeyDown:
+    case eKeyPress:
+    case eKeyUp:
+      MOZ_ASSERT(aEvent->mClass == eKeyboardEventClass);
+      break;
+    case eCompositionStart:
+    case eCompositionChange:
+    case eCompositionCommit:
+    case eCompositionCommitAsIs:
+      MOZ_ASSERT(aEvent->mClass == eCompositionEventClass);
+      break;
+    default:
+      return NS_ERROR_INVALID_ARG;
+  }
+
+  if (aEvent->mFlags.mIsSynthesizedForTests) {
+    // If the event is for an automated test and this instance dispatched
+    // an event to the parent process, we can assume that this is already
+    // initialized properly.
+    if (mInputTransactionType == eAsyncTestInputTransaction) {
+      return NS_OK;
+    }
+    // Even if the event coming from the parent process is synthesized for
+    // tests, this process should treat it as "sync" test here because
+    // it won't be go back to the parent process.
+    nsresult rv =
+      BeginInputTransactionInternal(
+        static_cast<TextEventDispatcherListener*>(aPuppetWidget),
+        eSameProcessSyncTestInputTransaction);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+  } else {
+    nsresult rv = BeginNativeInputTransaction();
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+  }
+
+  // Emulate modifying members which indicate the state of composition.
+  // If we need to manage more states and/or more complexly, we should create
+  // internal methods which are called by both here and each event dispatcher
+  // method of this class.
+  switch (aEvent->mMessage) {
+    case eKeyDown:
+    case eKeyPress:
+    case eKeyUp:
+      return NS_OK;
+    case eCompositionStart:
+      MOZ_ASSERT(!mIsComposing);
+      mIsComposing = true;
+      return NS_OK;
+    case eCompositionChange:
+      MOZ_ASSERT(mIsComposing);
+      mIsComposing = true;
+      return NS_OK;
+    case eCompositionCommit:
+    case eCompositionCommitAsIs:
+      MOZ_ASSERT(mIsComposing);
+      mIsComposing = false;
+      return NS_OK;
+    default:
+      MOZ_ASSERT_UNREACHABLE("You forgot to handle the event");
+      return NS_ERROR_UNEXPECTED;
+  }
+}
 void
 TextEventDispatcher::EndInputTransaction(TextEventDispatcherListener* aListener)
 {
   if (NS_WARN_IF(IsComposing()) || NS_WARN_IF(IsDispatchingEvent())) {
     return;
   }
 
   mInputTransactionType = eNoInputTransaction;
@@ -230,16 +305,18 @@ TextEventDispatcher::StartComposition(ns
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   if (NS_WARN_IF(mIsComposing)) {
     return NS_ERROR_FAILURE;
   }
 
+  // When you change some members from here, you may need same change in
+  // BeginInputTransactionFor().
   mIsComposing = true;
   WidgetCompositionEvent compositionStartEvent(true, eCompositionStart,
                                                mWidget);
   InitEvent(compositionStartEvent);
   if (aEventTime) {
     compositionStartEvent.AssignEventTime(*aEventTime);
   }
   rv = DispatchEvent(mWidget, compositionStartEvent, aStatus);
@@ -310,16 +387,19 @@ TextEventDispatcher::CommitComposition(n
   rv = StartCompositionAutomaticallyIfNecessary(aStatus, aEventTime);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   if (aStatus == nsEventStatus_eConsumeNoDefault) {
     return NS_OK;
   }
 
+  // When you change some members from here, you may need same change in
+  // BeginInputTransactionFor().
+
   // End current composition and make this free for other IMEs.
   mIsComposing = false;
 
   EventMessage message = aCommitString ? eCompositionCommit :
                                          eCompositionCommitAsIs;
   WidgetCompositionEvent compositionCommitEvent(true, message, widget);
   InitEvent(compositionCommitEvent);
   if (aEventTime) {
--- a/widget/TextEventDispatcher.h
+++ b/widget/TextEventDispatcher.h
@@ -14,16 +14,18 @@
 #include "mozilla/TextRange.h"
 #include "mozilla/widget/IMEData.h"
 
 class nsIWidget;
 
 namespace mozilla {
 namespace widget {
 
+class PuppetWidget;
+
 /**
  * TextEventDispatcher is a helper class for dispatching widget events defined
  * in TextEvents.h.  Currently, this is a helper for dispatching
  * WidgetCompositionEvent and WidgetKeyboardEvent.  This manages the behavior
  * of them for conforming to DOM Level 3 Events.
  * An instance of this class is created by nsIWidget instance and owned by it.
  * This is typically created only by the top level widgets because only they
  * handle IME.
@@ -55,16 +57,23 @@ public:
    *                              definition below.
    */
   nsresult BeginInputTransaction(TextEventDispatcherListener* aListener);
   nsresult BeginTestInputTransaction(TextEventDispatcherListener* aListener,
                                      bool aIsAPZAware);
   nsresult BeginNativeInputTransaction();
 
   /**
+   * BeginInputTransactionFor() should be used when aPuppetWidget dispatches
+   * a composition or keyboard event coming from its parent process.
+   */
+  nsresult BeginInputTransactionFor(const WidgetGUIEvent* aEvent,
+                                    PuppetWidget* aPuppetWidget);
+
+  /**
    * EndInputTransaction() should be called when the listener stops using
    * the TextEventDispatcher.
    *
    * @param aListener       The listener using the TextEventDispatcher instance.
    */
   void EndInputTransaction(TextEventDispatcherListener* aListener);
 
   /**
@@ -363,23 +372,31 @@ private:
   uint16_t mDispatchingEvent;
 
   enum InputTransactionType : uint8_t
   {
     // No input transaction has been started.
     eNoInputTransaction,
     // Input transaction for native IME or keyboard event handler.  Note that
     // keyboard events may be dispatched via parent process if there is.
+    // In remote processes, this is also used when events come from the parent
+    // process and are not for tests because we cannot distinguish if
+    // TextEventDispatcher has which type of transaction when it dispatches
+    // (eNativeInputTransaction or eSameProcessSyncInputTransaction).
     eNativeInputTransaction,
     // Input transaction for automated tests which are APZ-aware.  Note that
     // keyboard events may be dispatched via parent process if there is.
     eAsyncTestInputTransaction,
     // Input transaction for automated tests which assume events are fired
     // synchronously.  I.e., keyboard events are always dispatched in the
     // current process.
+    // In remote processes, this is also used when events come from the parent
+    // process and are not dispatched by the instance itself for APZ-aware
+    // tests because this instance won't dispatch the events via the parent
+    // process again.
     eSameProcessSyncTestInputTransaction,
     // Input transaction for Others (must be IME on B2G).  Events are fired
     // synchronously because TextInputProcessor which is the only user of
     // this input transaction type supports only keyboard apps on B2G.
     // Keyboard apps on B2G doesn't want to dispatch keyboard events to
     // chrome process. Therefore, this should dispatch key events only in
     // the current process.
     eSameProcessSyncInputTransaction