Allow extensions to remove and override the bookmark shortcut key

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

Index: chrome/browser/extensions/api/commands/command_service.cc
diff --git a/chrome/browser/extensions/api/commands/command_service.cc b/chrome/browser/extensions/api/commands/command_service.cc
index 4c2906def1022f85f6542f1a33c54f2c0ef9dbdf..8d64c41ea2da87764f465bc1cca820fba63744a2 100644
--- a/chrome/browser/extensions/api/commands/command_service.cc
+++ b/chrome/browser/extensions/api/commands/command_service.cc
@@ -11,6 +11,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/commands/commands.h"
 #include "chrome/browser/extensions/extension_commands_global_registry.h"
@@ -20,6 +21,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/accelerator_utils.h"
 #include "chrome/common/extensions/api/commands/commands_handler.h"
+#include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h"
 #include "chrome/common/pref_names.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/notification_details.h"
@@ -27,6 +29,7 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/permissions/permissions_data.h"
 
 using extensions::Extension;
 using extensions::ExtensionPrefs;
@@ -78,6 +81,26 @@ bool InitialBindingsHaveBeenAssigned(
   return assigned;
 }
 
+bool IsReservedChromeAccelerator(const ui::Accelerator& accelerator,
+                                 const Extension* extension,
+                                 Profile* profile) {
+  if (!chrome::IsChromeAccelerator(accelerator, profile))
+    return false;
+
+  using extensions::SettingsOverrides;
+  using extensions::FeatureSwitch;
+  if (SettingsOverrides::Get(extension)->RemovesBookmarkShortcut() &&

[Finnur, 2014/02/05 13:37:40]

I mentioned this in the design doc, but worth repeating here: It is a bit
confusing to be able to assign a shortcut based on the fact that a
remove_shortcut permission is granted. This should really be
AllowBookmarkShortcutOverride(), or something.

[Mike Wittman, 2014/02/05 19:17:07]

I'll update this as necessary pending agreement in the design doc.

+      (extensions::PermissionsData::HasAPIPermission(
+          extension,
+          extensions::APIPermission::kBookmarkManagerPrivate) ||

[Finnur, 2014/02/05 13:37:40]

You mentioned you didn't want a dependency on the bookmarks permission, but you
have a dependency on the bookmarkManager permission instead. Should that be
listed in the design doc?

[Mike Wittman, 2014/02/05 19:17:07]

The check for kBookmarkManagerPrivate API permission is done as a means of
whitelisting Chrome-internal bookmarks extensions to use this functionality. It
has no effect on authors of general extensions, so I don't see a reason to
mention it in the design doc.

+       FeatureSwitch::enable_override_bookmarks_ui()->IsEnabled())) {
+    return accelerator !=
+        chrome::GetChromeAcceleratorForCommandId(IDC_BOOKMARK_PAGE);
+  }
+
+  return true;
+}
+
 bool IsWhitelistedGlobalShortcut(const extensions::Command& command) {
   // Non-global shortcuts are always allowed.
   if (!command.global())
@@ -364,8 +387,9 @@ void CommandService::AssignInitialKeybindings(const Extension* extension) {
   for (; iter != commands->end(); ++iter) {
     // Make sure registered Chrome shortcuts cannot be automatically assigned
     // (overwritten) by extension developers. Media keys are an exception here.
-    if ((!chrome::IsChromeAccelerator(iter->second.accelerator(), profile_) &&
-        IsWhitelistedGlobalShortcut(iter->second)) ||
+    if ((!IsReservedChromeAccelerator(
+            iter->second.accelerator(), extension, profile_) &&
+         IsWhitelistedGlobalShortcut(iter->second)) ||
         extensions::CommandService::IsMediaKey(iter->second.accelerator())) {
       AddKeybindingPref(iter->second.accelerator(),
                         extension->id(),
@@ -378,8 +402,8 @@ void CommandService::AssignInitialKeybindings(const Extension* extension) {
   const extensions::Command* browser_action_command =
       CommandsInfo::GetBrowserActionCommand(extension);
   if (browser_action_command) {
-    if (!chrome::IsChromeAccelerator(
-        browser_action_command->accelerator(), profile_)) {
+    if (!IsReservedChromeAccelerator(
+        browser_action_command->accelerator(), extension, profile_)) {
       AddKeybindingPref(browser_action_command->accelerator(),
                         extension->id(),
                         browser_action_command->command_name(),
@@ -391,8 +415,8 @@ void CommandService::AssignInitialKeybindings(const Extension* extension) {
   const extensions::Command* page_action_command =
       CommandsInfo::GetPageActionCommand(extension);
   if (page_action_command) {
-    if (!chrome::IsChromeAccelerator(
-        page_action_command->accelerator(), profile_)) {
+    if (!IsReservedChromeAccelerator(
+        page_action_command->accelerator(), extension, profile_)) {
       AddKeybindingPref(page_action_command->accelerator(),
                         extension->id(),
                         page_action_command->command_name(),