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

buildflags/BUILD.gn

@@ -19,6 +19,7 @@ buildflag_header("buildflags") {
     "ENABLE_COLOR_CHOOSER=$enable_color_chooser",
     "ENABLE_ELECTRON_EXTENSIONS=$enable_electron_extensions",
     "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

@@ -20,6 +20,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,18 @@ 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,
-which also defines a bunch of other stuff that depends on extensions.
-since we only need IsMediaKey, and we don't want the extensions stuff
-(and aren't compiling command.cc), it's safe to duplicate the
-definition. A candidate for upstreaming would be to move the IsMediaKey
-function into //ui.
+Override MediaKeysListener::IsMediaKeycode to also listen for Volume Up, Volume Down,
+and Mute. We also need to patch out Chromium's usage of RemoteCommandCenterDelegate, as
+it uses MPRemoteCommandCenter. MPRemoteCommandCenter makes it such that GlobalShortcuts
+in Electron will not work as intended, because by design an app does not receive remote
+control events until it begins playing audio. This means that a media shortcut would not kick
+into effect until you, for example, began playing a YouTube video which sort of defeats the
+purpose of GlobalShortcuts.
 
-Also apply electron/electron@0f67b1866a9f00b852370e721affa4efda623f3a
-and electron/electron@d2368d2d3b3de9eec4cc32b6aaf035cc89921bf1 as
-patches.
+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.
 
-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,10 +57,34 @@ 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..e319e7a3e34e05b0e96d4a2dbb456355f327137a 100644
+--- a/ui/base/accelerators/media_keys_listener.cc
++++ b/ui/base/accelerators/media_keys_listener.cc
+@@ -13,7 +13,9 @@ 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 f4e3126a4efd66f05c4f13e40ba23db10b8cca96..bb4c1a891dd13855227b39a0e582fd4dbc342ec9 100644
 --- a/ui/base/accelerators/media_keys_listener_mac.mm
 +++ b/ui/base/accelerators/media_keys_listener_mac.mm
+@@ -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/remote_command_media_keys_listener_mac.h"
+ #include "ui/base/now_playing/remote_command_center_delegate.h"
 @@ -33,6 +33,12 @@ KeyboardCode MediaKeyCodeToKeyboardCode(int key_code) {
      case NX_KEYTYPE_NEXT:
      case NX_KEYTYPE_FAST:
@@ -116,3 +110,20 @@ index f4e3126a4efd66f05c4f13e40ba23db10b8cca96..bb4c1a891dd13855227b39a0e582fd4d
      return event;
    }
 
+@@ -233,6 +234,7 @@ 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 (@available(macOS 10.12.2, *)) {
+     if (scope == Scope::kGlobal &&
+         now_playing::RemoteCommandCenterDelegate::GetInstance()) {
+@@ -242,7 +244,7 @@ static CGEventRef EventTapCallback(CGEventTapProxy proxy,
+       return std::move(listener);
+     }
+   }
+-
++#endif
+   return std::make_unique<MediaKeysListenerImpl>(delegate, scope);
+ }
+