Mac global keybindings

chrome/browser/extensions/global_shortcut_listener_mac.h

Index: chrome/browser/extensions/global_shortcut_listener_mac.h
===================================================================
--- chrome/browser/extensions/global_shortcut_listener_mac.h	(revision 236766)
+++ chrome/browser/extensions/global_shortcut_listener_mac.h	(working copy)
@@ -5,15 +5,25 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_GLOBAL_SHORTCUT_LISTENER_MAC_H_
 #define CHROME_BROWSER_EXTENSIONS_GLOBAL_SHORTCUT_LISTENER_MAC_H_
 
-#include "base/lazy_instance.h"
 #include "chrome/browser/extensions/global_shortcut_listener.h"
 
+#include <Carbon/Carbon.h>
+#include <CoreFoundation/CoreFoundation.h>
+
+#include <map>
+
+#include "base/lazy_instance.h"
+#include "base/mac/scoped_nsobject.h"
+
 namespace extensions {
 
 // Mac-specific implementation of the GlobalShortcutListener class that
 // listens for global shortcuts. Handles basic keyboard intercepting and
 // forwards its output to the base class for processing.
-// TODO(finnur/smus): Implement this class.
+//
+// Uses an event tap for intercepting media keys (PlayPause, NextTrack,
+// PreviousTrack). Also uses the Carbon RegisterEventHotKey API for binding to
+// non-media key global keyboard shortcuts (eg. Command-Shift-1).
 class GlobalShortcutListenerMac : public GlobalShortcutListener {
  public:
   virtual ~GlobalShortcutListenerMac();
@@ -21,6 +31,10 @@
   virtual void StartListening() OVERRIDE;
   virtual void StopListening() OVERRIDE;
 
+  // Keyboard event callbacks.
+  bool OnKeyEvent(EventHotKeyID hot_key_id);
+  bool OnMediaKeyEvent(int key_code);
+
  private:
   friend struct base::DefaultLazyInstanceTraits<GlobalShortcutListenerMac>;
 
@@ -35,9 +49,51 @@
       const ui::Accelerator& accelerator,
       GlobalShortcutListener::Observer* observer) OVERRIDE;
 
+  // Mac-specific functions for registering a keyboard shortcut.
+  void RegisterHotKey(const ui::Accelerator& accelerator);
+  void UnregisterHotKey(const ui::Accelerator& accelerator);
+
+  // Enable and disable the media key event tap.
+  void StartWatchingMediaKeys();
+  void StopWatchingMediaKeys();
+
+  // Convert from mac media key code to the equivalent keyboard code.
+  ui::KeyboardCode MediaKeyCodeToKeyboardCode(int key_code);

[Mark Mentovai, 2013/11/26 15:32:46]

This can be static. And private + static implies that it doesn’t really need to
be part of the class at all, it can actually be an anonymous-namespaced function
in the implementation file.

[smus, 2013/12/09 08:37:16]

Done.

+
+  // Whether or not any media keys are currently registered.
+  bool AreMediaKeysRegistered();

[Mark Mentovai, 2013/11/26 15:32:46]

The comment is accurate but the name of the function makes it sound like it
checks for ALL media keys, not ANY media key.

[smus, 2013/12/09 08:37:16]

Renamed to IsAnyMediaKeyRegistered.

+
+  // Re-enable the event tap if it was disabled by a timeout.
+  void EnableTap();
+
+  // The callback for when an event tap happens.
+  static CGEventRef EventTapCallback(
+      CGEventTapProxy proxy, CGEventType type, CGEventRef event, void* refcon);
+
   // Whether this object is listening for global shortcuts.
   bool is_listening_;
 
+  // A counter representing the current hotkey identifier.

[Mark Mentovai, 2013/11/26 15:32:46]

What does “current” mean?

I looked at the implementation and see how you’re using it, but this comment
isn’t helpful at all.

I also have concerns about using a dumb counter.

[smus, 2013/12/09 08:37:16]

Updated comment. What's the issue with a dumb counter?

+  int hot_key_id_ = 0;

[Mark Mentovai, 2013/11/26 15:32:46]

Don’t initialize here, initialize in the constructor.

[smus, 2013/12/09 08:37:16]

Done.

+
+  // A map of all hotkeys (media keys and shortcuts) mapping to their
+  // corresponding hotkey IDs. For quickly finding if an accelerator is
+  // registered.
+  typedef std::map<ui::Accelerator, int> HotKeyIdMap;
+  HotKeyIdMap hot_key_ids_;
+
+  // The inverse map for quickly looking up accelerators by hotkey id.
+  typedef std::map<int, ui::Accelerator> IdHotKeyMap;
+  IdHotKeyMap id_hot_keys_;
+
+  // Keyboard shortcut IDs to hotkeys map for unregistration.
+  typedef std::map<int, EventHotKeyRef> IdHotKeyRefMap;
+  IdHotKeyRefMap id_hot_key_refs_;
+
+  // Event tap for intercepting mac media keys.
+  CFMachPortRef event_tap_;
+  CFRunLoopSourceRef event_tap_source_;
+
   DISALLOW_COPY_AND_ASSIGN(GlobalShortcutListenerMac);
 };