Implement first part of supporting global extension commands.

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 5d38936f1aba3355f5fa43942d57741e4bc2eae1..63a16a5628f9614690eaaf79164ec0571d75b539 100644
--- a/chrome/browser/extensions/api/commands/command_service.cc
+++ b/chrome/browser/extensions/api/commands/command_service.cc
@@ -9,6 +9,7 @@
 #include "base/strings/utf_string_conversions.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"
 #include "chrome/browser/extensions/extension_function_registry.h"
 #include "chrome/browser/extensions/extension_keybinding_registry.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -57,6 +58,15 @@ bool InitialBindingsHaveBeenAssigned(
   return assigned;
 }
 
+bool WhitelistedGlobalShortcut(const ui::Accelerator& accelerator) {

[Yoyo Zhou, 2013/10/03 22:08:50]

IsWhitelisted... seems like a clearer name.

[Finnur, 2013/10/04 17:57:02]

Agreed. I also added the scope check here (good catch).

+  if (!accelerator.IsCtrlDown())
+    return false;
+  if (!accelerator.IsShiftDown())
+    return false;
+  return (accelerator.key_code() >= ui::VKEY_0 &&
+          accelerator.key_code() <= ui::VKEY_9);
+}
+
 }  // namespace
 
 namespace extensions {
@@ -74,6 +84,8 @@ CommandService::CommandService(Profile* profile)
   ExtensionFunctionRegistry::GetInstance()->
       RegisterFunction<GetAllCommandsFunction>();
 
+  ExtensionCommandsGlobalRegistry::GetFactoryInstance()->GetForProfile(profile);

[Yoyo Zhou, 2013/10/03 22:08:50]

A better way to do this is to call DependsOn in
CommandService::DeclareFactoryDependencies.

[Finnur, 2013/10/04 17:57:02]

Done.

+
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALLED,
       content::Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
@@ -125,9 +137,13 @@ bool CommandService::GetScriptBadgeCommand(
 
 bool CommandService::GetNamedCommands(const std::string& extension_id,
                                       QueryType type,
+                                      CommandScope scope,
                                       extensions::CommandMap* command_map) {
-  const ExtensionSet* extensions =
-      ExtensionSystem::Get(profile_)->extension_service()->extensions();
+  ExtensionService* extension_service =
+      ExtensionSystem::Get(profile_)->extension_service();
+  if (!extension_service)
+    return false;  // Can occur during testing.
+  const ExtensionSet* extensions = extension_service->extensions();
   const Extension* extension = extensions->GetByID(extension_id);
   CHECK(extension);
 
@@ -146,6 +162,9 @@ bool CommandService::GetNamedCommands(const std::string& extension_id,
       continue;
 
     extensions::Command command = iter->second;
+    if (scope != ANY_SCOPE && ((scope == GLOBAL) != command.global()))
+      continue;
+
     if (shortcut_assigned.key_code() != ui::VKEY_UNKNOWN)
       command.set_accelerator(shortcut_assigned);
 
@@ -265,7 +284,8 @@ void CommandService::AssignInitialKeybindings(const Extension* extension) {
   extensions::CommandMap::const_iterator iter = commands->begin();
   for (; iter != commands->end(); ++iter) {
     if (!chrome::IsChromeAccelerator(
-        iter->second.accelerator(), profile_)) {
+            iter->second.accelerator(), profile_) &&
+        WhitelistedGlobalShortcut(iter->second.accelerator())) {

[Yoyo Zhou, 2013/10/03 22:08:50]

Why is this check here regardless of the scope?

[Finnur, 2013/10/04 17:57:02]

Fixed (see above).

       AddKeybindingPref(iter->second.accelerator(),
                         extension->id(),
                         iter->second.command_name(),