Port Extension Commands to Mac.

This implements the Mac part of Extension Commands for Browser Actions, Page Actions, Script Badges and Named Commands.

Known issues: Support for specifying Command key as a shortcut is not yet fully implemented.

BUG=27702
TEST=Automated test included.
Review URL: https://chromiumcodereview.appspot.com/10824307

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152076 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/commands/command_service.cc

@@ -125,7 +125,7 @@ bool CommandService::AddKeybindingPref(
   std::string key = GetPlatformKeybindingKeyForAccelerator(accelerator);
 
   if (!allow_overrides && bindings->HasKey(key))
-      return false;  // Already taken.
+    return false;  // Already taken.
 
   DictionaryValue* keybinding = new DictionaryValue();
   keybinding->SetString(kExtension, extension_id);

chrome/browser/extensions/extension_keybinding_apitest.cc

@@ -45,7 +45,6 @@ class ScriptBadgesCommandsApiTest : public ExtensionApiTest {
   virtual ~ScriptBadgesCommandsApiTest() {}
 };
 
-#if !defined(OS_MACOSX)
 // Test the basic functionality of the Keybinding API:
 // - That pressing the shortcut keys should perform actions (activate the
 //   browser action or send an event).
@@ -168,4 +167,3 @@ IN_PROC_BROWSER_TEST_F(ScriptBadgesCommandsApiTest, ScriptBadge_DISABLED) {
   }
 }
 
-#endif  // !OS_MACOSX

chrome/browser/ui/cocoa/browser_window_cocoa.mm

@@ -506,6 +506,10 @@ bool BrowserWindowCocoa::PreHandleKeyboardEvent(
   if (![BrowserWindowUtils shouldHandleKeyboardEvent:event])
     return false;
 
+  if (event.type == WebKit::WebInputEvent::RawKeyDown &&
+      [controller_ handledByExtensionCommand:event.os_event])
+    return true;
+
   int id = [BrowserWindowUtils getCommandId:event];
   if (id == -1)
     return false;

chrome/browser/ui/cocoa/browser_window_controller.h

@@ -34,6 +34,7 @@ class BrowserWindowCocoa;
 @class ChromeToMobileBubbleController;
 @class DevToolsController;
 @class DownloadShelfController;
+class ExtensionKeybindingRegistryCocoa;
 @class FindBarCocoaController;
 @class FullscreenWindow;
 @class GTMWindowSheetController;
@@ -161,6 +162,10 @@ class WebContents;
   // -windowDidEnterFullScreen: gets called.
   GURL fullscreenUrl_;
   FullscreenExitBubbleType fullscreenBubbleType_;
+
+  // The Extension Command Registry used to determine which keyboard events to
+  // handle.
+  scoped_ptr<ExtensionKeybindingRegistryCocoa> extension_keybinding_registry_;
 }
 
 // A convenience class method which gets the |BrowserWindowController| for a
@@ -272,6 +277,10 @@ class WebContents;
 // "chrome/app/chrome_command_ids.h" file.
 - (void)executeCommand:(int)command;
 
+// Consults the Command Registry to see if this |event| needs to be handled as
+// an extension command and returns YES if so (NO otherwise).
+- (BOOL)handledByExtensionCommand:(NSEvent*)event;
+
 // Delegate method for the status bubble to query its base frame.
 - (NSRect)statusBubbleBaseFrame;
 

chrome/browser/ui/cocoa/browser_window_controller.mm

@@ -44,6 +44,7 @@
 #import "chrome/browser/ui/cocoa/dev_tools_controller.h"
 #import "chrome/browser/ui/cocoa/download/download_shelf_controller.h"
 #import "chrome/browser/ui/cocoa/event_utils.h"
+#include "chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.h"
 #import "chrome/browser/ui/cocoa/fast_resize_view.h"
 #import "chrome/browser/ui/cocoa/find_bar/find_bar_bridge.h"
 #import "chrome/browser/ui/cocoa/find_bar/find_bar_cocoa_controller.h"
