Bug 429824: Properly forward native OSX events to the native menu bar if they haven't been handled by the child process in e10s. r=mstange,masayuki
authorStephen A Pohl <spohl.mozilla.bugs@gmail.com>
Mon, 15 May 2017 22:59:35 -0400
changeset 406665 dc96471117ab6243119c8593b5f562e413036f81
parent 406664 92ab3e4bdcb957b1fed5f0495f6d9f95e7b3f3cb
child 406666 c399da282963b7c78338935b00edd14ddb639195
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange, masayuki
bugs429824
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 429824: Properly forward native OSX events to the native menu bar if they haven't been handled by the child process in e10s. r=mstange,masayuki
dom/events/EventStateManager.cpp
dom/ipc/TabChild.cpp
dom/ipc/TabParent.cpp
widget/TextEvents.h
widget/cocoa/TextInputHandler.h
widget/cocoa/TextInputHandler.mm
widget/cocoa/nsChildView.h
widget/cocoa/nsChildView.mm
widget/nsBaseWidget.cpp
widget/nsIWidget.h
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -2835,16 +2835,28 @@ void
 EventStateManager::PostHandleKeyboardEvent(WidgetKeyboardEvent* aKeyboardEvent,
                                            nsEventStatus& aStatus,
                                            bool dispatchedToContentProcess)
 {
   if (aStatus == nsEventStatus_eConsumeNoDefault) {
     return;
   }
 
+  if (!dispatchedToContentProcess) {
+    // The widget expects a reply for every keyboard event. If the event wasn't
+    // dispatched to a content process (non-e10s or no content process
+    // running), we need to short-circuit here. Otherwise, we need to wait for
+    // the content process to handle the event.
+    aKeyboardEvent->mWidget->PostHandleKeyEvent(aKeyboardEvent);
+    if (aKeyboardEvent->DefaultPrevented()) {
+      aStatus = nsEventStatus_eConsumeNoDefault;
+      return;
+    }
+  }
+
   // XXX Currently, our automated tests don't support mKeyNameIndex.
   //     Therefore, we still need to handle this with keyCode.
   switch(aKeyboardEvent->mKeyCode) {
     case NS_VK_TAB:
     case NS_VK_F6:
       // This is to prevent keyboard scrolling while alt modifier in use.
       if (!aKeyboardEvent->IsAlt()) {
         aStatus = nsEventStatus_eConsumeNoDefault;
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1968,16 +1968,17 @@ TabChild::RecvRealKeyEvent(const WidgetK
                       bindings.richTextCommands());
     } else {
       autoCache.CacheNoCommands();
     }
   }
 
   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
   // some incoming events in case event handling took long time.
   UpdateRepeatedKeyEventEndTime(localEvent);
 
   if (aEvent.mMessage == eKeyDown) {
     mIgnoreKeyPressEvent = status == nsEventStatus_eConsumeNoDefault;
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1997,16 +1997,26 @@ TabParent::RecvReplyKeyEvent(const Widge
   NS_ENSURE_TRUE(presShell, IPC_OK());
   nsPresContext* presContext = presShell->GetPresContext();
   NS_ENSURE_TRUE(presContext, IPC_OK());
 
   AutoHandlingUserInputStatePusher userInpStatePusher(localEvent.IsTrusted(),
                                                       &localEvent, doc);
 
   EventDispatcher::Dispatch(mFrameElement, presContext, &localEvent);
+
+  if (!localEvent.DefaultPrevented() &&
+      !localEvent.mFlags.mIsSynthesizedForTests) {
+    nsCOMPtr<nsIWidget> widget = GetWidget();
+    if (widget) {
+      widget->PostHandleKeyEvent(&localEvent);
+      localEvent.StopPropagation();
+    }
+  }
+
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvAccessKeyNotHandled(const WidgetKeyboardEvent& aEvent)
 {
   NS_ENSURE_TRUE(mFrameElement, IPC_OK());
 
--- a/widget/TextEvents.h
+++ b/widget/TextEvents.h
@@ -235,19 +235,17 @@ public:
   uint32_t mPseudoCharCode;
   // One of eKeyLocation*
   uint32_t mLocation;
   // True if accesskey handling was forwarded to the child via
   // TabParent::HandleAccessKey. In this case, parent process menu access key
   // handling should be delayed until it is determined that there exists no
   // overriding access key in the content process.
   bool mAccessKeyForwardedToChild;
-  // Unique id associated with a keydown / keypress event. Used in identifing
-  // keypress events for removal from async event dispatch queue in metrofx
-  // after preventDefault is called on keydown events. It's ok if this wraps
+  // Unique id associated with a keydown / keypress event. It's ok if this wraps
   // over long periods.
   uint32_t mUniqueId;
 
 #ifdef XP_MACOSX
   // Values given by a native NSEvent, for use with Cocoa NPAPI plugins.
   uint32_t mNativeModifierFlags;
   uint16_t mNativeKeyCode;
 #endif // #ifdef XP_MACOSX
--- a/widget/cocoa/TextInputHandler.h
+++ b/widget/cocoa/TextInputHandler.h
@@ -524,59 +524,68 @@ protected:
     // Handling native key event
     NSEvent* mKeyEvent;
     // String specified by InsertText().  This is not null only during a
     // call of InsertText().
     nsAString* mInsertString;
     // String which are included in [mKeyEvent characters] and already handled
     // by InsertText() call(s).
     nsString mInsertedString;
+    // Unique id associated with a keydown / keypress event. It's ok if this
+    // wraps over long periods.
+    uint32_t mUniqueId;
     // Whether keydown event was consumed by web contents or chrome contents.
     bool mKeyDownHandled;
     // Whether keypress event was dispatched for mKeyEvent.
     bool mKeyPressDispatched;
     // Whether keypress event was consumed by web contents or chrome contents.
     bool mKeyPressHandled;
     // Whether the key event causes other key events via IME or something.
     bool mCausedOtherKeyEvents;
     // Whether the key event causes composition change or committing
     // composition.  So, even if InsertText() is called, this may be false
     // if it dispatches keypress event.
     bool mCompositionDispatched;
 
-    KeyEventState() : mKeyEvent(nullptr)
+    KeyEventState()
+      : mKeyEvent(nullptr)
+      , mUniqueId(0)
     {
       Clear();
-    }    
+    }
 
-    explicit KeyEventState(NSEvent* aNativeKeyEvent) : mKeyEvent(nullptr)
+    explicit KeyEventState(NSEvent* aNativeKeyEvent, uint32_t aUniqueId = 0)
+      : mKeyEvent(nullptr)
+      , mUniqueId(0)
     {
       Clear();
-      Set(aNativeKeyEvent);
+      Set(aNativeKeyEvent, aUniqueId);
     }
 
     KeyEventState(const KeyEventState &aOther) = delete;
 
     ~KeyEventState()
     {
       Clear();
     }
 
-    void Set(NSEvent* aNativeKeyEvent)
+    void Set(NSEvent* aNativeKeyEvent, uint32_t aUniqueId = 0)
     {
       NS_PRECONDITION(aNativeKeyEvent, "aNativeKeyEvent must not be NULL");
       Clear();
       mKeyEvent = [aNativeKeyEvent retain];
+      mUniqueId = aUniqueId;
     }
 
     void Clear()
     {
       if (mKeyEvent) {
         [mKeyEvent release];
         mKeyEvent = nullptr;
+        mUniqueId = 0;
       }
       mInsertString = nullptr;
       mInsertedString.Truncate();
       mKeyDownHandled = false;
       mKeyPressDispatched = false;
       mKeyPressHandled = false;
       mCausedOtherKeyEvents = false;
       mCompositionDispatched = false;
@@ -665,31 +674,31 @@ protected:
    * mFirstKeyEvent must be used for first key event.  This member prevents
    * memory fragmentation for most key events.
    */
   KeyEventState mFirstKeyEvent;
 
   /**
    * PushKeyEvent() adds the current key event to mCurrentKeyEvents.
    */
-  KeyEventState* PushKeyEvent(NSEvent* aNativeKeyEvent)
+  KeyEventState* PushKeyEvent(NSEvent* aNativeKeyEvent, uint32_t aUniqueId = 0)
   {
     uint32_t nestCount = mCurrentKeyEvents.Length();
     for (uint32_t i = 0; i < nestCount; i++) {
       // When the key event is caused by another key event, all key events
       // which are being handled should be marked as "consumed".
       mCurrentKeyEvents[i]->mCausedOtherKeyEvents = true;
     }
 
     KeyEventState* keyEvent = nullptr;
     if (nestCount == 0) {
-      mFirstKeyEvent.Set(aNativeKeyEvent);
+      mFirstKeyEvent.Set(aNativeKeyEvent, aUniqueId);
       keyEvent = &mFirstKeyEvent;
     } else {
-      keyEvent = new KeyEventState(aNativeKeyEvent);
+      keyEvent = new KeyEventState(aNativeKeyEvent, aUniqueId);
     }
     return *mCurrentKeyEvents.AppendElement(keyEvent);
   }
 
   /**
    * RemoveCurrentKeyEvent() removes the current key event from
    * mCurrentKeyEvents.
    */
@@ -1106,20 +1115,21 @@ public:
 
   TextInputHandler(nsChildView* aWidget, NSView<mozView> *aNativeView);
   virtual ~TextInputHandler();
 
   /**
    * KeyDown event handler.
    *
    * @param aNativeEvent          A native NSKeyDown event.
-   * @return                      TRUE if the event is consumed by web contents
-   *                              or chrome contents.  Otherwise, FALSE.
+   * @param aUniqueId             A unique ID for the event.
+   * @return                      TRUE if the event is dispatched to web
+   *                              contents or chrome contents. Otherwise, FALSE.
    */
-  bool HandleKeyDownEvent(NSEvent* aNativeEvent);
+  bool HandleKeyDownEvent(NSEvent* aNativeEvent, uint32_t aUniqueId);
 
   /**
    * KeyUp event handler.
    *
    * @param aNativeEvent          A native NSKeyUp event.
    */
   void HandleKeyUpEvent(NSEvent* aNativeEvent);
 
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -1561,17 +1561,18 @@ TextInputHandler::TextInputHandler(nsChi
 }
 
 TextInputHandler::~TextInputHandler()
 {
   [mView uninstallTextInputHandler];
 }
 
 bool
-TextInputHandler::HandleKeyDownEvent(NSEvent* aNativeEvent)
+TextInputHandler::HandleKeyDownEvent(NSEvent* aNativeEvent,
+                                     uint32_t aUniqueId)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN;
 
   if (Destroyed()) {
     MOZ_LOG(gLog, LogLevel::Info,
       ("%p TextInputHandler::HandleKeyDownEvent, "
        "widget has been already destroyed", this));
     return false;
@@ -1596,17 +1597,17 @@ TextInputHandler::HandleKeyDownEvent(NSE
   //   even without dispatching eKeyPress events)
   // - Hide mouse cursor even when a plugin has focus
   if (!([aNativeEvent modifierFlags] & NSCommandKeyMask)) {
     [NSCursor setHiddenUntilMouseMoves:YES];
   }
 
   RefPtr<nsChildView> widget(mWidget);
 
-  KeyEventState* currentKeyEvent = PushKeyEvent(aNativeEvent);
+  KeyEventState* currentKeyEvent = PushKeyEvent(aNativeEvent, aUniqueId);
   AutoKeyEventStateCleaner remover(this);
 
   ComplexTextInputPanel* ctiPanel = ComplexTextInputPanel::GetSharedComplexTextInputPanel();
   if (ctiPanel && ctiPanel->IsInComposition()) {
     nsAutoString committed;
     ctiPanel->InterpretKeyEvent(aNativeEvent, committed);
     if (!committed.IsEmpty()) {
       nsresult rv = mDispatcher->BeginNativeInputTransaction();
@@ -2802,16 +2803,22 @@ IMEInputHandler::WillDispatchKeyboardEve
   // nothing here.
   if (!aData) {
     return;
   }
 
   KeyEventState* currentKeyEvent = static_cast<KeyEventState*>(aData);
   NSEvent* nativeEvent = currentKeyEvent->mKeyEvent;
   nsAString* insertString = currentKeyEvent->mInsertString;
+  if (aKeyboardEvent.mMessage == eKeyPress && aIndexOfKeypress == 0 &&
+      (!insertString || insertString->IsEmpty())) {
+    // Inform the child process that this is an event that we want a reply
+    // from.
+    aKeyboardEvent.mFlags.mWantReplyFromContentProcess = true;
+  }
   if (KeyboardLayoutOverrideRef().mOverrideEnabled) {
     TISInputSourceWrapper tis;
     tis.InitByLayoutID(KeyboardLayoutOverrideRef().mKeyboardLayout, true);
     tis.WillDispatchKeyboardEvent(nativeEvent, insertString, aKeyboardEvent);
     return;
   }
   TISInputSourceWrapper::CurrentInputSource().
     WillDispatchKeyboardEvent(nativeEvent, insertString, aKeyboardEvent);
@@ -4701,16 +4708,17 @@ TextInputHandlerBase::KeyEventState::Ini
                    windowNumber:[mKeyEvent windowNumber]
                         context:[mKeyEvent context]
                      characters:unhandledNSString
     charactersIgnoringModifiers:[mKeyEvent charactersIgnoringModifiers]
                       isARepeat:[mKeyEvent isARepeat]
                         keyCode:[mKeyEvent keyCode]];
   }
 
+  aKeyEvent.mUniqueId = mUniqueId;
   aHandler->InitKeyEvent(nativeEvent, aKeyEvent, mInsertString);
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 void
 TextInputHandlerBase::KeyEventState::GetUnhandledString(
                                        nsAString& aUnhandledString) const
--- a/widget/cocoa/nsChildView.h
+++ b/widget/cocoa/nsChildView.h
@@ -257,16 +257,20 @@ class WidgetRenderingContext;
 - (void)endGestureWithEvent:(NSEvent *)anEvent;
 
 - (void)scrollWheel:(NSEvent *)anEvent;
 - (void)handleAsyncScrollEvent:(CGEventRef)cgEvent ofType:(CGEventType)type;
 
 - (void)setUsingOMTCompositor:(BOOL)aUseOMTC;
 
 - (NSEvent*)lastKeyDownEvent;
+
++ (uint32_t)sUniqueKeyEventId;
+
++ (NSMutableDictionary*)sNativeKeyEventsMap;
 @end
 
 class ChildViewMouseTracker {
 
 public:
 
   static void MouseMoved(NSEvent* aEvent);
   static void MouseScrolled(NSEvent* aEvent);
@@ -370,16 +374,18 @@ public:
 
   virtual nsresult  SetTitle(const nsAString& title) override;
 
   virtual MOZ_MUST_USE nsresult
                     GetAttention(int32_t aCycleCount) override;
 
   virtual bool HasPendingInputEvent() override;
 
+  bool              SendEventToNativeMenuSystem(NSEvent* aEvent);
+  virtual void      PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent) override;
   virtual nsresult  ActivateNativeMenuItemAt(const nsAString& indexString) override;
   virtual nsresult  ForceUpdateNativeMenuAt(const nsAString& indexString) override;
   virtual MOZ_MUST_USE nsresult
                     GetSelectionAsPlaintext(nsAString& aResult) override;
 
   virtual void SetInputContext(const InputContext& aContext,
                                const InputContextAction& aAction) override;
   virtual InputContext GetInputContext() override;
--- a/widget/cocoa/nsChildView.mm
+++ b/widget/cocoa/nsChildView.mm
@@ -149,16 +149,21 @@ static void blinkRgn(RgnHandle rgn);
 bool gUserCancelledDrag = false;
 
 uint32_t nsChildView::sLastInputEventCount = 0;
 
 static uint32_t gNumberOfWidgetsNeedingEventThread = 0;
 
 static bool sIsTabletPointerActivated = false;
 
+static uint32_t sUniqueKeyEventId = 0;
+
+static NSMutableDictionary* sNativeKeyEventsMap =
+  [NSMutableDictionary dictionary];
+
 @interface ChildView(Private)
 
 // sets up our view, attaching it to its owning gecko view
 - (id)initWithFrame:(NSRect)inFrame geckoChild:(nsChildView*)inChild;
 - (void)forceRefreshOpenGL;
 
 // set up a gecko mouse event based on a cocoa mouse event
 - (void) convertCocoaMouseWheelEvent:(NSEvent*)aMouseEvent
@@ -1244,16 +1249,60 @@ static NSMenuItem* NativeMenuItemWithLoc
       else
         return nil;
     }
   }
 
   return nil;
 }
 
+bool
+nsChildView::SendEventToNativeMenuSystem(NSEvent* aEvent)
+{
+  bool handled = false;
+  nsCocoaWindow* widget = GetXULWindowWidget();
+  if (widget) {
+    nsMenuBarX* mb = widget->GetMenuBar();
+    if (mb) {
+      // Check if main menu wants to handle the event.
+      handled = mb->PerformKeyEquivalent(aEvent);
+    }
+  }
+
+  if (!handled && sApplicationMenu) {
+    // Check if application menu wants to handle the event.
+    handled = [sApplicationMenu performKeyEquivalent:aEvent];
+  }
+
+  return handled;
+}
+
+void
+nsChildView::PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent)
+{
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
+
+  // We always allow keyboard events to propagate to keyDown: but if they are
+  // not handled we give menu items a chance to act. This allows for handling of
+  // custom shortcuts. Note that existing shortcuts cannot be reassigned yet and
+  // will have been handled by keyDown: before we get here.
+  NSEvent* cocoaEvent =
+    [sNativeKeyEventsMap objectForKey:@(aEvent->mUniqueId)];
+  [sNativeKeyEventsMap removeObjectForKey:@(aEvent->mUniqueId)];
+  if (!cocoaEvent) {
+    return;
+  }
+
+  if (SendEventToNativeMenuSystem(cocoaEvent)) {
+    aEvent->PreventDefault();
+  }
+
+  NS_OBJC_END_TRY_ABORT_BLOCK;
+}
+
 // Used for testing native menu system structure and event handling.
 nsresult
 nsChildView::ActivateNativeMenuItemAt(const nsAString& indexString)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
 
   NSString* locationString = [NSString stringWithCharacters:reinterpret_cast<const unichar*>(indexString.BeginReading())
                                                      length:indexString.Length()];
@@ -5544,39 +5593,31 @@ GetIntegerDeltaForEvent(NSEvent* aEvent)
 #endif
       MOZ_CRASH(CRASH_MESSAGE);
       #undef CRASH_MESSAGE
     }
   }
 #endif // #if !defined(RELEASE_OR_BETA) || defined(DEBUG)
 
   nsAutoRetainCocoaObject kungFuDeathGrip(self);
