From 40a8e4fb58e66d8b736bd8c5e72d57473361a4e6 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 21 Oct 2021 21:39:38 +0200 Subject: [PATCH] fix: MediaMetadata not working properly (#31492) --- ...media_key_usage_with_globalshortcuts.patch | 95 +++++++++++++++---- .../api/electron_api_global_shortcut.cc | 42 +++++--- 2 files changed, 106 insertions(+), 31 deletions(-) diff --git a/patches/chromium/fix_media_key_usage_with_globalshortcuts.patch b/patches/chromium/fix_media_key_usage_with_globalshortcuts.patch index 1072fb48d301..6bf9bb899a4a 100644 --- a/patches/chromium/fix_media_key_usage_with_globalshortcuts.patch +++ b/patches/chromium/fix_media_key_usage_with_globalshortcuts.patch @@ -10,33 +10,94 @@ receive remote control events until it begins playing audio. This runs counter to the design of globalShortcuts, and so we need to instead use `ui::MediaKeysListener`. +diff --git a/chrome/browser/extensions/global_shortcut_listener.cc b/chrome/browser/extensions/global_shortcut_listener.cc +index bc009606d01469125052e68a9cdc82aaa697c764..ff18043cb07d748a49adea9874517fb29e3e7f9f 100644 +--- a/chrome/browser/extensions/global_shortcut_listener.cc ++++ b/chrome/browser/extensions/global_shortcut_listener.cc +@@ -7,6 +7,7 @@ + #include "base/check.h" + #include "base/notreached.h" + #include "content/public/browser/browser_thread.h" ++#include "content/public/browser/media_keys_listener_manager.h" + #include "ui/base/accelerators/accelerator.h" + + using content::BrowserThread; +@@ -66,6 +67,22 @@ void GlobalShortcutListener::UnregisterAccelerator( + StopListening(); + } + ++// static ++void GlobalShortcutListener::SetShouldUseInternalMediaKeyHandling(bool should_use) { ++ if (content::MediaKeysListenerManager:: ++ IsMediaKeysListenerManagerEnabled()) { ++ content::MediaKeysListenerManager* media_keys_listener_manager = ++ content::MediaKeysListenerManager::GetInstance(); ++ DCHECK(media_keys_listener_manager); ++ ++ if (should_use) { ++ media_keys_listener_manager->EnableInternalMediaKeyHandling(); ++ } else { ++ media_keys_listener_manager->DisableInternalMediaKeyHandling(); ++ } ++ } ++} ++ + void GlobalShortcutListener::UnregisterAccelerators(Observer* observer) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (IsShortcutHandlingSuspended()) +diff --git a/chrome/browser/extensions/global_shortcut_listener.h b/chrome/browser/extensions/global_shortcut_listener.h +index 554930bc33d87ee88a9bcc5f0cf17cef09c27ef0..8df4f91d3db453afb9f73bcaeb82c919ca8d1841 100644 +--- a/chrome/browser/extensions/global_shortcut_listener.h ++++ b/chrome/browser/extensions/global_shortcut_listener.h +@@ -34,6 +34,8 @@ class GlobalShortcutListener { + + static GlobalShortcutListener* GetInstance(); + ++ static void SetShouldUseInternalMediaKeyHandling(bool should_use); ++ + // Register an observer for when a certain |accelerator| is struck. Returns + // true if register successfully, or false if 1) the specificied |accelerator| + // has been registered by another caller or other native applications, or diff --git a/content/browser/media/media_keys_listener_manager_impl.cc b/content/browser/media/media_keys_listener_manager_impl.cc -index 5938f75742b793868638e693a9a8c8dc686dfc46..bf8782c23095a09dec62c68d7d902df24abcb13e 100644 +index 5938f75742b793868638e693a9a8c8dc686dfc46..1263d679a5174beb960265989c370dd4a58ae7b4 100644 --- a/content/browser/media/media_keys_listener_manager_impl.cc +++ b/content/browser/media/media_keys_listener_manager_impl.cc -@@ -231,7 +231,7 @@ void MediaKeysListenerManagerImpl::StartListeningForMediaKeysIfNecessary() { +@@ -231,18 +231,24 @@ void MediaKeysListenerManagerImpl::StartListeningForMediaKeysIfNecessary() { media::AudioManager::GetGlobalAppName()); #endif - if (system_media_controls_) { -+ if (/* DISABLES CODE */ (0)) { - system_media_controls_->AddObserver(this); - system_media_controls_notifier_ = - std::make_unique( -@@ -239,8 +239,13 @@ void MediaKeysListenerManagerImpl::StartListeningForMediaKeysIfNecessary() { - } else { - // If we can't access system media controls, then directly listen for media - // key keypresses instead. -+#if defined(OS_MAC) -+ media_keys_listener_ = ui::MediaKeysListener::Create( -+ this, ui::MediaKeysListener::Scope::kGlobalRequiresAccessibility); -+#else +- system_media_controls_->AddObserver(this); +- system_media_controls_notifier_ = +- std::make_unique( +- system_media_controls_.get()); +- } else { +- // If we can't access system media controls, then directly listen for media +- // key keypresses instead. ++ // This is required for proper functioning of MediaMetadata. ++ system_media_controls_->AddObserver(this); ++ system_media_controls_notifier_ = ++ std::make_unique( ++ system_media_controls_.get()); ++ ++ // Directly listen for media key keypresses when using GlobalShortcuts. ++#if defined(OS_MACOS) ++ auto scope = media_key_handling_enabled_ ? ++ ui::MediaKeysListener::Scope::kGlobal : ++ ui::MediaKeysListener::Scope::kGlobalRequiresAccessibility; media_keys_listener_ = ui::MediaKeysListener::Create( - this, ui::MediaKeysListener::Scope::kGlobal); +- this, ui::MediaKeysListener::Scope::kGlobal); +- DCHECK(media_keys_listener_); +- } ++ this, scope); ++#else ++ media_keys_listener_ = ui::MediaKeysListener::Create( ++ this, ui::MediaKeysListener::Scope::kGlobal); +#endif - DCHECK(media_keys_listener_); - } ++ DCHECK(media_keys_listener_); + EnsureAuxiliaryServices(); + } diff --git a/ui/base/accelerators/media_keys_listener.h b/ui/base/accelerators/media_keys_listener.h index c2b03328c0e508995bdc135031500783f500ceba..1b6b14dc2999c99445cef6ffc04d49a7c1728a54 100644 --- a/ui/base/accelerators/media_keys_listener.h diff --git a/shell/browser/api/electron_api_global_shortcut.cc b/shell/browser/api/electron_api_global_shortcut.cc index 26fff141067c..868936ca3c00 100644 --- a/shell/browser/api/electron_api_global_shortcut.cc +++ b/shell/browser/api/electron_api_global_shortcut.cc @@ -9,6 +9,7 @@ #include "base/containers/contains.h" #include "base/stl_util.h" #include "base/strings/utf_string_conversions.h" +#include "chrome/common/extensions/command.h" #include "gin/dictionary.h" #include "gin/object_template_builder.h" #include "shell/browser/api/electron_api_system_preferences.h" @@ -21,6 +22,7 @@ #include "base/mac/mac_util.h" #endif +using extensions::Command; using extensions::GlobalShortcutListener; namespace { @@ -28,22 +30,23 @@ namespace { #if defined(OS_MAC) 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_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)) { - bool trusted = - electron::api::SystemPreferences::IsTrustedAccessibilityClient(false); - if (!trusted) + if (Command::IsMediaKey(accelerator)) { + if (!electron::api::SystemPreferences::IsTrustedAccessibilityClient( + false)) return true; } } return false; } + +bool MapHasMediaKeys( + const std::map& accelerator_map) { + auto media_key = std::find_if( + accelerator_map.begin(), accelerator_map.end(), + [](const auto& ac) { return Command::IsMediaKey(ac.first); }); + + return media_key != accelerator_map.end(); +} #endif } // namespace @@ -83,7 +86,7 @@ bool GlobalShortcut::RegisterAll( for (auto& accelerator : accelerators) { if (!Register(accelerator, callback)) { - // unregister all shortcuts if any failed + // Unregister all shortcuts if any failed. UnregisterSome(registered); return false; } @@ -101,8 +104,12 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator, return false; } #if defined(OS_MAC) - if (RegisteringMediaKeyForUntrustedClient(accelerator)) - return false; + if (Command::IsMediaKey(accelerator)) { + if (RegisteringMediaKeyForUntrustedClient(accelerator)) + return false; + + GlobalShortcutListener::SetShouldUseInternalMediaKeyHandling(false); + } #endif if (!GlobalShortcutListener::GetInstance()->RegisterAccelerator(accelerator, @@ -123,6 +130,13 @@ void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) { if (accelerator_callback_map_.erase(accelerator) == 0) return; +#if defined(OS_MAC) + if (Command::IsMediaKey(accelerator) && + !MapHasMediaKeys(accelerator_callback_map_)) { + GlobalShortcutListener::SetShouldUseInternalMediaKeyHandling(true); + } +#endif + GlobalShortcutListener::GetInstance()->UnregisterAccelerator(accelerator, this); }