@@ -413,6 +414,10 @@ enum {
     if ([self hasToolbar])  // Do not create the buttons in popups.
       [toolbarController_ createBrowserActionButtons];
 
+    extension_keybinding_registry_.reset(
+        new ExtensionKeybindingRegistryCocoa(browser_->profile(),
+                                             [self window]));
+
     // We are done initializing now.
     initializing_ = NO;
   }
@@ -1166,6 +1171,11 @@ enum {
   chrome::ExecuteCommand(browser_.get(), command);
 }
 
+- (BOOL)handledByExtensionCommand:(NSEvent*)event {
+  return extension_keybinding_registry_->ProcessKeyEvent(
+      content::NativeWebKeyboardEvent(event));
+}
+
 // StatusBubble delegate method: tell the status bubble the frame it should
 // position itself in.
 - (NSRect)statusBubbleBaseFrame {

chrome/browser/ui/cocoa/chrome_event_processing_window.mm

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #import "chrome/browser/ui/cocoa/browser_command_executor.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller_private.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 #include "chrome/browser/global_keyboard_shortcuts_mac.h"
 #import "content/public/browser/render_widget_host_view_mac_base.h"
@@ -63,6 +64,15 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
   if (redispatchingEvent_)
     return NO;
 
+  NSWindow* window = event.window;
+  if (window) {
+    BrowserWindowController* controller = [window windowController];
+    if ([controller respondsToSelector:@selector(handledByExtensionCommand:)]) {
+      if ([controller handledByExtensionCommand:event])
+        return YES;
+    }
+  }
+
   // Give the web site a chance to handle the event. If it doesn't want to
   // handle it, it will call us back with one of the |handle*| methods above.
   NSResponder* r = [self firstResponder];

chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm

@@ -15,6 +15,7 @@
 #include "chrome/browser/sessions/restore_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/browser_window.h"
 #import "chrome/browser/ui/cocoa/extensions/browser_action_button.h"
 #import "chrome/browser/ui/cocoa/extensions/browser_actions_container_view.h"
 #import "chrome/browser/ui/cocoa/extensions/extension_popup_controller.h"
@@ -175,9 +176,13 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
                                        public ExtensionToolbarModel::Observer {
  public:
   ExtensionServiceObserverBridge(BrowserActionsController* owner,
-                                  Profile* profile) : owner_(owner) {
+                                 Browser* browser)
+    : owner_(owner), browser_(browser) {
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
-                   content::Source<Profile>(profile));
+                   content::Source<Profile>(browser->profile()));
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_EXTENSION_COMMAND_BROWSER_ACTION_MAC,
+                   content::Source<Profile>(browser->profile()));
   }
 
   // Overridden from content::NotificationObserver.
@@ -192,6 +197,25 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
 
         break;
       }
+      case chrome::NOTIFICATION_EXTENSION_COMMAND_BROWSER_ACTION_MAC: {
+        std::pair<const std::string, gfx::NativeWindow>* payload =
+            content::Details<std::pair<const std::string, gfx::NativeWindow> >(
+                details).ptr();
+        std::string extension_id = payload->first;
+        gfx::NativeWindow window = payload->second;
+        if (window != browser_->window()->GetNativeWindow())
+          break;
+        ExtensionService* service = browser_->profile()->GetExtensionService();
+        if (!service)
+          break;
+        const Extension* extension = service->GetExtensionById(extension_id,
+                                                               false);
+        if (!extension)
+          break;
+        BrowserActionButton* button = [owner_ buttonForExtension:extension];
+        [owner_ browserActionClicked:button];
+        break;
+      }
       default:
         NOTREACHED() << L"Unexpected notification";
     }
@@ -212,6 +236,9 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
   // The object we need to inform when we get a notification. Weak. Owns us.
   BrowserActionsController* owner_;
 
+  // The browser we listen for events from. Weak.
+  Browser* browser_;
+
   // Used for registering to receive notifications and automatic clean up.
   content::NotificationRegistrar registrar_;
 
@@ -237,7 +264,7 @@ class ExtensionServiceObserverBridge : public content::NotificationObserver,
         prefs::kBrowserActionContainerWidth))
       [BrowserActionsController registerUserPrefs:profile_->GetPrefs()];
 
