Mac global keybindings

chrome/browser/extensions/global_shortcut_listener_mac.mm

+// Copyright (c) 2013 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 "chrome/browser/extensions/global_shortcut_listener_mac.h"
+
+#include <ApplicationServices/ApplicationServices.h>
+#import <Cocoa/Cocoa.h>
+#include <IOKit/hidsystem/ev_keymap.h>
+
+#include "chrome/browser/extensions/api/commands/command_service.h"
+#include "content/public/browser/browser_thread.h"
+#include "ui/base/accelerators/accelerator.h"
+#include "ui/events/event.h"
+#import "ui/events/keycodes/keyboard_code_conversion_mac.h"
+
+using content::BrowserThread;
+using extensions::GlobalShortcutListenerMac;
+
+namespace {
+
+// The media keys subtype.
+const int kSystemDefinedEventMediaKeys = 8;

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

Can you describe how you found this number? Documentation or something else? You
should do this any time you need to hard-code constant that’s beyond our
control.

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

Unfortunately I can't find official docs. I don't believe it's a public API, but
here is the earliest mention I could find:
http://lists.apple.com/archives/cocoa-dev/2007/Aug/msg00499.html. Updated the
comment.

+
+OSStatus HotKeyHandler(
+    EventHandlerCallRef next_handler, EventRef event, void* user_data) {
+  DVLOG(0) << "HotKeyHandler fired with event: " << event;
+  // Extract the hotkey from the event.
+  EventHotKeyID hot_key_id;
+  int result = GetEventParameter(event, kEventParamDirectObject,
+      typeEventHotKeyID, NULL, sizeof(hot_key_id), NULL, &hot_key_id);
+  if (result != noErr)
+    return result;
+
+  GlobalShortcutListenerMac* shortcut_listener =
+      static_cast<GlobalShortcutListenerMac*>(user_data);
+  shortcut_listener->OnKeyEvent(hot_key_id);
+  return noErr;
+}
+
+static base::LazyInstance<extensions::GlobalShortcutListenerMac> g_instance =
+    LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+namespace extensions {
+
+// static
+GlobalShortcutListener* GlobalShortcutListener::GetInstance() {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  return g_instance.Pointer();
+}
+
+GlobalShortcutListenerMac::GlobalShortcutListenerMac()
+    : is_listening_(false) {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+}
+
+GlobalShortcutListenerMac::~GlobalShortcutListenerMac() {
+  if (is_listening_)
+    StopListening();
+
+  // If media keys are still registered, make sure we stop the tap.
+  if (AreMediaKeysRegistered())
+    StopWatchingMediaKeys();
+}
+
+void GlobalShortcutListenerMac::StartListening() {
+  DCHECK(!is_listening_);  // Don't start twice.
+  DCHECK(!hot_key_ids_.empty());  // Don't start if no hot key registered.
+  DCHECK(!id_hot_keys_.empty());
+  is_listening_ = true;
+}
+
+void GlobalShortcutListenerMac::StopListening() {
+  DCHECK(is_listening_);  // No point if we are not already listening.
+  DCHECK(hot_key_ids_.empty());  // Make sure the set is clean.
+  DCHECK(id_hot_keys_.empty());
+  is_listening_ = false;
+}
+
+bool GlobalShortcutListenerMac::OnKeyEvent(EventHotKeyID hot_key_id) {
+  // Look up the accelerator based on this hot key ID.
+  DVLOG(0) << "OnKeyEvent! hot_key_id: " << hot_key_id.id;
+  const ui::Accelerator& accelerator = id_hot_keys_[hot_key_id.id];
+  DVLOG(0) << "Key code: " << accelerator.key_code()
+          << " modifiers: " << accelerator.modifiers();
+  NotifyKeyPressed(accelerator);
+  return true;
+}
+
+bool GlobalShortcutListenerMac::OnMediaKeyEvent(int media_key_code) {
+  ui::KeyboardCode key_code = MediaKeyCodeToKeyboardCode(media_key_code);
+  DVLOG(0) << "OnMediaKeyEvent! keyCode: " << key_code;
+  // Create an accelerator corresponding to the keyCode.
+  ui::Accelerator accelerator(key_code, 0);
+  // Look for a match with a bound hot_key.
+  if (hot_key_ids_.find(accelerator) != hot_key_ids_.end()) {
+    // If matched, callback to the event handling system.
+    NotifyKeyPressed(accelerator);
+    return true;
+  }
+  return false;
+}
+
+void GlobalShortcutListenerMac::RegisterAccelerator(
+    const ui::Accelerator& accelerator,
+    GlobalShortcutListener::Observer* observer) {
+  DVLOG(0) << "Registered keyCode: " << accelerator.key_code()
+      << ", modifiers: " << accelerator.modifiers();
+
+  // If this is the first media key registered, start the event tap.
+  if (CommandService::IsMediaKey(accelerator) && !AreMediaKeysRegistered()) {
+    DVLOG(0) << "Registered the first media key, so starting event tap.";
+    StartWatchingMediaKeys();
+  }
+
+  // Register hot_key if they are non-media keyboard shortcuts.
+  if (!CommandService::IsMediaKey(accelerator))
+    RegisterHotKey(accelerator);
+
+  // Store the hotkey-ID mappings we will need for lookup later.
+  id_hot_keys_[hot_key_id_] = accelerator;
+  hot_key_ids_[accelerator] = hot_key_id_;
+  ++hot_key_id_;
+  GlobalShortcutListener::RegisterAccelerator(accelerator, observer);
+}
+
+void GlobalShortcutListenerMac::UnregisterAccelerator(
+    const ui::Accelerator& accelerator,
+    GlobalShortcutListener::Observer* observer) {
+  // Unregister the hot_key if it's a keyboard shortcut.
+  if (!CommandService::IsMediaKey(accelerator))
+    UnregisterHotKey(accelerator);
+
+  // Remove hot_key from the mappings.
+  int id = hot_key_ids_[accelerator];
+  id_hot_keys_.erase(id);
+  hot_key_ids_.erase(accelerator);
+  GlobalShortcutListener::UnregisterAccelerator(accelerator, observer);
+
+  // If we unregistered a media key, and now if no media keys are registered,
+  // stop the media key tap.
+  if (CommandService::IsMediaKey(accelerator) && !AreMediaKeysRegistered()) {
+    DVLOG(0) << "Unregistered the last media key, stopping event tap.";
+    StopWatchingMediaKeys();
+  }
+}
+
+void GlobalShortcutListenerMac::RegisterHotKey(

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

RegisterAccelerator, RegisterHotKey, StartWatchingMediaKeys, …

It’s confusing to have all of these similar names while you’re reading code
elsewhere in this file. You have to keep flipping back and forth to figure out
which RegisterX function something is referring to. Can you define your
terminology somewhere? Or choose more descriptive names that aren’t so much like
synonyms? “Media keys” is an example of a descriptive name, but if the others
aren’t descriptive, it’s not helpful: is a media key “hot” or not? Is it an
“accelerator” or not?

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

Accelerator is the Chrome terminology, so stuck with it. It encompasses both
media keys and hot keys. HotKey is the Carbon terminology for keyboard shortcuts
involving a modifier, so stuck with that too. I updated the class doc.

+    const ui::Accelerator& accelerator) {
+  DVLOG(0) << "Registering hot_key. Keycode: " << accelerator.key_code();
+  EventHotKeyRef hot_key_ref;
+  EventHotKeyID event_hot_key_id;
+  EventHandlerUPP hot_key_function = NewEventHandlerUPP(HotKeyHandler);
+
+  EventTypeSpec event_type;
+  event_type.eventClass = kEventClassKeyboard;
+  event_type.eventKind = kEventHotKeyPressed;
+  InstallApplicationEventHandler(hot_key_function, 1, &event_type, this, NULL);

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

You install this each time you pass through RegisterHotKey. That seems
unnecessary, the application event handler itself is not specific to any
specific hot key.

You also never remove the application event handler (RemoveEventHandler).

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

From http://oreilly.com/catalog/learncarbon/chapter/ch06.html:

"In particular, the handler will be disposed of automatically when you dispose
of the target object it's associated with, so there's no need to call
RemoveEventHandler explicitly unless for some reason you want to deinstall the
handler while the underlying target object still exists."

[Mark Mentovai, 2013/12/09 19:31:54]

smus wrote:
The target object is the application event target (GetApplicationEventTarget()),
not “this”. The application event target is never destroyed.

Reiterating:

You install this each time you pass through RegisterHotKey. That seems
unnecessary, the application event handler itself is not specific to any
specific hot key.

You also never remove the application event handler (RemoveEventHandler).

[smus, 2013/12/09 23:37:06]

Sorry, completely misunderstood the first time! Now installing handler in
StartListening, removing in StopListening.

+
+  // Signature uniquely identifies the application that owns this hot_key.
+  event_hot_key_id.signature = 'rimZ';

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

You should get the bundle signature instead of hard-coding this here.

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

Can you provide any pointer for doing this? I don't see any mention of the
creator bundle signature in the codebase, so the way to do it would be via
FSGetCatalogInfo?

+  event_hot_key_id.id = hot_key_id_;

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

Perhaps the uses of hot_key_id_ here and on line 185 would be less scary if it
were a parameter to this function instead of a direct access to the member
variable. That way, you wouldn’t need the comment on line 184 that makes the
whole operation seem very fragile and sensitive to the call sequence.

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

Done.

+
+  // Translate ui::Accelerator modifiers to cmdKey, altKey, etc.
+  int modifiers = 0;
+  modifiers += (accelerator.IsShiftDown() ? shiftKey : 0);

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

These are bits, right? Use |= instead of +=.

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

Done.

+  modifiers += (accelerator.IsCtrlDown() ? controlKey : 0);
+  modifiers += (accelerator.IsAltDown() ? optionKey : 0);
+  modifiers += (accelerator.IsCmdDown() ? cmdKey : 0);
+
+  unichar character;
+  unichar character_nomods;
+  int key_code = ui::MacKeyCodeForWindowsKeyCode(accelerator.key_code(), 0,
+      &character, &character_nomods);
+  DVLOG(0) << "RegisterHotKey. Code: " << key_code
+           << " modifier: " << modifiers;
+
+  // Register the event hot key.
+  RegisterEventHotKey(key_code, modifiers, event_hot_key_id,
+      GetApplicationEventTarget(), 0, &hot_key_ref);
+
+  // Note: hot_key_id_ will be incremented in the caller (RegisterAccelerator).
+  id_hot_key_refs_[hot_key_id_] = hot_key_ref;
+}
+
+void GlobalShortcutListenerMac::UnregisterHotKey(
+    const ui::Accelerator& accelerator) {
+  // Get the ref corresponding to this accelerator.
+  int id = hot_key_ids_[accelerator];
+  EventHotKeyRef ref = id_hot_key_refs_[id];
+  // Unregister the event hot key.
+  UnregisterEventHotKey(ref);
+
+  // Remove the event from the mapping.
+  id_hot_key_refs_.erase(id);
+}
+
+void GlobalShortcutListenerMac::StartWatchingMediaKeys() {
+  DVLOG(0) << "StartWatchingMediaKeys";
+  // Make sure there's no existing event tap.
+  DCHECK(event_tap_ == NULL);

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

It’s not necessarily going to be NULL because you never initialized it in the
constructor. You should also initialize event_tap_source_.

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

Done.

+
+  // Add an event tap to intercept the system defined media key events.
+  event_tap_ = CGEventTapCreate(kCGSessionEventTap,
+      kCGHeadInsertEventTap,
+      kCGEventTapOptionDefault,
+      CGEventMaskBit(NX_SYSDEFINED),
+      EventTapCallback,
+      this);
+  if (event_tap_ == NULL) {
+    LOG(ERROR) << "Error: failed to create event tap.";
+    return;
+  }
+
+  event_tap_source_ = CFMachPortCreateRunLoopSource(kCFAllocatorSystemDefault,
+      event_tap_, 0);
+  if (event_tap_source_ == NULL) {
+    LOG(ERROR) << "Error: failed to create new run loop source.";
+    return;
+  }
+
+  CFRunLoopAddSource(CFRunLoopGetCurrent(), event_tap_source_,

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

You never remove this source (in StopWatchingMediaKeys?)

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

Oops, good point, thanks!

+      kCFRunLoopCommonModes);
+}
+
+void GlobalShortcutListenerMac::StopWatchingMediaKeys() {
+  DVLOG(0) << "StopWatchingMediaKeys";
+  // Invalidate the event tap.
+  DCHECK(event_tap_ != NULL);
+  CFMachPortInvalidate(event_tap_);
+  CFRelease(event_tap_);
+  event_tap_ = NULL;
+
+  // Release the event tap source.
+  DCHECK(event_tap_source_ != NULL);
+  CFRelease(event_tap_source_);
+  event_tap_source_ = NULL;
+}
+
+ui::KeyboardCode GlobalShortcutListenerMac::MediaKeyCodeToKeyboardCode(
+    int key_code) {
+  switch (key_code) {
+    case NX_KEYTYPE_PLAY:
+      return ui::VKEY_MEDIA_PLAY_PAUSE;
+    case NX_KEYTYPE_PREVIOUS:
+    case NX_KEYTYPE_REWIND:
+      return ui::VKEY_MEDIA_PREV_TRACK;
+    case NX_KEYTYPE_NEXT:
+    case NX_KEYTYPE_FAST:
+      return ui::VKEY_MEDIA_NEXT_TRACK;
+  }
+  return ui::VKEY_UNKNOWN;
+}
+
+bool GlobalShortcutListenerMac::AreMediaKeysRegistered() {
+  // Iterate through registered accelerators, looking for media keys.
+  HotKeyIdMap::iterator iter;
+  for (iter = hot_key_ids_.begin(); iter != hot_key_ids_.end(); ++iter) {
+    if (CommandService::IsMediaKey(iter->first))
+      return true;
+  }
+  return false;
+}
+
+void GlobalShortcutListenerMac::EnableTap() {
+  CGEventTapEnable(event_tap_, TRUE);
+}
+
+// Processed events should propagate if they aren't handled by any listeners.
+// For events that don't matter, this handler should return as quickly as
+// possible.
+// Returning event causes the event to propagate to other applications.
+// Returning NULL prevents the event from propagating.
+// static
+CGEventRef GlobalShortcutListenerMac::EventTapCallback(
+    CGEventTapProxy proxy, CGEventType type, CGEventRef event, void* refcon) {
+  GlobalShortcutListenerMac* shortcut_listener =
+      static_cast<GlobalShortcutListenerMac*>(refcon);
+
+  // Handle the timeout case by re-enabling the tap.
+  if (type == kCGEventTapDisabledByTimeout) {
+    shortcut_listener->EnableTap();

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

EnableTap is a one-liner that is only used here. It’s OK to write it in-line
here and get rid of the standalone above.

  CGEventTapEnable(shortcut_listener->event_tap_, TRUE);

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

Done.

+    return event;
+  }
+
+  // TODO(smus): do some error handling since eventWithCGEvent can fail.

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

What kind of error handling? Why not do it?

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

Done.

+  NSEvent* ns_event = [NSEvent eventWithCGEvent:event];

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

Can you point (in a comment) to some documentation that describes what’s going
on in the system-defined event and its data1 field?

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

Added a reference to http://weblog.rogueamoeba.com/2007/09/29/ but these are
basically undocumented APIs.

+
+  // Ignore events that are not system defined media keys.
+  if (type != NX_SYSDEFINED ||
+      [ns_event subtype] != kSystemDefinedEventMediaKeys) {

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

For safety, [ns_event type] should be NSSystemDefined. I’m not sure if that’s
the same as checking whether type == NX_SYSDEFINED, but since you’re operating
on ns_event here, it seems prudent to check the type field on ns_event in
preference to the CGEventType parameter.

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

Done.

+    return event;
+  }
+
+  NSInteger data1 = [ns_event data1];
+  // Ignore media keys that aren't previous, next and play/pause.
+  int key_code = ((data1 & 0xFFFF0000) >> 16);
+  if (key_code != NX_KEYTYPE_PLAY && key_code != NX_KEYTYPE_NEXT &&
+      key_code != NX_KEYTYPE_PREVIOUS && key_code != NX_KEYTYPE_FAST &&
+      key_code != NX_KEYTYPE_REWIND) {
+    return event;
+  }
+
+  int key_flags = (data1 & 0x0000FFFF);
+  bool is_key_pressed = (((key_flags & 0xFF00) >> 8)) == 0xA;

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

More magic constants that we don’t control. Documentation references are needed.

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

See above.

+
+  // If the key wasn't pressed (eg. was released), ignore this event.
+  if (!is_key_pressed)
+    return event;
+
+  // Now we have a media key that we care about. Send it to the caller.
+  bool was_handled = shortcut_listener->OnMediaKeyEvent(key_code);
+
+  // Prevent event from proagating to other apps if handled by Chrome.
+  if (was_handled)
+    return NULL;
+
+  // By default, pass the event through.
+  return event;
+}
+
+}  // namespace extensions