mac: Load the system hotkeys after launch.

content/browser/cocoa/system_hotkey_helper_mac.mm

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/cocoa/system_hotkey_helper_mac.h"
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram.h"
+#include "content/browser/cocoa/system_hotkey_map.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace {
+
+NSString* kSystemHotkeyPlistPath =
+    @"~/Library/Preferences/com.apple.symbolichotkeys.plist";
+
+// Amount of time to delay loading the hotkeys.

[Robert Sesek, 2014/07/08 13:40:01]

Units? In the name of the symbol, too, please.

[erikchen, 2014/07/10 02:07:31]

Done.

+const int kLoadHotkeysDelay = 10;
+
+}  // namespace
+
+namespace content {
+
+// static
+SystemHotkeyHelperMac* SystemHotkeyHelperMac::GetInstance() {
+  return Singleton<SystemHotkeyHelperMac,
+                   LeakySingletonTraits<SystemHotkeyHelperMac> >::get();

[Robert Sesek, 2014/07/08 13:40:01]

Why does this need to be leaky?

[erikchen, 2014/07/10 02:07:31]

I was copy-pasting some code, and didn't read it too carefully. You're right, it
doesn't need to be leaky.

+}
+
+void SystemHotkeyHelperMac::DeferredLoadSystemHotkeys() {
+  base::MessageLoop::current()->PostDelayedTask(

[Robert Sesek, 2014/07/08 13:40:00]

You can post this directly to the FILE thread.

[erikchen, 2014/07/10 02:07:32]

Done.

+      FROM_HERE,
+      base::Bind(&SystemHotkeyHelperMac::LoadSystemHotkeys,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(kLoadHotkeysDelay));
+}
+
+SystemHotkeyHelperMac::SystemHotkeyHelperMac()
+    : map_(new SystemHotkeyMap), weak_ptr_factory_(this) {
+}
+
+SystemHotkeyHelperMac::~SystemHotkeyHelperMac() {
+}
+
+void SystemHotkeyHelperMac::LoadSystemHotkeys() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&SystemHotkeyHelperMac::LoadFile, base::Unretained(this)));

[erikchen, 2014/07/08 01:51:39]

I couldn't use a weak ptr here because the weak ptr/factory is bound to a
different thread. Making a second weak_ptr_factory doesn't make sense.
Alternatives?

[Robert Sesek, 2014/07/08 13:40:01]

Right, weak pointer only works on a given thread. Why do you think you even need
a weak pointer? Since this is a singleton, there's no danger in using
base::Unretained.

[erikchen, 2014/07/10 02:07:31]

Removed all the weak pointers.

+}
+
+void SystemHotkeyHelperMac::LoadFile() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  NSString* expandedFilePath =
+      [kSystemHotkeyPlistPath stringByExpandingTildeInPath];

[Robert Sesek, 2014/07/08 13:40:00]

base::mac::GetUserLibraryPath()

[erikchen, 2014/07/10 02:07:31]

Done.

+  NSData* data = [NSData dataWithContentsOfFile:expandedFilePath];

[Robert Sesek, 2014/07/08 13:40:01]

Alternatively, use base::ReadFileToString and pass around a std::string(). Then
FileDidLoad doesn't need to do any manual memory management.

[Robert Sesek, 2014/07/08 13:40:01]

Use alloc/init so you don't need to manually retain it.

[erikchen, 2014/07/10 02:07:32]

Are you suggesting that we pass around a C++ object instead of a pointer? That
sounds like there will be much more overhead. Let me know if I'm misinterpreting
your suggestion.

+  [data retain];
+
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          base::Bind(&SystemHotkeyHelperMac::FileDidLoad,
+                                     weak_ptr_factory_.GetWeakPtr(),
+                                     data));
+}
+
+void SystemHotkeyHelperMac::FileDidLoad(NSData* data) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  bool success = map()->ParseData(data);

[Robert Sesek, 2014/07/08 13:40:01]

How long does parsing take? Consider doing it on the FILE thread as well?

+  UMA_HISTOGRAM_BOOLEAN("Mac.SystemHotkeyMap.Load", success);

[Robert Sesek, 2014/07/08 13:40:01]

Use OSX. instead of Mac.

[erikchen, 2014/07/10 02:07:31]

Done.

+  [data release];
+}
+
+}  // namespace content