Bug 1339543 part 2 eKeyPress event should have edit commands for all editor types when it's dispatched to a remote process r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 19 May 2017 17:24:20 +0900
changeset 359280 198e58f7052f48338f8c10c08c6b6713bae8bafb
parent 359279 68c474271ba5786863f2e89bfcf8e6069383c38a
child 359281 a950fbfa6d8cb1c3287a5ed14b4b09d497057224
push id31851
push userkwierso@gmail.com
push dateFri, 19 May 2017 21:18:45 +0000
treeherdermozilla-central@5a8f2dcbeac0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1339543, 1316330
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1339543 part 2 eKeyPress event should have edit commands for all editor types when it's dispatched to a remote process r=smaug When eKeyPress event is dispatched from TabParent to a remote process, it should store edit command for all editor types. Then, copied WidgetKeyboardEvent in the remote process doesn't need to request the edit commands when its ExecuteEditCommands() is called. Note that this patch also changes a automated test, browser_bug1316330.js, that uses nsIDOMWindowUtils.dispatchDOMEventViaPresShell() to dispatch repeated keyboard events in the tab. However, it should use synthesizeKey() to emulate everything of native keyboard events and the API can dispatch repeated keyboard events too. (And the test has a bug. It tries to wait 0.5 sec when every keydown or keypress event. However, it fails since startTime is never initialized. This patch fixes this bug too.) MozReview-Commit-ID: IYhyxqH3Ch8
dom/ipc/PBrowser.ipdl
dom/ipc/TabChild.cpp
dom/ipc/TabChild.h
dom/ipc/TabParent.cpp
dom/tests/browser/browser_bug1316330.js
widget/PuppetWidget.cpp
widget/TextEvents.h
widget/WidgetEventImpl.cpp
widget/nsGUIEventIPC.h
--- a/dom/ipc/PBrowser.ipdl
+++ b/dom/ipc/PBrowser.ipdl
@@ -686,17 +686,17 @@ child:
      * Mouse move events with |reason == eSynthesized| are sent via a separate
      * message because they do not generate DOM 'mousemove' events, and the
      * 'compress' attribute on RealMouseMoveEvent() could result in a
      * |reason == eReal| event being dropped in favour of an |eSynthesized|
      * event, and thus a DOM 'mousemove' event to be lost.
      */
     async SynthMouseMoveEvent(WidgetMouseEvent event, ScrollableLayerGuid aGuid, uint64_t aInputBlockId);
     async RealMouseButtonEvent(WidgetMouseEvent event, ScrollableLayerGuid aGuid, uint64_t aInputBlockId);
-    async RealKeyEvent(WidgetKeyboardEvent event, MaybeNativeKeyBinding keyBinding);
+    async RealKeyEvent(WidgetKeyboardEvent event);
     async MouseWheelEvent(WidgetWheelEvent event, ScrollableLayerGuid aGuid, uint64_t aInputBlockId);
     async RealTouchEvent(WidgetTouchEvent aEvent,
                          ScrollableLayerGuid aGuid,
                          uint64_t aInputBlockId,
                          nsEventStatus aApzResponse);
     async HandleTap(TapType aType, LayoutDevicePoint point, Modifiers aModifiers,
                     ScrollableLayerGuid aGuid, uint64_t aInputBlockId);
     async RealTouchMoveEvent(WidgetTouchEvent aEvent,
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1939,39 +1939,30 @@ TabChild::UpdateRepeatedKeyEventEndTime(
 {
   if (aEvent.mIsRepeat &&
       (aEvent.mMessage == eKeyDown || aEvent.mMessage == eKeyPress)) {
     mRepeatedKeyEventTime = TimeStamp::Now();
   }
 }
 
 mozilla::ipc::IPCResult
-TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent,
-                           const MaybeNativeKeyBinding& aBindings)
+TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent)
 {
   if (SkipRepeatedKeyEvent(aEvent)) {
     return IPC_OK();
   }
 
-  AutoCacheNativeKeyCommands autoCache(mPuppetWidget);
-
-  if (aEvent.mMessage == eKeyPress) {
-    // If content code called preventDefault() on a keydown event, then we don't
-    // want to process any following keypress events.
-    if (mIgnoreKeyPressEvent) {
-      return IPC_OK();
-    }
-    if (aBindings.type() == MaybeNativeKeyBinding::TNativeKeyBinding) {
-      const NativeKeyBinding& bindings = aBindings;
-      autoCache.Cache(bindings.singleLineCommands(),
-                      bindings.multiLineCommands(),
-                      bindings.richTextCommands());
-    } else {
-      autoCache.CacheNoCommands();
-    }
+  MOZ_ASSERT(aEvent.mMessage != eKeyPress ||
+             aEvent.AreAllEditCommandsInitialized(),
+    "eKeyPress event should have native key binding information");
+
+  // If content code called preventDefault() on a keydown event, then we don't
+  // want to process any following keypress events.
+  if (aEvent.mMessage == eKeyPress && mIgnoreKeyPressEvent) {
+    return IPC_OK();
   }
 
   WidgetKeyboardEvent localEvent(aEvent);
   localEvent.mWidget = mPuppetWidget;
   localEvent.mUniqueId = aEvent.mUniqueId;
   nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent);
 
   // Update the end time of the possible repeated event so that we can skip
--- a/dom/ipc/TabChild.h
+++ b/dom/ipc/TabChild.h
@@ -384,18 +384,17 @@ public:
                                                            const ScrollableLayerGuid& aGuid,
                                                            const uint64_t& aInputBlockId) override;
 
   virtual mozilla::ipc::IPCResult RecvRealDragEvent(const WidgetDragEvent& aEvent,
                                                     const uint32_t& aDragAction,
                                                     const uint32_t& aDropEffect) override;
 
   virtual mozilla::ipc::IPCResult
