fix: MediaKey globalShortcuts not working on macOS (#21442)

buildflags/BUILD.gn

@@ -21,6 +21,7 @@ buildflag_header("buildflags") {
     "ENABLE_ELECTRON_EXTENSIONS=$enable_electron_extensions",
     "ENABLE_BUILTIN_SPELLCHECKER=$enable_builtin_spellchecker",
     "ENABLE_PICTURE_IN_PICTURE=$enable_picture_in_picture",
+    "ENABLE_MEDIA_KEY_OVERRIDES=$enable_media_key_overrides",
     "OVERRIDE_LOCATION_PROVIDER=$enable_fake_location_provider",
   ]
 }

buildflags/buildflags.gni

@@ -22,6 +22,8 @@ declare_args() {
 
   enable_picture_in_picture = true
 
+  enable_media_key_overrides = true
+
   # Provide a fake location provider for mocking
   # the geolocation responses. Disable it if you
   # need to test with chromium's location provider.

patches/chromium/command-ismediakey.patch

@@ -3,48 +3,22 @@ From: Jeremy Apthorp <jeremya@chromium.org>
 Date: Wed, 10 Oct 2018 15:07:34 -0700
 Subject: command-ismediakey.patch
 
-define Command::IsMediaKey on mac; copied from //chrome/common/extensions/command.cc,
+Override MediaKeysListener::IsMediaKeycode to also listen for Volume Up, Volume Down,
-which also defines a bunch of other stuff that depends on extensions.
+and Mute. We also need to patch out Chromium's usage of RemoteCommandCenterDelegate, as
-since we only need IsMediaKey, and we don't want the extensions stuff
+it uses MPRemoteCommandCenter. MPRemoteCommandCenter makes it such that GlobalShortcuts 
-(and aren't compiling command.cc), it's safe to duplicate the
+in Electron will not work as intended, because by design an app does not receive remote
-definition. A candidate for upstreaming would be to move the IsMediaKey
+control events until it begins playing audio. This means that a media shortcut would not kick
-function into //ui.
+into effect until you, for example, began playing a YouTube video which sort of defeats the
+purpose of GlobalShortcuts.
+
+At the moment there is no upstream possibility for this; but perhaps Chromium may 
+consider some kind of switch, enabled by default, which would conditionally choose to avoid usage of
+RemoteCommandCenterDelegate on macOS.
 
 Also apply electron/electron@0f67b1866a9f00b852370e721affa4efda623f3a
 and electron/electron@d2368d2d3b3de9eec4cc32b6aaf035cc89921bf1 as
 patches.
 
-diff --git a/chrome/browser/extensions/global_shortcut_listener_mac.mm b/chrome/browser/extensions/global_shortcut_listener_mac.mm
-index befe726af9c10b1563a7fc0bb77cc55f65943d5c..46c6fe08bab8471007f78d3ef227e5195bfdf0e1 100644
---- a/chrome/browser/extensions/global_shortcut_listener_mac.mm
-+++ b/chrome/browser/extensions/global_shortcut_listener_mac.mm
-@@ -21,6 +21,26 @@
- 
- namespace extensions {
- 
-+// NOTE: this is defined in command.cc, but command.cc is full of
-+// chrome-extensions-specific logic that we don't want to depend on.
-+// Since we don't build command.cc in Electron, it's safe to re-define this
-+// function here. Ideally, though, `IsMediaKey` would be the responsibility of
-+// `ui::Accelerator`, rather than `extensions::Command`.
-+
-+// static
-+bool Command::IsMediaKey(const ui::Accelerator& accelerator) {
-+  if (accelerator.modifiers() != 0)
-+    return false;
-+
-+  return (accelerator.key_code() == ui::VKEY_MEDIA_NEXT_TRACK ||
-+          accelerator.key_code() == ui::VKEY_MEDIA_PREV_TRACK ||
-+          accelerator.key_code() == ui::VKEY_MEDIA_PLAY_PAUSE ||
-+          accelerator.key_code() == ui::VKEY_MEDIA_STOP ||
-+          accelerator.key_code() == ui::VKEY_VOLUME_UP ||
-+          accelerator.key_code() == ui::VKEY_VOLUME_DOWN ||
-+          accelerator.key_code() == ui::VKEY_VOLUME_MUTE);
-+}
-+
- // static
- GlobalShortcutListener* GlobalShortcutListener::GetInstance() {
-   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 diff --git a/chrome/browser/extensions/global_shortcut_listener_win.cc b/chrome/browser/extensions/global_shortcut_listener_win.cc
 index c5125495b4d178ffb18be4d2d9670f7556412cbd..cddb321abb938c667a4a2089f87eab999510e9b1 100644
 --- a/chrome/browser/extensions/global_shortcut_listener_win.cc
@@ -87,11 +61,33 @@ index 392cf3d58c64c088596e8d321a2ce37b0ec60b6e..43e30f47240dc10a3a9b950255d4e487
  
    ui::Accelerator accelerator(
        ui::KeyboardCodeFromXKeyEvent(x_event), modifiers);
+diff --git a/ui/base/accelerators/media_keys_listener.cc b/ui/base/accelerators/media_keys_listener.cc
+index 1145e1f3d79482b5bb468c3128431ac674310e5f..e9f595045e0c076e0735f27dfc38bfbc7951d372 100644
+--- a/ui/base/accelerators/media_keys_listener.cc
++++ b/ui/base/accelerators/media_keys_listener.cc
+@@ -13,7 +13,8 @@ MediaKeysListener::~MediaKeysListener() = default;
+ // static
+ bool MediaKeysListener::IsMediaKeycode(KeyboardCode key_code) {
+   return key_code == VKEY_MEDIA_PLAY_PAUSE || key_code == VKEY_MEDIA_STOP ||
+-         key_code == VKEY_MEDIA_PREV_TRACK || key_code == VKEY_MEDIA_NEXT_TRACK;
++         key_code == VKEY_MEDIA_PREV_TRACK || key_code == VKEY_MEDIA_NEXT_TRACK ||
++         key_code == VKEY_VOLUME_UP || key_code == VKEY_VOLUME_DOWN || key_code == VKEY_VOLUME_MUTE;
+ }
+ 
+ }  // namespace ui
 diff --git a/ui/base/accelerators/media_keys_listener_mac.mm b/ui/base/accelerators/media_keys_listener_mac.mm
-index 85378bb565de617b1bd611d28c8714361747a357..36de4c0b0353be2418dacd388e92d7c38a7ee139 100644
+index f4e3126a4efd66f05c4f13e40ba23db10b8cca96..9b4d1895731f758c09e5cb8350d1f225ab0f8a71 100644
 --- a/ui/base/accelerators/media_keys_listener_mac.mm
 +++ b/ui/base/accelerators/media_keys_listener_mac.mm
-@@ -32,6 +32,12 @@ KeyboardCode MediaKeyCodeToKeyboardCode(int key_code) {
+@@ -11,6 +11,7 @@
+ #include <IOKit/hidsystem/ev_keymap.h>
+ 
+ #include "base/containers/flat_set.h"
++#include "electron/buildflags/buildflags.h"
+ #include "ui/base/accelerators/accelerator.h"
+ #include "ui/base/accelerators/system_media_controls_media_keys_listener.h"
+
+@@ -33,6 +33,12 @@ KeyboardCode MediaKeyCodeToKeyboardCode(int key_code) {
      case NX_KEYTYPE_NEXT:
      case NX_KEYTYPE_FAST:
        return VKEY_MEDIA_NEXT_TRACK;
@@ -116,3 +112,18 @@ index 85378bb565de617b1bd611d28c8714361747a357..36de4c0b0353be2418dacd388e92d7c3
      return event;
    }
  
+@@ -223,12 +223,14 @@ static CGEventRef EventTapCallback(CGEventTapProxy proxy,
+   // For Mac OS 10.12.2 or later, we want to use MPRemoteCommandCenter for
+   // getting media keys globally if there is a RemoteCommandCenterDelegate
+   // available.
++#if !BUILDFLAG(ENABLE_MEDIA_KEY_OVERRIDES)
+   if (scope == Scope::kGlobal) {
+     auto listener =
+         std::make_unique<SystemMediaControlsMediaKeysListener>(delegate);
+     if (listener->Initialize())
+       return listener;
+   }
++#endif
+ 
+   return std::make_unique<MediaKeysListenerImpl>(delegate, scope);
+ }