fix: don't register some shortcuts without accessibility (#16125)

Fixed crash on macOS when using globalShortcut for media keys when accessibility access is not granted.

atom/browser/api/atom_api_global_shortcut.cc

@@ -7,15 +7,44 @@
 #include <string>
 #include <vector>
 
+#include "atom/browser/api/atom_api_system_preferences.h"
 #include "atom/common/native_mate_converters/accelerator_converter.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "base/stl_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "native_mate/dictionary.h"
 
 #include "atom/common/node_includes.h"
 
+#if defined(OS_MACOSX)
+#include "base/mac/mac_util.h"
+#endif
+
 using extensions::GlobalShortcutListener;
 
+namespace {
+
+#if defined(OS_MACOSX)
+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};
+
+    if (std::find(std::begin(mediaKeys), std::end(mediaKeys),
+                  accelerator.key_code()) != std::end(mediaKeys)) {
+      bool trusted =
+          atom::api::SystemPreferences::IsTrustedAccessibilityClient(false);
+      if (!trusted)
+        return true;
+    }
+  }
+  return false;
+}
+#endif
+
+}  // namespace
+
 namespace atom {
 
 namespace api {
@@ -32,7 +61,7 @@ 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
-    // notifes us with wrong accelerator.
+    // notifies us with wrong accelerator.
     NOTREACHED();
     return;
   }
@@ -43,14 +72,19 @@ bool GlobalShortcut::RegisterAll(
     const std::vector<ui::Accelerator>& accelerators,
     const base::Closure& callback) {
   std::vector<ui::Accelerator> registered;
+
   for (auto& accelerator : accelerators) {
+#if defined(OS_MACOSX)
+    if (RegisteringMediaKeyForUntrustedClient(accelerator))
+      return false;
+
     GlobalShortcutListener* listener = GlobalShortcutListener::GetInstance();
     if (!listener->RegisterAccelerator(accelerator, this)) {
       // unregister all shortcuts if any failed
       UnregisterSome(registered);
       return false;
     }
-
+#endif
     registered.push_back(accelerator);
     accelerator_callback_map_[accelerator] = callback;
   }
@@ -59,6 +93,11 @@ bool GlobalShortcut::RegisterAll(
 
 bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
                               const base::Closure& callback) {
+#if defined(OS_MACOSX)
+  if (RegisteringMediaKeyForUntrustedClient(accelerator))
+    return false;
+#endif
+
   if (!GlobalShortcutListener::GetInstance()->RegisterAccelerator(accelerator,
                                                                   this)) {
     return false;

atom/browser/api/atom_api_system_preferences.h

@@ -95,7 +95,7 @@ class SystemPreferences : public mate::EventEmitter<SystemPreferences>
 
   std::string GetSystemColor(const std::string& color, mate::Arguments* args);
 
-  bool IsTrustedAccessibilityClient(bool prompt);
+  static bool IsTrustedAccessibilityClient(bool prompt);
 
   // TODO(codebytere): Write tests for these methods once we
   // are running tests on a Mojave machine

atom/browser/api/atom_api_system_preferences_mac.mm

@@ -437,6 +437,7 @@ std::string SystemPreferences::GetSystemColor(const std::string& color,
   }
 }
 
+// static
 bool SystemPreferences::IsTrustedAccessibilityClient(bool prompt) {
   NSDictionary* options = @{(id)kAXTrustedCheckOptionPrompt : @(prompt)};
   return AXIsProcessTrustedWithOptions((CFDictionaryRef)options);

docs/api/global-shortcut.md

@@ -54,6 +54,14 @@ When the accelerator is already taken by other applications, this call will
 silently fail. This behavior is intended by operating systems, since they don't
 want applications to fight for global shortcuts.
 
+The following accelerators will not be registered successfully on macOS 10.14 Mojave unless
+the app has been authorized as a [trusted accessibility client](https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXTestingApps.html):
+
+* "Media Play/Pause"
+* "Media Next Track"
+* "Media Previous Track"
+* "Media Stop"
+
 ### `globalShortcut.registerAll(accelerators, callback)`
 
 * `accelerators` String[] - an array of [Accelerator](accelerator.md)s.
@@ -65,6 +73,14 @@ When a given accelerator is already taken by other applications, this call will
 silently fail. This behavior is intended by operating systems, since they don't
 want applications to fight for global shortcuts.
 
+The following accelerators will not be registered successfully on macOS 10.14 Mojave unless
+the app has been authorized as a [trusted accessibility client](https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXTestingApps.html):
+
+* "Media Play/Pause"
+* "Media Next Track"
+* "Media Previous Track"
+* "Media Stop"
+
 ### `globalShortcut.isRegistered(accelerator)`
 
 * `accelerator` [Accelerator](accelerator.md)