-    observer_.reset(new ExtensionServiceObserverBridge(self, profile_));
+    observer_.reset(new ExtensionServiceObserverBridge(self, browser_));
     ExtensionService* extensionService = profile_->GetExtensionService();
     // |extensionService| can be NULL in Incognito.
     if (extensionService) {

chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.h

+// Copyright (c) 2012 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.
+
+#ifndef CHROME_BROWSER_UI_COCOA_EXTENSIONS_EXTENSION_KEYBINDING_REGISTRY_COCOA_H_
+#define CHROME_BROWSER_UI_COCOA_EXTENSIONS_EXTENSION_KEYBINDING_REGISTRY_COCOA_H_
+
+#include <map>
+#include <string>
+#include <utility>
+
+#include "chrome/browser/extensions/extension_keybinding_registry.h"
+#include "ui/base/accelerators/accelerator.h"
+#include "ui/gfx/native_widget_types.h"
+
+class Profile;
+
+namespace content {
+  struct NativeWebKeyboardEvent;
+}
+namespace extensions {
+  class Extension;
+}
+
+// The ExtensionKeybindingRegistryCocoa is the Cocoa specialization of the
+// ExtensionKeybindingRegistry class that handles turning keyboard shortcuts
+// into events that get sent to the extension.
+
+// ExtensionKeybindingRegistryCocoa is a class that handles Cocoa-specific
+// implemenation of the Extension Commands shortcuts (keyboard accelerators).
+// It also routes the events to the intended recipient (ie. to the browser
+// action button in case of browser action commands).
+class ExtensionKeybindingRegistryCocoa
+    : public extensions::ExtensionKeybindingRegistry {
+ public:
+  ExtensionKeybindingRegistryCocoa(Profile* profile, gfx::NativeWindow window);
+  virtual ~ExtensionKeybindingRegistryCocoa();
+
+  static void set_shortcut_handling_suspended(bool suspended) {
+    shortcut_handling_suspended_ = suspended;
+  }
+  static bool shortcut_handling_suspended() {
+    return shortcut_handling_suspended_;
+  }
+
+  // For a given keyboard |event|, see if a known Extension Command registration
+  // exists and route the event to it. Returns true if the event was handled,
+  // false otherwise.
+  bool ProcessKeyEvent(const content::NativeWebKeyboardEvent& event);
+
+ protected:
+  // Overridden from ExtensionKeybindingRegistry:
+  virtual void AddExtensionKeybinding(
+      const extensions::Extension* extension,
+      const std::string& command_name) OVERRIDE;
+  virtual void RemoveExtensionKeybinding(
+      const extensions::Extension* extension,
+      const std::string& command_name) OVERRIDE;
+
+ private:
+  // Keeps track of whether shortcut handling is currently suspended. Shortcuts
+  // are suspended briefly while capturing which shortcut to assign to an
+  // extension command in the Config UI. If handling isn't suspended while
+  // capturing then trying to assign Ctrl+F to a command would instead result
+  // in the Find box opening.
+  static bool shortcut_handling_suspended_;
+
+  // Weak pointer to the our profile. Not owned by us.
+  Profile* profile_;
+
+  // The window we are associated with.
+  gfx::NativeWindow window_;
+
+  // Maps an accelerator to a string pair (extension id, command name) for
+  // commands that have been registered. Unlike its Views counterpart, this map
+  // contains browserAction and pageAction commands (but does not route those
+  // events), so that we can query priority handlers in HasPriorityHandler(...).
+  typedef std::map< ui::Accelerator,
+                    std::pair<std::string, std::string> > EventTargets;
+  EventTargets event_targets_;
+
+  // The content notification registrar for listening to extension events.
+  content::NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionKeybindingRegistryCocoa);
+};
+
+#endif  // CHROME_BROWSER_UI_COCOA_EXTENSIONS_EXTENSION_KEYBINDING_REGISTRY_COCOA_H_

chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm

