Fix system hotkeys like cmd+` in app shims.

SystemHotkeyHelperMac, which just managed a SystemHotkeyMap, was never
initialized in app shim processes, and I didn't see a good place to
initialize it because it's private to /content. This change replaces
that class with a function that lazily constructs a SystemHotkeyMap.

The SystemHotkeyMap is now created synchronously on first use instead of
on a background thread ten seconds after startup. This gets rid of the
time after startup where system hotkeys aren't respected and simplifies
the code a bunch, but could jank the main thread if loading from disk is
slow. I'd favor janking the main thread during the first keyboard
shortcut instead of mis-routing it.

Bug: 919579
Change-Id: I78ab8dfaf7b9115ce32e1d2d3e1c27abb7f6a9b5
Reviewed-on: https://chromium-review.googlesource.com/c/1446847Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Auto-Submit: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629626}

content/browser/browser_main_loop.cc

@@ -178,7 +178,6 @@
 
 #if defined(OS_MACOSX)
 #include "base/memory/memory_pressure_monitor_mac.h"
-#include "content/browser/cocoa/system_hotkey_helper_mac.h"
 #include "content/browser/mach_broker_mac.h"
 #include "content/browser/renderer_host/browser_compositor_view_mac.h"
 #include "content/browser/theme_helper_mac.h"
@@ -1461,7 +1460,6 @@ int BrowserMainLoop::BrowserThreadsStarted() {
 
 #if defined(OS_MACOSX)
   ThemeHelperMac::GetInstance();
-  SystemHotkeyHelperMac::GetInstance()->DeferredLoadSystemHotkeys();
 #endif  // defined(OS_MACOSX)
 
   responsiveness_watcher_ = new responsiveness::Watcher;

content/browser/cocoa/system_hotkey_helper_mac.h

@@ -5,54 +5,12 @@
 #ifndef CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_HELPER_MAC_H_
 #define CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_HELPER_MAC_H_
 
-#include <memory>
-
-#include "base/macros.h"
-#include "base/memory/singleton.h"
-#include "base/memory/weak_ptr.h"
-
-#ifdef __OBJC__
-@class NSDictionary;
-#else
-class NSDictionary;
-#endif
-
 namespace content {
 
 class SystemHotkeyMap;
 
-// This singleton holds a global mapping of hotkeys reserved by OSX.
-class SystemHotkeyHelperMac {
- public:
-  // Return pointer to the singleton instance for the current process.
-  static SystemHotkeyHelperMac* GetInstance();
-
-  // Loads the system hot keys after a brief delay, to reduce file system access
-  // immediately after launch.
-  void DeferredLoadSystemHotkeys();
-
-  // Guaranteed to not be NULL.
-  SystemHotkeyMap* map() { return map_.get(); }
-
- private:
-  friend struct base::DefaultSingletonTraits<SystemHotkeyHelperMac>;
-
-  SystemHotkeyHelperMac();
-  ~SystemHotkeyHelperMac();
-
-  // Must be called from the FILE thread. Loads the file containing the system
-  // hotkeys into a NSDictionary* object, and passes the result to FileDidLoad
-  // on the UI thread.
-  void LoadSystemHotkeys();
-
-  // Must be called from the UI thread.  This takes ownership of |dictionary|.
-  // Parses the system hotkeys from the plist stored in |dictionary|.
-  void FileDidLoad(NSDictionary* dictionary);
-
-  std::unique_ptr<SystemHotkeyMap> map_;
-
-  DISALLOW_COPY_AND_ASSIGN(SystemHotkeyHelperMac);
-};
+// Guaranteed to not be NULL.
+SystemHotkeyMap* GetSystemHotkeyMap();
 
 }  // namespace content
 

content/browser/cocoa/system_hotkey_helper_mac.mm

@@ -4,76 +4,37 @@
 
 #include "content/browser/cocoa/system_hotkey_helper_mac.h"
 
-#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/mac/foundation_util.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/task/post_task.h"
-#include "base/task/task_traits.h"
-#include "base/threading/scoped_blocking_call.h"
+#include "base/no_destructor.h"
 #include "content/browser/cocoa/system_hotkey_map.h"
-#include "content/public/browser/browser_task_traits.h"
-#include "content/public/browser/browser_thread.h"
 
 namespace {
 
-NSString* kSystemHotkeyPlistExtension =
-    @"/Preferences/com.apple.symbolichotkeys.plist";
+constexpr auto* kSystemHotkeyPlistPath =
+    "Preferences/com.apple.symbolichotkeys.plist";
 
-// Amount of time to delay loading the hotkeys in seconds.
-const int kLoadHotkeysDelaySeconds = 10;
+content::SystemHotkeyMap LoadSystemHotkeyMap() {
+  auto* hotkey_plist_url = base::mac::FilePathToNSURL(
+      base::mac::GetUserLibraryPath().Append(kSystemHotkeyPlistPath));
+  NSDictionary* dictionary =
+      [NSDictionary dictionaryWithContentsOfURL:hotkey_plist_url];
+
+  content::SystemHotkeyMap map;
+  bool success = map.ParseDictionary(dictionary);
+  UMA_HISTOGRAM_BOOLEAN("OSX.SystemHotkeyMap.LoadSuccess", success);
+  return map;
+}
 
 }  // namespace
 
 namespace content {
 
 // static
-SystemHotkeyHelperMac* SystemHotkeyHelperMac::GetInstance() {
-  return base::Singleton<SystemHotkeyHelperMac>::get();
-}
-
-void SystemHotkeyHelperMac::DeferredLoadSystemHotkeys() {
-  base::PostDelayedTaskWithTraits(
-      FROM_HERE,
-      {base::TaskPriority::USER_VISIBLE,
-       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN, base::MayBlock()},
-      base::Bind(&SystemHotkeyHelperMac::LoadSystemHotkeys,
-                 base::Unretained(this)),
-      base::TimeDelta::FromSeconds(kLoadHotkeysDelaySeconds));
-}
-
-SystemHotkeyHelperMac::SystemHotkeyHelperMac() : map_(new SystemHotkeyMap) {
-}
-
-SystemHotkeyHelperMac::~SystemHotkeyHelperMac() {
-}
-
-void SystemHotkeyHelperMac::LoadSystemHotkeys() {
-  base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
-
-  std::string library_path(base::mac::GetUserLibraryPath().value());
-  NSString* expanded_file_path =
-      [NSString stringWithFormat:@"%s%@",
-                                 library_path.c_str(),
-                                 kSystemHotkeyPlistExtension];
-
-  // Loads the file into memory.
-  NSData* data = [NSData dataWithContentsOfFile:expanded_file_path];
-  // Intentionally create the object with +1 retain count, as FileDidLoad
-  // will destroy the object.
-  NSDictionary* dictionary = [SystemHotkeyMap::DictionaryFromData(data) retain];
-
-  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI},
-                           base::Bind(&SystemHotkeyHelperMac::FileDidLoad,
-                                      base::Unretained(this), dictionary));
-}
-
-void SystemHotkeyHelperMac::FileDidLoad(NSDictionary* dictionary) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  bool success = map()->ParseDictionary(dictionary);
-  UMA_HISTOGRAM_BOOLEAN("OSX.SystemHotkeyMap.LoadSuccess", success);
-  [dictionary release];
+SystemHotkeyMap* GetSystemHotkeyMap() {
+  static base::NoDestructor<SystemHotkeyMap> instance(LoadSystemHotkeyMap());
+  return instance.get();
 }
 
 }  // namespace content

content/browser/cocoa/system_hotkey_map.h

@@ -22,12 +22,9 @@ struct SystemHotkey;
 class CONTENT_EXPORT SystemHotkeyMap {
  public:
   SystemHotkeyMap();
+  SystemHotkeyMap(SystemHotkeyMap&&);
   ~SystemHotkeyMap();
 
-  // Converts the plist stored in |data| into an NSDictionary. Returns nil on
-  // error.
-  static NSDictionary* DictionaryFromData(NSData* data);
-
   // Parses the property list data commonly stored at
   // ~/Library/Preferences/com.apple.symbolichotkeys.plist
   // Returns false on encountering an irrecoverable error.

content/browser/cocoa/system_hotkey_map.mm

@@ -50,28 +50,9 @@ struct SystemHotkey {
 
 #pragma mark - SystemHotkeyMap
 
-SystemHotkeyMap::SystemHotkeyMap() {
-}
-SystemHotkeyMap::~SystemHotkeyMap() {
-}
-
-NSDictionary* SystemHotkeyMap::DictionaryFromData(NSData* data) {
-  if (!data)
-    return nil;
-
-  NSError* error = nil;
-  NSPropertyListFormat format;
-  NSDictionary* dictionary =
-      [NSPropertyListSerialization propertyListWithData:data
-                                                options:0
-                                                 format:&format
-                                                  error:&error];
-
-  if (![dictionary isKindOfClass:[NSDictionary class]])
-    return nil;
-
-  return dictionary;
-}
+SystemHotkeyMap::SystemHotkeyMap() = default;
+SystemHotkeyMap::SystemHotkeyMap(SystemHotkeyMap&&) = default;
+SystemHotkeyMap::~SystemHotkeyMap() = default;
 
 bool SystemHotkeyMap::ParseDictionary(NSDictionary* dictionary) {
   system_hotkeys_.clear();

content/browser/cocoa/system_hotkey_map_unittest.mm

@@ -8,6 +8,7 @@
 #import <Cocoa/Cocoa.h>
 
 #include "base/files/file_path.h"
+#import "base/mac/foundation_util.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/path_service.h"
 #import "content/browser/cocoa/system_hotkey_map.h"
@@ -19,17 +20,15 @@ class SystemHotkeyMapTest : public ::testing::Test {
  protected:
   SystemHotkeyMapTest() {}
 
-  NSData* DataFromTestFile(const char* file) {
+  NSDictionary* DictionaryFromTestFile(const char* file) {
     base::FilePath test_data_dir;
     bool result = base::PathService::Get(DIR_TEST_DATA, &test_data_dir);
     if (!result)
       return nil;
 
     base::FilePath test_path = test_data_dir.AppendASCII(file);
-    std::string test_path_string = test_path.AsUTF8Unsafe();
-    NSString* file_path =
-        [NSString stringWithUTF8String:test_path_string.c_str()];
-    return [NSData dataWithContentsOfFile:file_path];
+    return [NSDictionary
+        dictionaryWithContentsOfURL:base::mac::FilePathToNSURL(test_path)];
   }
 
   void AddEntryToDictionary(BOOL enabled,
@@ -84,10 +83,8 @@ class SystemHotkeyMapTest : public ::testing::Test {
 TEST_F(SystemHotkeyMapTest, Parse) {
   // This plist was pulled from a real machine. It is extensively populated,
   // and has no missing or incomplete entries.
-  NSData* data = DataFromTestFile("mac/mac_system_hotkeys.plist");
-  ASSERT_TRUE(data);
-
-  NSDictionary* dictionary = SystemHotkeyMap::DictionaryFromData(data);
+  NSDictionary* dictionary =
+      DictionaryFromTestFile("mac/mac_system_hotkeys.plist");
   ASSERT_TRUE(dictionary);
 
   SystemHotkeyMap map;
@@ -124,10 +121,8 @@ TEST_F(SystemHotkeyMapTest, ParseNil) {
 TEST_F(SystemHotkeyMapTest, ParseMouse) {
   // This plist was pulled from a real machine. It has missing entries,
   // incomplete entries, and mouse hotkeys.
-  NSData* data = DataFromTestFile("mac/mac_system_hotkeys_sparse.plist");
-  ASSERT_TRUE(data);
-
-  NSDictionary* dictionary = SystemHotkeyMap::DictionaryFromData(data);
+  NSDictionary* dictionary =
+      DictionaryFromTestFile("mac/mac_system_hotkeys_sparse.plist");
   ASSERT_TRUE(dictionary);
 
   SystemHotkeyMap map;

content/browser/renderer_host/render_widget_host_view_cocoa.mm

@@ -92,9 +92,7 @@ NSString* const kWebContentTouchBarId = @"web-content";
 
 // Whether a keyboard event has been reserved by OSX.
 BOOL EventIsReservedBySystem(NSEvent* event) {
-  content::SystemHotkeyHelperMac* helper =
-      content::SystemHotkeyHelperMac::GetInstance();
-  return helper->map()->IsEventReserved(event);
+  return content::GetSystemHotkeyMap()->IsEventReserved(event);
 }
 
 // TODO(suzhe): Upstream this function.