fix: volume key globalShortcut registration (#23824)

Co-authored-by: Shelley Vohr <[email protected]>

patches/chromium/command-ismediakey.patch

@@ -3,17 +3,8 @@ From: Jeremy Apthorp <[email protected]>
 Date: Wed, 10 Oct 2018 15:07:34 -0700
 Subject: command-ismediakey.patch
 
-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.
-
-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.
+Override MediaKeysListener::IsMediaKeycode and associated functions to also listen for
+Volume Up, Volume Down, and Mute.
 
 Also apply electron/electron@0f67b1866a9f00b852370e721affa4efda623f3a
 and electron/electron@d2368d2d3b3de9eec4cc32b6aaf035cc89921bf1 as
@@ -95,3 +86,19 @@ index 85378bb565de617b1bd611d28c8714361747a357..36de4c0b0353be2418dacd388e92d7c3
      return event;
    }
  
+diff --git a/ui/base/accelerators/system_media_controls_media_keys_listener.cc b/ui/base/accelerators/system_media_controls_media_keys_listener.cc
+index 9d6084ceaccfd071549e63e3015f55ef292312ec..3f6af8b1b49bf0f226e9336c222884b07bf69e55 100644
+--- a/ui/base/accelerators/system_media_controls_media_keys_listener.cc
++++ b/ui/base/accelerators/system_media_controls_media_keys_listener.cc
+@@ -65,6 +65,11 @@ bool SystemMediaControlsMediaKeysListener::StartWatchingMediaKey(
+     case VKEY_MEDIA_STOP:
+       service_->SetIsStopEnabled(true);
+       break;
++    case VKEY_VOLUME_DOWN:
++    case VKEY_VOLUME_UP:
++    case VKEY_VOLUME_MUTE:
++      // Do nothing.
++      break;
+     default:
+       NOTREACHED();
+   }

shell/browser/api/electron_api_global_shortcut.cc

@@ -29,7 +29,9 @@ bool RegisteringMediaKeyForUntrustedClient(const ui::Accelerator& accelerator) {
   if (base::mac::IsAtLeastOS10_14()) {
     constexpr ui::KeyboardCode mediaKeys[] = {
         ui::VKEY_MEDIA_PLAY_PAUSE, ui::VKEY_MEDIA_NEXT_TRACK,
-        ui::VKEY_MEDIA_PREV_TRACK, ui::VKEY_MEDIA_STOP};
+        ui::VKEY_MEDIA_PREV_TRACK, ui::VKEY_MEDIA_STOP,
+        ui::VKEY_VOLUME_UP,        ui::VKEY_VOLUME_DOWN,
+        ui::VKEY_VOLUME_MUTE};
 
     if (std::find(std::begin(mediaKeys), std::end(mediaKeys),
                   accelerator.key_code()) != std::end(mediaKeys)) {
@@ -60,7 +62,7 @@ GlobalShortcut::~GlobalShortcut() {
 void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
   if (accelerator_callback_map_.find(accelerator) ==
       accelerator_callback_map_.end()) {
-    // This should never occur, because if it does, GlobalGlobalShortcutListener
+    // This should never occur, because if it does, GlobalShortcutListener
     // notifies us with wrong accelerator.
     NOTREACHED();
     return;

spec-main/api-global-shortcut-spec.ts

@@ -38,4 +38,21 @@ ifdescribe(process.platform !== 'win32')('globalShortcut module', () => {
     expect(globalShortcut.isRegistered(accelerators[0])).to.be.false('first unregistered');
     expect(globalShortcut.isRegistered(accelerators[1])).to.be.false('second unregistered');
   });
+
+  it('does not crash when registering media keys as global shortcuts', () => {
+    const accelerators = [
+      'VolumeUp',
+      'VolumeDown',
+      'VolumeMute',
+      'MediaNextTrack',
+      'MediaPreviousTrack',
+      'MediaStop', 'MediaPlayPause'
+    ];
+
+    expect(() => {
+      globalShortcut.registerAll(accelerators, () => {});
+    }).to.not.throw();
+
+    globalShortcut.unregisterAll();
+  });
 });