+// Copyright (c) 2012 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/ui/cocoa/extensions/extension_keybinding_registry_cocoa.h"
+
+#include "chrome/browser/extensions/api/commands/command_service.h"
+#include "chrome/browser/extensions/api/commands/command_service_factory.h"
+#include "chrome/browser/extensions/browser_event_router.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
+#include "content/public/browser/native_web_keyboard_event.h"
+#include "content/public/browser/notification_service.h"
+
+namespace values = extension_manifest_values;
+
+// static
+void extensions::ExtensionKeybindingRegistry::SetShortcutHandlingSuspended(
+    bool suspended) {
+  ExtensionKeybindingRegistryCocoa::set_shortcut_handling_suspended(suspended);
+}
+
+bool ExtensionKeybindingRegistryCocoa::shortcut_handling_suspended_ = false;
+
+ExtensionKeybindingRegistryCocoa::ExtensionKeybindingRegistryCocoa(
+    Profile* profile, gfx::NativeWindow window)
+    : ExtensionKeybindingRegistry(profile),
+      profile_(profile),
+      window_(window) {
+  Init();
+}
+
+ExtensionKeybindingRegistryCocoa::~ExtensionKeybindingRegistryCocoa() {
+}
+
+bool ExtensionKeybindingRegistryCocoa::ProcessKeyEvent(
+    const content::NativeWebKeyboardEvent& event) {
+  if (shortcut_handling_suspended_)
+    return false;
+
+  ui::Accelerator accelerator(
+      static_cast<ui::KeyboardCode>(event.windowsKeyCode),
+      content::GetModifiersFromNativeWebKeyboardEvent(event));
+  EventTargets::iterator it = event_targets_.find(accelerator);
+  if (it == event_targets_.end())
+    return false;
+
+  std::string extension_id = it->second.first;
+  std::string command_name = it->second.second;
+  int type = 0;
+  if (command_name == values::kPageActionCommandEvent) {
+    type = chrome::NOTIFICATION_EXTENSION_COMMAND_PAGE_ACTION_MAC;
+  } else if (command_name == values::kBrowserActionCommandEvent) {
+    type = chrome::NOTIFICATION_EXTENSION_COMMAND_BROWSER_ACTION_MAC;
+  } else if (command_name == values::kScriptBadgeCommandEvent) {
+    type = chrome::NOTIFICATION_EXTENSION_COMMAND_SCRIPT_BADGE_MAC;
+  } else {
+    // Not handled by using notifications. Route it through the Browser Event
+    // Router.
+    ExtensionService* service = profile_->GetExtensionService();
+    service->browser_event_router()->CommandExecuted(
+        profile_, extension_id, command_name);
+    return true;
+  }
+
+  std::pair<const std::string, gfx::NativeWindow> details =
+      std::make_pair(extension_id, window_);
+  content::NotificationService::current()->Notify(
+      type,
+      content::Source<Profile>(profile_),
+      content::Details<
+          std::pair<const std::string, gfx::NativeWindow> >(&details));
+  return true;
+}
+
+void ExtensionKeybindingRegistryCocoa::AddExtensionKeybinding(
+    const extensions::Extension* extension,
+    const std::string& command_name) {
+  extensions::CommandService* command_service =
+      extensions::CommandServiceFactory::GetForProfile(profile_);
+  extensions::CommandMap commands;
+  command_service->GetNamedCommands(
+          extension->id(),
+          extensions::CommandService::ACTIVE_ONLY,
+          &commands);
+
+  for (extensions::CommandMap::const_iterator iter = commands.begin();
+       iter != commands.end(); ++iter) {
+    if (!command_name.empty() && (iter->second.command_name() != command_name))
+      continue;
+
+    ui::Accelerator accelerator(iter->second.accelerator());
+    event_targets_[accelerator] =
+        std::make_pair(extension->id(), iter->second.command_name());
+  }
+
+  // Mac implemenetation behaves like GTK with regards to what is kept in the
+  // event_targets_ map, because both GTK and Mac need to keep track of Browser
+  // and Page actions, as well as Script Badges.
+  extensions::Command browser_action;
+  if (command_service->GetBrowserActionCommand(
+          extension->id(),
+          extensions::CommandService::ACTIVE_ONLY,
+          &browser_action,
+          NULL)) {
+    ui::Accelerator accelerator(browser_action.accelerator());
+    event_targets_[accelerator] =
+        std::make_pair(extension->id(), browser_action.command_name());
+  }
+
+  // Add the Page Action (if any).
+  extensions::Command page_action;
+  if (command_service->GetPageActionCommand(
+          extension->id(),
+          extensions::CommandService::ACTIVE_ONLY,
+          &page_action,
+          NULL)) {
+    ui::Accelerator accelerator(page_action.accelerator());
+    event_targets_[accelerator] =
+        std::make_pair(extension->id(), page_action.command_name());
+  }
+
+  // Add the Script Badge (if any).
+  extensions::Command script_badge;
+  if (command_service->GetScriptBadgeCommand(
+          extension->id(),
+          extensions::CommandService::ACTIVE_ONLY,
+          &script_badge,
+          NULL)) {
+    ui::Accelerator accelerator(script_badge.accelerator());
+    event_targets_[accelerator] =
+        std::make_pair(extension->id(), script_badge.command_name());
+  }
+}
+
+void ExtensionKeybindingRegistryCocoa::RemoveExtensionKeybinding(
+    const extensions::Extension* extension,
+    const std::string& command_name) {
+  EventTargets::iterator iter = event_targets_.begin();
+  while (iter != event_targets_.end()) {
+    EventTargets::iterator old = iter++;
+    if (old->second.first == extension->id() &&
+        (command_name.empty() || (old->second.second == command_name)))
+      event_targets_.erase(old);
+  }
+}

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h

