Bug 1333459 - part2-3: Make nsMenuBarListener::KeyPress() wait reply from remote process if the eKeyPress event will be sent to a remote process later r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 19 Jul 2017 18:39:34 +0900
changeset 613983 9ece1905194d14105847453cb589abf9105bc939
parent 613982 5d078048caa4c28c41e44579e1ec622a4869686e
child 613984 f2e79675ac28c353b0933b8df5c6c45bc6173968
push id69882
push usermasayuki@d-toybox.com
push dateSun, 23 Jul 2017 15:20:52 +0000
reviewerssmaug
bugs1333459
milestone56.0a1
Bug 1333459 - part2-3: Make nsMenuBarListener::KeyPress() wait reply from remote process if the eKeyPress event will be sent to a remote process later r?smaug nsMenuBarListener::KeyPress() is eKeyEvent listener in the system event group. If the target is a remote process, it shouldn't handle accesskey immediately because preceding eKeyDown event may be consumed in the remote process or eKeyPress event itself may be consumed in the remote process. This patch makes nsMenuBarListener::KeyPress() mark eKeyPress event as "waiting reply from remote process" only when the event matches with a menu item's accesskey and it will be send to a remote process later. Then, reply event should be handled in this method if it's available. MozReview-Commit-ID: KOpCVgElnca
dom/xbl/nsXBLWindowKeyHandler.cpp
layout/xul/nsMenuBarListener.cpp
widget/BasicEvents.h
widget/WidgetEventImpl.cpp
--- a/dom/xbl/nsXBLWindowKeyHandler.cpp
+++ b/dom/xbl/nsXBLWindowKeyHandler.cpp
@@ -528,24 +528,20 @@ nsXBLWindowKeyHandler::HandleEventOnCapt
 
 void
 nsXBLWindowKeyHandler::HandleEventOnCaptureInSystemEventGroup(
                          nsIDOMKeyEvent* aEvent)
 {
   WidgetKeyboardEvent* widgetEvent =
     aEvent->AsEvent()->WidgetEventPtr()->AsKeyboardEvent();
 
-  if (widgetEvent->IsCrossProcessForwardingStopped() ||
-      widgetEvent->mFlags.mOnlySystemGroupDispatchInContent) {
-    return;
-  }
-
-  nsCOMPtr<mozilla::dom::Element> originalTarget =
-    do_QueryInterface(aEvent->AsEvent()->WidgetEventPtr()->mOriginalTarget);
-  if (!EventStateManager::IsRemoteTarget(originalTarget)) {
+  // If the event won't be sent to remote process, this listener needs to do
+  // nothing.
+  if (widgetEvent->mFlags.mOnlySystemGroupDispatchInContent ||
+      !widgetEvent->WillBeSentToRemoteProcess()) {
     return;
   }
 
   if (!HasHandlerForEvent(aEvent)) {
     return;
   }
 
   // If this event wasn't marked as IsCrossProcessForwardingStopped,
--- a/layout/xul/nsMenuBarListener.cpp
+++ b/layout/xul/nsMenuBarListener.cpp
@@ -295,19 +295,27 @@ nsMenuBarListener::KeyPress(nsIDOMEvent*
     }
 
     if (IsAccessKeyPressed(keyEvent) && hasAccessKeyCandidates) {
       // Do shortcut navigation.
       // A letter was pressed. We want to see if a shortcut gets matched. If
       // so, we'll know the menu got activated.
       nsMenuFrame* result = mMenuBarFrame->FindMenuWithShortcut(keyEvent);
       if (result) {
-        // TODO: If the event target is a remote process and not stopped
-        //       sending the event to it, we need to mark the event as
-        //       waiting reply from remote process and stop handling the event.
+        // If the keyboard event matches with a menu item's accesskey and
+        // will be sent to a remote process, it should be executed with
+        // reply event from the focused remote process.  Note that if the
+        // menubar is active, the event is already marked as "stop cross
+        // process dispatching".  So, in that case, this won't wait
+        // reply from the remote content.
+        if (nativeKeyEvent->WillBeSentToRemoteProcess()) {
+          nativeKeyEvent->StopImmediatePropagation();
+          nativeKeyEvent->MarkAsWaitingReplyFromRemoteProcess();
+          return NS_OK;
+        }
         mMenuBarFrame->SetActiveByKeyboard();
         mMenuBarFrame->SetActive(true);
         result->OpenMenu(true);
 
         // The opened menu will listen next keyup event.
         // Therefore, we should clear the keydown flags here.
         mAccessKeyDown = mAccessKeyDownCanceled = false;
 
--- a/widget/BasicEvents.h
+++ b/widget/BasicEvents.h
@@ -784,16 +784,21 @@ public:
    */
   bool HasPluginActivationEventMessage() const;
 
   /**
    * Returns true if the event can be sent to remote process.
    */
   bool CanBeSentToRemoteProcess() const;
   /**
+   * Returns true if the original target is a remote process and the event
+   * will be posted to the remote process later.
+   */
+  bool WillBeSentToRemoteProcess() const;
+  /**
    * Returns true if the event is native event deliverer event for plugin and
    * it should be retarted to focused document.
    */
   bool IsRetargetedNativeEventDelivererForPlugin() const;
   /**
    * Returns true if the event is native event deliverer event for plugin and
    * it should NOT be retarted to focused document.
    */
--- a/widget/WidgetEventImpl.cpp
+++ b/widget/WidgetEventImpl.cpp
@@ -1,22 +1,24 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gfxPrefs.h"
 #include "mozilla/BasicEvents.h"
 #include "mozilla/ContentEvents.h"
+#include "mozilla/EventStateManager.h"
 #include "mozilla/InternalMutationEvent.h"
 #include "mozilla/MiscEvents.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/TextEvents.h"
 #include "mozilla/TouchEvents.h"
+#include "nsIContent.h"
 #include "nsIDOMEventTarget.h"
 #include "nsPrintfCString.h"
 
 namespace mozilla {
 
 /******************************************************************************
  * Global helper methods
  ******************************************************************************/
@@ -345,16 +347,34 @@ WidgetEvent::CanBeSentToRemoteProcess() 
     case eDrop:
       return true;
     default:
       return false;
   }
 }
 
 bool
+WidgetEvent::WillBeSentToRemoteProcess() const
+{
+  // This event won't be posted to remote process if it's already explicitly
+  // stopped.
+  if (IsCrossProcessForwardingStopped()) {
+    return false;
+  }
+
+  // When mOriginalTarget is nullptr, this method shouldn't be used.
+  if (NS_WARN_IF(!mOriginalTarget)) {
+    return false;
+  }
+
+  nsCOMPtr<nsIContent> originalTarget = do_QueryInterface(mOriginalTarget);
+  return EventStateManager::IsRemoteTarget(originalTarget);
+}
+
+bool
 WidgetEvent::IsRetargetedNativeEventDelivererForPlugin() const
 {
   const WidgetPluginEvent* pluginEvent = AsPluginEvent();
   return pluginEvent && pluginEvent->mRetargetToFocusedDocument;
 }
 
 bool
 WidgetEvent::IsNonRetargetedNativeEventDelivererForPlugin() const