-  bool handled = false;
-  if (mGeckoChild && mTextInputHandler) {
-    handled = mTextInputHandler->HandleKeyDownEvent(theEvent);
-  }
-
-  // We always allow keyboard events to propagate to keyDown: but if they are
-  // not handled we give menu items a chance to act. This allows for handling of
-  // custom shortcuts. Note that existing shortcuts cannot be reassigned yet and
-  // will have been handled by keyDown: before we get here.
-  if (!handled && mGeckoChild) {
-    nsCocoaWindow* widget = mGeckoChild->GetXULWindowWidget();
-    if (widget) {
-      nsMenuBarX* mb = widget->GetMenuBar();
-      if (mb) {
-        // Check if main menu wants to handle the event.
-        handled = mb->PerformKeyEquivalent(theEvent);
-      }
+  if (mGeckoChild) {
+    if (mTextInputHandler) {
+      sUniqueKeyEventId++;
+      [sNativeKeyEventsMap setObject:theEvent forKey:@(sUniqueKeyEventId)];
+      // Purge old native events, in case we're still holding on to them. We
+      // keep at most 10 references to 10 different native events.
+      [sNativeKeyEventsMap removeObjectForKey:@(sUniqueKeyEventId - 10)];
+      mTextInputHandler->HandleKeyDownEvent(theEvent, sUniqueKeyEventId);
+    } else {
+      // There was no text input handler. Offer the event to the native menu
+      // system to check if there are any registered custom shortcuts for this
+      // event.
+      mGeckoChild->SendEventToNativeMenuSystem(theEvent);
     }
   }
-  if (!handled && sApplicationMenu) {
-    // Check if application menu wants to handle the event.
-    handled = [sApplicationMenu performKeyEquivalent:theEvent];
-  }
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 - (void)keyUp:(NSEvent*)theEvent
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
@@ -6494,16 +6535,20 @@ nsChildView::GetSelectionAsPlaintext(nsA
 
   return [accessible accessibilityAttributeValue:attribute];
 
   NS_OBJC_END_TRY_ABORT_BLOCK_NIL;
 }
 
 #endif /* ACCESSIBILITY */
 
++ (uint32_t)sUniqueKeyEventId { return sUniqueKeyEventId; }
+
++ (NSMutableDictionary*)sNativeKeyEventsMap { return sNativeKeyEventsMap; }
+
 @end
 
 #pragma mark -
 
 void
 ChildViewMouseTracker::OnDestroyView(ChildView* aView)
 {
   if (sLastMouseEventView == aView) {
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -2278,16 +2278,21 @@ nsIWidget::IMENotificationRequestsRef()
 
 nsresult
 nsIWidget::OnWindowedPluginKeyEvent(const NativeEventData& aKeyEventData,
                                     nsIKeyEventInPluginCallback* aCallback)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+void
+nsIWidget::PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent)
+{
+}
+
 namespace mozilla {
 namespace widget {
 
 const char*
 ToChar(IMEMessage aIMEMessage)
 {
   switch (aIMEMessage) {
     case NOTIFY_IME_OF_NOTHING:
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@@ -1683,16 +1683,24 @@ private:
   static void OnLongTapTimerCallback(nsITimer* aTimer, void* aClosure);
 
   mozilla::UniquePtr<LongTapInfo> mLongTapTouchPoint;
   nsCOMPtr<nsITimer> mLongTapTimer;
   static int32_t sPointerIdCounter;
 
 public:
     /**
+     * If key events have not been handled by content or XBL handlers, they can
+     * be offered to the system (for custom application shortcuts set in system
+     * preferences, for example).
+     */
+    virtual void
+    PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent);
+
+    /**
      * Activates a native menu item at the position specified by the index
      * string. The index string is a string of positive integers separated
      * by the "|" (pipe) character. The last integer in the string represents
      * the item index in a submenu located using the integers preceding it.
      *
      * Example: 1|0|4
      * In this string, the first integer represents the top-level submenu
      * in the native menu bar. Since the integer is 1, it is the second submeu