@@ -126,6 +126,9 @@ class LocationBarViewMac : public LocationBar,
   void SetPreviewEnabledPageAction(ExtensionAction* page_action,
                                    bool preview_enabled);
 
+  // Retrieve the frame for the given |page_action|.
+  NSRect GetPageActionFrame(ExtensionAction* page_action);
+
   // Return |page_action|'s info-bubble point in window coordinates.
   // This function should always be called with a visible page action.
   // If |page_action| is not a page action or not visible, NOTREACHED()

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm

@@ -403,15 +403,24 @@ void LocationBarViewMac::SetPreviewEnabledPageAction(
   decoration->UpdateVisibility(contents, GURL(toolbar_model_->GetText()));
 }
 
-NSPoint LocationBarViewMac::GetPageActionBubblePoint(
-    ExtensionAction* page_action) {
+NSRect LocationBarViewMac::GetPageActionFrame(ExtensionAction* page_action) {
   PageActionDecoration* decoration = GetPageActionDecoration(page_action);
   if (!decoration)
-    return NSZeroPoint;
+    return NSZeroRect;
 
   AutocompleteTextFieldCell* cell = [field_ cell];
   NSRect frame = [cell frameForDecoration:decoration inFrame:[field_ bounds]];
   DCHECK(!NSIsEmptyRect(frame));
+  return frame;
+}
+
+NSPoint LocationBarViewMac::GetPageActionBubblePoint(
+    ExtensionAction* page_action) {
+  PageActionDecoration* decoration = GetPageActionDecoration(page_action);
+  if (!decoration)
+    return NSZeroPoint;
+
+  NSRect frame = GetPageActionFrame(page_action);
   if (NSIsEmptyRect(frame))
     return NSZeroPoint;
 

chrome/browser/ui/cocoa/location_bar/page_action_decoration.h

@@ -62,6 +62,9 @@ class PageActionDecoration : public ImageDecoration,
   virtual NSMenu* GetMenu() OVERRIDE;
 
  private:
+  // Activate the page action in the given |frame|.
+  bool ActivatePageAction(NSRect frame);
+
   // Show the popup in the frame, with the given URL.
   void ShowPopup(const NSRect& frame, const GURL& popup_url);
 

chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm

@@ -14,6 +14,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
 #import "chrome/browser/ui/cocoa/extensions/extension_action_context_menu.h"
 #import "chrome/browser/ui/cocoa/extensions/extension_popup_controller.h"
 #include "chrome/browser/ui/cocoa/last_active_browser_cocoa.h"
@@ -75,6 +76,10 @@ PageActionDecoration::PageActionDecoration(
 
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
       content::Source<Profile>(browser_->profile()));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_COMMAND_PAGE_ACTION_MAC,
+      content::Source<Profile>(browser_->profile()));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_COMMAND_SCRIPT_BADGE_MAC,
+      content::Source<Profile>(browser_->profile()));
 
   // We set the owner last of all so that we can determine whether we are in
   // the process of initializing this class or not.