-  RecvRealKeyEvent(const mozilla::WidgetKeyboardEvent& aEvent,
-                   const MaybeNativeKeyBinding& aBindings) override;
+  RecvRealKeyEvent(const mozilla::WidgetKeyboardEvent& aEvent) override;
 
   virtual mozilla::ipc::IPCResult RecvMouseWheelEvent(const mozilla::WidgetWheelEvent& aEvent,
                                                       const ScrollableLayerGuid& aGuid,
                                                       const uint64_t& aInputBlockId) override;
 
   virtual mozilla::ipc::IPCResult RecvRealTouchEvent(const WidgetTouchEvent& aEvent,
                                                      const ScrollableLayerGuid& aGuid,
                                                      const uint64_t& aInputBlockId,
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1439,41 +1439,25 @@ TabParent::RecvClearNativeTouchSequence(
 bool
 TabParent::SendRealKeyEvent(WidgetKeyboardEvent& aEvent)
 {
   if (mIsDestroyed) {
     return false;
   }
   aEvent.mRefPoint += GetChildProcessOffset();
 
-  MaybeNativeKeyBinding bindings;
-  bindings = void_t();
   if (aEvent.mMessage == eKeyPress) {
-    nsCOMPtr<nsIWidget> widget = GetWidget();
-
-    AutoTArray<mozilla::CommandInt, 4> singleLine;
-    AutoTArray<mozilla::CommandInt, 4> multiLine;
-    AutoTArray<mozilla::CommandInt, 4> richText;
-
-    widget->ExecuteNativeKeyBinding(
-              nsIWidget::NativeKeyBindingsForSingleLineEditor,
-              aEvent, DoCommandCallback, &singleLine);
-    widget->ExecuteNativeKeyBinding(
-              nsIWidget::NativeKeyBindingsForMultiLineEditor,
-              aEvent, DoCommandCallback, &multiLine);
-    widget->ExecuteNativeKeyBinding(
-              nsIWidget::NativeKeyBindingsForRichTextEditor,
-              aEvent, DoCommandCallback, &richText);
-
-    if (!singleLine.IsEmpty() || !multiLine.IsEmpty() || !richText.IsEmpty()) {
-      bindings = NativeKeyBinding(singleLine, multiLine, richText);
-    }
+    // XXX Should we do this only when input context indicates an editor having
+    //     focus and the key event won't cause inputting text?
+    aEvent.InitAllEditCommands();
+  } else {
+    aEvent.PreventNativeKeyBindings();
   }
 
-  return PBrowserParent::SendRealKeyEvent(aEvent, bindings);
+  return PBrowserParent::SendRealKeyEvent(aEvent);
 }
 
 bool
 TabParent::SendRealTouchEvent(WidgetTouchEvent& aEvent)
 {
   if (mIsDestroyed) {
     return false;
   }
--- a/dom/tests/browser/browser_bug1316330.js
+++ b/dom/tests/browser/browser_bug1316330.js
@@ -1,54 +1,36 @@
 const URL =
   "data:text/html,<script>" +
   "window.focus();" + 
   "var down = 0; var press = 0;" +
   "onkeydown = function(e) {" +
+  "  var startTime = Date.now();" +
   "  document.body.setAttribute('data-down', ++down);" +
-  "  if (e.keyCode == 'D') while (Date.now() - startTime < 500) {}" +
+  "  if (e.keyCode == KeyboardEvent.DOM_VK_D) while (Date.now() - startTime < 500) {}" +
   "};" +
   "onkeypress = function(e) {" +
+  "  var startTime = Date.now();" +
   "  document.body.setAttribute('data-press', ++press);" +
-  "  if (e.charCode == 'P') while (Date.now() - startTime < 500) {}" +
+  "  if (e.charCode == 'p'.charCodeAt(0)) while (Date.now() - startTime < 500) {}" +
   "};" +
   "</script>";
 
-function createKeyEvent(type, id, repeated) {
-  var code = id.charCodeAt(0);
-  return new KeyboardEvent(type, { keyCode: code, charCode: code, bubbles: true, repeat: repeated });
-}
-
 add_task(function* () {
   let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, URL);
   let browser = tab.linkedBrowser;
 
-  // Need to dispatch a DOM Event explicitly via PresShell to get KeyEvent with .repeat = true to 
-  // be handled by EventStateManager, which then forwards the event to the child process.
-  var utils = EventUtils._getDOMWindowUtils(window);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keydown", "D", false), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keypress", "D", false), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keydown", "D", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keypress", "D", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keydown", "D", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keypress", "D", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keyup", "D", true), true);
+  EventUtils.synthesizeKey("d", { code: "KeyD", repeat: 3 });
 
   yield ContentTask.spawn(browser, null, function* () {
     is(content.document.body.getAttribute("data-down"), "2", "Correct number of events");
     is(content.document.body.getAttribute("data-press"), "2", "Correct number of events");
   });
 
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keydown", "P", false), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keypress", "P", false), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keydown", "P", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keypress", "P", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keydown", "P", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keypress", "P", true), true);
-  utils.dispatchDOMEventViaPresShell(browser, createKeyEvent("keyup", "P", true), true);
+  EventUtils.synthesizeKey("p", { code: "KeyP", repeat: 3 });
 
   yield ContentTask.spawn(browser, null, function* () {
     is(content.document.body.getAttribute("data-down"), "4", "Correct number of events");
     is(content.document.body.getAttribute("data-press"), "4", "Correct number of events");
   });
 
   gBrowser.removeCurrentTab();
 });
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -337,19 +337,24 @@ PuppetWidget::DispatchEvent(WidgetGUIEve
 {
 #ifdef DEBUG
   debug_DumpEvent(stdout, aEvent->mWidget, aEvent, "PuppetWidget", 0);
 #endif
 
   MOZ_ASSERT(!mChild || mChild->mWindowType == eWindowType_popup,
              "Unexpected event dispatch!");
 
+  MOZ_ASSERT(!aEvent->AsKeyboardEvent() ||
+             aEvent->mFlags.mIsSynthesizedForTests ||
+             aEvent->AsKeyboardEvent()->AreAllEditCommandsInitialized(),
+    "Non-sysnthesized keyboard events should have edit commands for all types "
+    "before dispatched");
+
   AutoCacheNativeKeyCommands autoCache(this);
-  if ((aEvent->mFlags.mIsSynthesizedForTests ||
-       aEvent->mFlags.mIsSuppressedOrDelayed) && !mNativeKeyCommandsValid) {
+  if (aEvent->mFlags.mIsSynthesizedForTests && !mNativeKeyCommandsValid) {
     WidgetKeyboardEvent* keyEvent = aEvent->AsKeyboardEvent();
     if (keyEvent) {
       mTabChild->RequestNativeKeyBindings(&autoCache, keyEvent);
     }
   }
 
   if (aEvent->mClass == eCompositionEventClass) {
     // Store the latest native IME context of parent process's widget or
--- a/widget/TextEvents.h
+++ b/widget/TextEvents.h
@@ -104,16 +104,17 @@ struct ShortcutKeyCandidate
  * mozilla::WidgetKeyboardEvent
  ******************************************************************************/
 
 class WidgetKeyboardEvent : public WidgetInputEvent
 {
 private:
   friend class dom::PBrowserParent;
   friend class dom::PBrowserChild;
+  friend struct IPC::ParamTraits<WidgetKeyboardEvent>;
 
 protected:
   WidgetKeyboardEvent()
     : mNativeKeyEvent(nullptr)
     , mKeyCode(0)
     , mCharCode(0)
     , mPseudoCharCode(0)
     , mLocation(eKeyLocationStandard)
@@ -282,27 +283,59 @@ public:
   bool mIsComposing;
   // Indicates if the key combination is reserved by chrome.  This is set by
   // nsXBLWindowKeyHandler at capturing phase of the default event group.
   bool mIsReserved;
   // Indicates whether the event is synthesized from Text Input Processor
   // or an actual event from nsAppShell.
   bool mIsSynthesizedByTIP;
 
+  /**
+   * Retrieves all edit commands from mWidget.  This shouldn't be called when
+   * the instance is an untrusted event, doesn't have widget or in non-chrome
+   * process.
+   */
+  void InitAllEditCommands();
+
+  /**
+   * PreventNativeKeyBindings() makes the instance to not cause any edit
+   * actions even if it matches with a native key binding.
+   */
+  void PreventNativeKeyBindings()
+  {
+    mEditCommandsForSingleLineEditor.Clear();
+    mEditCommandsForMultiLineEditor.Clear();
+    mEditCommandsForRichTextEditor.Clear();
+    mEditCommandsForSingleLineEditorInitialized = true;
+    mEditCommandsForMultiLineEditorInitialized = true;
+    mEditCommandsForRichTextEditorInitialized = true;
+  }
+
 #ifdef DEBUG
   /**
    * IsEditCommandsInitialized() returns true if edit commands for aType
    * was already initialized.  Otherwise, false.
    */
   bool IsEditCommandsInitialized(
          nsIWidget::NativeKeyBindingsType aType) const
   {
     return const_cast<WidgetKeyboardEvent*>(this)->
              IsEditCommandsInitializedRef(aType);
   }
+
+  /**
+   * AreAllEditCommandsInitialized() returns true if edit commands for all
+   * types were already initialized.  Otherwise, false.
+   */
+  bool AreAllEditCommandsInitialized() const
+  {
+    return mEditCommandsForSingleLineEditorInitialized &&
+           mEditCommandsForMultiLineEditorInitialized &&
+           mEditCommandsForRichTextEditorInitialized;
+  }
 #endif // #ifdef DEBUG
 
   /**
    * Execute edit commands for aType.
    *
    * @return        true if the caller should do nothing anymore.
    *                false, otherwise.
    */
--- a/widget/WidgetEventImpl.cpp
+++ b/widget/WidgetEventImpl.cpp
@@ -598,16 +598,41 @@ const char16_t* const WidgetKeyboardEven
 };
 #undef NS_DEFINE_PHYSICAL_KEY_CODE_NAME
 
 WidgetKeyboardEvent::KeyNameIndexHashtable*
   WidgetKeyboardEvent::sKeyNameIndexHashtable = nullptr;
 WidgetKeyboardEvent::CodeNameIndexHashtable*
   WidgetKeyboardEvent::sCodeNameIndexHashtable = nullptr;
 
+void
+WidgetKeyboardEvent::InitAllEditCommands()
+{
+  // If the event was created without widget, e.g., created event in chrome
+  // script, this shouldn't execute native key bindings.
+  if (NS_WARN_IF(!mWidget)) {
+    return;
+  }
+
+  // This event should be trusted event here and we shouldn't expose native
+  // key binding information to web contents with untrusted events.
+  if (NS_WARN_IF(!IsTrusted())) {
+    return;
+  }
+
+  MOZ_ASSERT(XRE_IsParentProcess(),
+    "It's too expensive to retrieve all edit commands from remote process");
+  MOZ_ASSERT(!AreAllEditCommandsInitialized(),
+    "Shouldn't be called two or more times");
+
+  InitEditCommandsFor(nsIWidget::NativeKeyBindingsForSingleLineEditor);
+  InitEditCommandsFor(nsIWidget::NativeKeyBindingsForMultiLineEditor);
+  InitEditCommandsFor(nsIWidget::NativeKeyBindingsForRichTextEditor);
+}
+
 static void
 DoCommandCallback(Command aCommand, void* aData)
 {
   static_cast<nsTArray<CommandInt>*>(aData)->AppendElement(aCommand);
 }
 
 void
 WidgetKeyboardEvent::InitEditCommandsFor(nsIWidget::NativeKeyBindingsType aType)
--- a/widget/nsGUIEventIPC.h
+++ b/widget/nsGUIEventIPC.h
@@ -432,18 +432,26 @@ struct ParamTraits<mozilla::WidgetKeyboa
                  (aParam.mInputMethodAppState));
 #ifdef XP_MACOSX
     WriteParam(aMsg, aParam.mNativeKeyCode);
     WriteParam(aMsg, aParam.mNativeModifierFlags);
     WriteParam(aMsg, aParam.mNativeCharacters);
     WriteParam(aMsg, aParam.mNativeCharactersIgnoringModifiers);
     WriteParam(aMsg, aParam.mPluginTextEventString);
 #endif
+
     // An OS-specific native event might be attached in |mNativeKeyEvent|,  but
     // that cannot be copied across process boundaries.
+
+    WriteParam(aMsg, aParam.mEditCommandsForSingleLineEditor);
+    WriteParam(aMsg, aParam.mEditCommandsForMultiLineEditor);
+    WriteParam(aMsg, aParam.mEditCommandsForRichTextEditor);
+    WriteParam(aMsg, aParam.mEditCommandsForSingleLineEditorInitialized);
+    WriteParam(aMsg, aParam.mEditCommandsForMultiLineEditorInitialized);
+    WriteParam(aMsg, aParam.mEditCommandsForRichTextEditorInitialized);
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
     mozilla::KeyNameIndexType keyNameIndex = 0;
     mozilla::CodeNameIndexType codeNameIndex = 0;
     paramType::InputMethodAppStateType inputMethodAppState = 0;
     if (ReadParam(aMsg, aIter,
@@ -457,26 +465,33 @@ struct ParamTraits<mozilla::WidgetKeyboa
         ReadParam(aMsg, aIter, &aResult->mPseudoCharCode) &&
         ReadParam(aMsg, aIter, &aResult->mAlternativeCharCodes) &&
         ReadParam(aMsg, aIter, &aResult->mIsRepeat) &&
         ReadParam(aMsg, aIter, &aResult->mIsReserved) &&
         ReadParam(aMsg, aIter, &aResult->mAccessKeyForwardedToChild) &&
         ReadParam(aMsg, aIter, &aResult->mLocation) &&
         ReadParam(aMsg, aIter, &aResult->mUniqueId) &&
         ReadParam(aMsg, aIter, &aResult->mIsSynthesizedByTIP) &&
-        ReadParam(aMsg, aIter, &inputMethodAppState)
+        ReadParam(aMsg, aIter, &inputMethodAppState) &&
 #ifdef XP_MACOSX
-        && ReadParam(aMsg, aIter, &aResult->mNativeKeyCode)
-        && ReadParam(aMsg, aIter, &aResult->mNativeModifierFlags)
-        && ReadParam(aMsg, aIter, &aResult->mNativeCharacters)
-        && ReadParam(aMsg, aIter, &aResult->mNativeCharactersIgnoringModifiers)
-        && ReadParam(aMsg, aIter, &aResult->mPluginTextEventString)
+        ReadParam(aMsg, aIter, &aResult->mNativeKeyCode) &&
+        ReadParam(aMsg, aIter, &aResult->mNativeModifierFlags) &&
+        ReadParam(aMsg, aIter, &aResult->mNativeCharacters) &&
+        ReadParam(aMsg, aIter, &aResult->mNativeCharactersIgnoringModifiers) &&
+        ReadParam(aMsg, aIter, &aResult->mPluginTextEventString) &&
 #endif
-        )
-    {
+        ReadParam(aMsg, aIter, &aResult->mEditCommandsForSingleLineEditor) &&
+        ReadParam(aMsg, aIter, &aResult->mEditCommandsForMultiLineEditor) &&
+        ReadParam(aMsg, aIter, &aResult->mEditCommandsForRichTextEditor) &&
+        ReadParam(aMsg, aIter,
+                  &aResult->mEditCommandsForSingleLineEditorInitialized) &&
+        ReadParam(aMsg, aIter,
+                  &aResult->mEditCommandsForMultiLineEditorInitialized) &&
+        ReadParam(aMsg, aIter,
+                  &aResult->mEditCommandsForRichTextEditorInitialized)) {
       aResult->mKeyNameIndex = static_cast<mozilla::KeyNameIndex>(keyNameIndex);
       aResult->mCodeNameIndex =
         static_cast<mozilla::CodeNameIndex>(codeNameIndex);
       aResult->mNativeKeyEvent = nullptr;
       aResult->mInputMethodAppState =
         static_cast<paramType::InputMethodAppState>(inputMethodAppState);
       return true;
     }