@@ -96,6 +101,10 @@ bool PageActionDecoration::AcceptsMousePress() {
 // Either notify listeners or show a popup depending on the Page
 // Action.
 bool PageActionDecoration::OnMousePressed(NSRect frame) {
+  return ActivatePageAction(frame);
+}
+
+bool PageActionDecoration::ActivatePageAction(NSRect frame) {
   TabContents* tab_contents = owner_->GetTabContents();
   if (!tab_contents) {
     // We don't want other code to try and handle this click. Returning true
@@ -265,6 +274,22 @@ void PageActionDecoration::Observe(
 
       break;
     }
+    case chrome::NOTIFICATION_EXTENSION_COMMAND_PAGE_ACTION_MAC:
+    case chrome::NOTIFICATION_EXTENSION_COMMAND_SCRIPT_BADGE_MAC: {
+      std::pair<const std::string, gfx::NativeWindow>* payload =
+      content::Details<std::pair<const std::string, gfx::NativeWindow> >(
+          details).ptr();
+      std::string extension_id = payload->first;
+      gfx::NativeWindow window = payload->second;
+      if (window != browser_->window()->GetNativeWindow())
+        break;
+      if (extension_id != page_action_->extension_id())
+        break;
+      NSRect frame = owner_->GetPageActionFrame(page_action_);
+      ActivatePageAction(frame);
+      break;
+    }
+
     default:
       NOTREACHED() << "Unexpected notification";
       break;

chrome/browser/ui/webui/extensions/command_handler.cc

@@ -83,13 +83,9 @@ void CommandHandler::HandleSetExtensionCommandShortcut(
 }
 
 void CommandHandler::HandleSetShortcutHandlingSuspended(const ListValue* args) {
-#if !defined(OS_MACOSX)
   bool suspended;
   if (args->GetBoolean(0, &suspended))
     ExtensionKeybindingRegistry::SetShortcutHandlingSuspended(suspended);
-#else
-  NOTIMPLEMENTED();
-#endif
 }
 
 void CommandHandler::GetAllCommands(base::DictionaryValue* commands) {

chrome/chrome_browser.gypi

@@ -2758,6 +2758,8 @@
         'browser/ui/cocoa/extensions/extension_installed_bubble_bridge.mm',
         'browser/ui/cocoa/extensions/extension_installed_bubble_controller.h',
         'browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm',
+        'browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.h',
+        'browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm',
         'browser/ui/cocoa/extensions/extension_popup_controller.h',
         'browser/ui/cocoa/extensions/extension_popup_controller.mm',
         'browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm',

chrome/common/chrome_notification_types.h

@@ -589,6 +589,21 @@ enum NotificationType {
   // and the name of the command being added).
   NOTIFICATION_EXTENSION_COMMAND_ADDED,
 
+  // Sent when an extension command shortcut for a browser action is activated
+  // on Mac. The source is the profile and the details is a std::string
+  // containing an extension ID.
+  NOTIFICATION_EXTENSION_COMMAND_BROWSER_ACTION_MAC,
+
+  // Sent when an extension command shortcut for a page action is activated
+  // on Mac. The source is the profile and the details is a std::string
+  // containing an extension ID.
+  NOTIFICATION_EXTENSION_COMMAND_PAGE_ACTION_MAC,
+
+  // Sent when an extension command shortcut for a script badge is activated
+  // on Mac. The source is the profile and the details is a std::string
+  // containing an extension ID.
+  NOTIFICATION_EXTENSION_COMMAND_SCRIPT_BADGE_MAC,
+
   // A new extension RenderViewHost has been registered. The details are
   // the RenderViewHost*.
   NOTIFICATION_EXTENSION_VIEW_REGISTERED,

chrome/common/extensions/command.cc

@@ -53,15 +53,13 @@ ui::Accelerator ParseImpl(const std::string& accelerator,
   for (size_t i = 0; i < tokens.size(); i++) {
     if (tokens[i] == "Ctrl") {
       modifiers |= ui::EF_CONTROL_DOWN;
-    } else if (tokens[i] == "Alt") {
+    } else if (tokens[i] == "Alt" ||
+               (tokens[i] == "Option" && platform_key == "mac")) {
       modifiers |= ui::EF_ALT_DOWN;
     } else if (tokens[i] == "Shift") {
       modifiers |= ui::EF_SHIFT_DOWN;
     } else if (tokens[i] == "Command" && platform_key == "mac") {
-      // TODO(finnur): Implement for Mac.
-      // TODO(finnur): Reject Shift modifier if no Cmd/Opt (see below).
-    } else if (tokens[i] == "Option" && platform_key == "mac") {
-      // TODO(finnur): Implement for Mac.
+      modifiers |= ui::EF_COMMAND_DOWN;
     } else if (tokens[i].size() == 1) {
       if (key != ui::VKEY_UNKNOWN) {
         // Multiple key assignments.
@@ -85,15 +83,19 @@ ui::Accelerator ParseImpl(const std::string& accelerator,
       return ui::Accelerator();
     }
   }
+  bool command = (modifiers & ui::EF_COMMAND_DOWN) != 0;
   bool ctrl = (modifiers & ui::EF_CONTROL_DOWN) != 0;
   bool alt = (modifiers & ui::EF_ALT_DOWN) != 0;
   bool shift = (modifiers & ui::EF_SHIFT_DOWN) != 0;
+
   // We support Ctrl+foo, Alt+foo, Ctrl+Shift+foo, Alt+Shift+foo, but not
   // Ctrl+Alt+foo and not Shift+foo either. For a more detailed reason why we
   // don't support Ctrl+Alt+foo see this article:
   // http://blogs.msdn.com/b/oldnewthing/archive/2004/03/29/101121.aspx.
+  // On Mac Command can also be used in combination with Shift or on its own,
+  // as a modifier.
   if (key == ui::VKEY_UNKNOWN || (ctrl && alt) ||
-      ((platform_key != "mac") && shift && !ctrl && !alt)) {
+      (shift && !ctrl && !alt && !command)) {
     *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
         errors::kInvalidKeyBinding,
         base::IntToString(index),

chrome/test/data/extensions/api_test/keybinding/basics/manifest.json

@@ -16,7 +16,7 @@
     "toggle-feature": {
       "suggested_key": {
         "windows": "Ctrl+Shift+Y",
-        "mac": "Command+Shift+Y",
+        "mac": "Ctrl+Shift+Y",
         "linux": "Ctrl+Shift+Y",
         "chromeos": "Ctrl+Shift+Y",
         "default": "Ctrl+Shift+Y"
@@ -25,7 +25,6 @@
     },
     "_execute_browser_action": {
       "suggested_key": {
-        "mac": "Command+Shift+F",
         "default": "Ctrl+Shift+F"
       }
     }

ui/base/accelerators/accelerator.cc

@@ -17,7 +17,7 @@
 #include "grit/ui_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
-#if !defined(OS_WIN) && defined(USE_AURA)
+#if !defined(OS_WIN) && (defined(USE_AURA) || defined(OS_MACOSX))
 #include "ui/base/keycodes/keyboard_code_conversion.h"
 #endif
 
@@ -146,11 +146,10 @@ string16 Accelerator::GetShortcutText() const {
     else
       key = LOWORD(::MapVirtualKeyW(key_code_, MAPVK_VK_TO_CHAR));
     shortcut += key;
-#elif defined(USE_AURA)
+#elif defined(USE_AURA) || defined(OS_MACOSX)
     const uint16 c = GetCharacterFromKeyCode(key_code_, false);
-    if (c != 0) {
+    if (c != 0)
       shortcut += static_cast<string16::value_type>(base::ToUpperASCII(c));
-    }
 #elif defined(TOOLKIT_GTK)
     const gchar* name = NULL;
     switch (key_code_) {