Extension Command Config UI part 2.

Part 1 was capturing what the user selected as the keyboard shortcut to trigger something in the extension. This changelist is about making it take effect at restart (of the browser). Part 3 will be about making it take effect immediately.

BUG=121420
TEST=Change the keyboard shortcut assignment for an extension command. Restart the browser. Make sure the new keyboard shortcut is active.
Review URL: https://chromiumcodereview.appspot.com/10633002

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

2 files changed, 168 insertions, 100 deletions

diff --git a/chrome/browser/extensions/api/commands/command_service.cc b/chrome/browser/extensions/api/commands/command_service.cc
index 6f91361..b6f76bc 100644
--- a/chrome/browser/extensions/api/commands/command_service.cc
+++ b/chrome/browser/extensions/api/commands/command_service.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/api/commands/command_service.h"
 
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_keybinding_registry.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -11,6 +12,7 @@
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_service.h"
 
@@ -49,99 +51,51 @@ CommandService::CommandService(Profile* profile)
 CommandService::~CommandService() {
 }
 
-const extensions::Command* CommandService::GetBrowserActionCommand(
-    const std::string& extension_id, QueryType type) {
-  const ExtensionSet* extensions =
-      ExtensionSystem::Get(profile_)->extension_service()->extensions();
-  const Extension* extension = extensions->GetByID(extension_id);
-  CHECK(extension);
-
-  const extensions::Command* command = extension->browser_action_command();
-  if (!command)
-    return NULL;
-  if (type == ACTIVE_ONLY &&
-      !IsKeybindingActive(command->accelerator(),
-                          extension_id,
-                          command->command_name())) {
-    return NULL;
-  }
-
-  return command;
+bool CommandService::GetBrowserActionCommand(
+    const std::string& extension_id,
+    QueryType type,
+    extensions::Command* command,
+    bool* active) {
+  return GetExtensionActionCommand(extension_id, type, command, active, true);
 }
 
-const extensions::Command* CommandService::GetPageActionCommand(
-    const std::string& extension_id, QueryType type) {
-  const ExtensionSet* extensions =
-      ExtensionSystem::Get(profile_)->extension_service()->extensions();
-  const Extension* extension = extensions->GetByID(extension_id);
-  CHECK(extension);
-
-  const extensions::Command* command = extension->page_action_command();
-  if (!command)
-    return NULL;
-  if (type == ACTIVE_ONLY &&
-      !IsKeybindingActive(command->accelerator(),
-                          extension_id,
-                          command->command_name())) {
-    return NULL;
-  }
-
-  return command;
+bool CommandService::GetPageActionCommand(
+    const std::string& extension_id,
+    QueryType type,
+    extensions::Command* command,
+    bool* active) {
+  return GetExtensionActionCommand(extension_id, type, command, active, false);
 }
 
-extensions::CommandMap CommandService::GetNamedCommands(
-    const std::string& extension_id, QueryType type) {
+bool CommandService::GetNamedCommands(const std::string& extension_id,
+                                      QueryType type,
+                                      extensions::CommandMap* command_map) {
   const ExtensionSet* extensions =
       ExtensionSystem::Get(profile_)->extension_service()->extensions();
   const Extension* extension = extensions->GetByID(extension_id);
   CHECK(extension);
 
-  extensions::CommandMap result;
+  command_map->clear();
   const extensions::CommandMap& commands = extension->named_commands();
   if (commands.empty())
-    return result;
+    return false;
 
   extensions::CommandMap::const_iterator iter = commands.begin();
   for (; iter != commands.end(); ++iter) {
-    if (type == ACTIVE_ONLY &&
-        !IsKeybindingActive(iter->second.accelerator(),
-                            extension_id,
-                            iter->second.command_name())) {
-      continue;
-    }
+    ui::Accelerator shortcut_assigned =
+        FindShortcutForCommand(extension_id, iter->second.command_name());
 
-    result[iter->second.command_name()] = iter->second;
-  }
-
-  return result;
-}
-
-bool CommandService::IsKeybindingActive(
-    const ui::Accelerator& accelerator,
-    const std::string& extension_id,
-    const std::string& command_name) const {
-  CHECK(!extension_id.empty());
-  CHECK(!command_name.empty());
-
-  std::string key = GetPlatformKeybindingKeyForAccelerator(accelerator);
-  const DictionaryValue* bindings =
-      profile_->GetPrefs()->GetDictionary(prefs::kExtensionKeybindings);
-  if (!bindings->HasKey(key))
-    return false;
-
-  DictionaryValue* value = NULL;
-  if (!bindings->GetDictionary(key, &value))
-    return false;
+    if (type == ACTIVE_ONLY && shortcut_assigned.key_code() == ui::VKEY_UNKNOWN)
+      continue;
 
-  std::string id;
-  if (!value->GetString(kExtension, &id) || id != extension_id)
-    return false;  // Not active for this extension.
+    extensions::Command command = iter->second;
+    if (shortcut_assigned.key_code() != ui::VKEY_UNKNOWN)
+      command.set_accelerator(shortcut_assigned);
 
-  std::string command;
-  if (!value->GetString(kCommandName, &command) || command != command_name)
-    return false;  // Not active for this command.
+    (*command_map)[iter->second.command_name()] = command;
+  }
 
-  return true;  // We found a match, this one is active.
+  return true;
 }
 
 bool CommandService::AddKeybindingPref(
@@ -149,13 +103,17 @@ bool CommandService::AddKeybindingPref(
     std::string extension_id,
     std::string command_name,
     bool allow_overrides) {
+  if (accelerator.key_code() == ui::VKEY_UNKNOWN)
+    return false;
+
   DictionaryPrefUpdate updater(profile_->GetPrefs(),
                                prefs::kExtensionKeybindings);
   DictionaryValue* bindings = updater.Get();
 
   std::string key = GetPlatformKeybindingKeyForAccelerator(accelerator);
-  if (bindings->HasKey(key) && !allow_overrides)
-    return false;  // Already taken.
+
+  if (!allow_overrides && bindings->HasKey(key))
+      return false;  // Already taken.
 
   DictionaryValue* keybinding = new DictionaryValue();
   keybinding->SetString(kExtension, extension_id);
@@ -175,7 +133,8 @@ void CommandService::Observe(
           content::Details<const Extension>(details).ptr());
       break;
     case chrome::NOTIFICATION_EXTENSION_UNINSTALLED:
-      RemoveKeybindingPrefs(*content::Details<std::string>(details).ptr());
+      RemoveKeybindingPrefs(*content::Details<std::string>(details).ptr(),
+                            std::string());
       break;
     default:
       NOTREACHED();
@@ -183,6 +142,45 @@ void CommandService::Observe(
   }
 }
 
+void CommandService::UpdateKeybindingPrefs(const std::string& extension_id,
+                                           const std::string& command_name,
+                                           const std::string& keystroke) {
+  // The extension command might be assigned another shortcut. Remove that
+  // shortcut before proceeding.
+  RemoveKeybindingPrefs(extension_id, command_name);
+
+  ui::Accelerator accelerator = Command::StringToAccelerator(keystroke);
+  AddKeybindingPref(accelerator, extension_id, command_name, true);
+}
+
+ui::Accelerator CommandService::FindShortcutForCommand(
+    const std::string& extension_id, const std::string& command) {
+  const DictionaryValue* bindings =
+      profile_->GetPrefs()->GetDictionary(prefs::kExtensionKeybindings);
+  for (DictionaryValue::key_iterator it = bindings->begin_keys();
+       it != bindings->end_keys(); ++it) {
+    DictionaryValue* item = NULL;
+    bindings->GetDictionary(*it, &item);
+
+    std::string extension;
+    item->GetString(kExtension, &extension);
+    if (extension != extension_id)
+      continue;
+    std::string command_name;
+    item->GetString(kCommandName, &command_name);
+    if (command != command_name)
+      continue;
+
+    std::string shortcut = *it;
+    if (StartsWithASCII(shortcut, Command::CommandPlatform() + ":", true))
+      shortcut = shortcut.substr(Command::CommandPlatform().length() + 1);
+
+    return Command::StringToAccelerator(shortcut);
+  }
+
+  return ui::Accelerator();
+}
+
 void CommandService::AssignInitialKeybindings(const Extension* extension) {
   const extensions::CommandMap& commands = extension->named_commands();
   extensions::CommandMap::const_iterator iter = commands.begin();
@@ -212,7 +210,8 @@ void CommandService::AssignInitialKeybindings(const Extension* extension) {
   }
 }
 
-void CommandService::RemoveKeybindingPrefs(std::string extension_id) {
+void CommandService::RemoveKeybindingPrefs(const std::string& extension_id,
+                                           const std::string& command_name) {
   DictionaryPrefUpdate updater(profile_->GetPrefs(),
                                prefs::kExtensionKeybindings);
   DictionaryValue* bindings = updater.Get();
@@ -227,8 +226,19 @@ void CommandService::RemoveKeybindingPrefs(std::string extension_id) {
 
     std::string extension;
     item->GetString(kExtension, &extension);
-    if (extension == extension_id)
+
+    if (extension == extension_id) {
+      // If |command_name| is specified, delete only that command. Otherwise,
+      // delete all commands.
+      if (!command_name.empty()) {
+        std::string command;
+        item->GetString(kCommandName, &command);
+        if (command_name != command)
+          continue;
+      }
+
       keys_to_remove.push_back(key);
+    }
   }
 
   for (KeysToRemove::const_iterator it = keys_to_remove.begin();
@@ -237,4 +247,39 @@ void CommandService::RemoveKeybindingPrefs(std::string extension_id) {
   }
 }
 
+bool CommandService::GetExtensionActionCommand(const std::string& extension_id,
+                                               QueryType type,
+                                               extensions::Command* command,
+                                               bool* active,
+                                               bool browser_action) {
+  const ExtensionSet* extensions =
+      ExtensionSystem::Get(profile_)->extension_service()->extensions();
+  const Extension* extension = extensions->GetByID(extension_id);
+  CHECK(extension);
+
+  if (active)
+    *active = false;
+
+  const extensions::Command* requested_command =
+      browser_action ? extension->browser_action_command() :
+                       extension->page_action_command();
+  if (!requested_command)
+    return false;
+
+  ui::Accelerator shortcut_assigned =
+      FindShortcutForCommand(extension_id, requested_command->command_name());
+
+  if (active)
+    *active = (shortcut_assigned.key_code() != ui::VKEY_UNKNOWN);
+
+  if (type == ACTIVE_ONLY && shortcut_assigned.key_code() == ui::VKEY_UNKNOWN)
+    return false;
+
+  *command = *requested_command;
+  if (shortcut_assigned.key_code() != ui::VKEY_UNKNOWN)
+    command->set_accelerator(shortcut_assigned);
+
+  return true;
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/commands/command_service.h b/chrome/browser/extensions/api/commands/command_service.h
index a8d18812..36fbd90 100644
--- a/chrome/browser/extensions/api/commands/command_service.h
+++ b/chrome/browser/extensions/api/commands/command_service.h
@@ -52,34 +52,34 @@ class CommandService : public ProfileKeyedService,
 
   // Gets the keybinding (if any) for the browser action of an extension given
   // its |extension_id|. The function consults the master list to see if
-  // the keybinding is active. Returns NULL if the extension has no browser
-  // action. Returns NULL if the keybinding is not active and |type| requested
-  // is ACTIVE_ONLY.
-  const extensions::Command* GetBrowserActionCommand(
-      const std::string& extension_id, QueryType type);
+  // the keybinding is active. Returns false if the extension has no browser
+  // action. Returns false if the keybinding is not active and |type| requested
+  // is ACTIVE_ONLY. |command| contains the command found and |active| (if not
+  // NULL) contains whether |command| is active.
+  bool GetBrowserActionCommand(const std::string& extension_id,
+                               QueryType type,
+                               extensions::Command* command,
+                               bool* active);
 
   // Gets the keybinding (if any) for the page action of an extension given
   // its |extension_id|. The function consults the master list to see if
-  // the keybinding is active. Returns NULL if the extension has no page
-  // action. Returns NULL if the keybinding is not active and |type| requested
-  // is ACTIVE_ONLY.
-  const extensions::Command* GetPageActionCommand(
-      const std::string& extension_id, QueryType type);
+  // the keybinding is active. Returns false if the extension has no page
+  // action. Returns false if the keybinding is not active and |type| requested
+  // is ACTIVE_ONLY. |command| contains the command found and |active| (if not
+  // NULL) contains whether |command| is active.
+  bool GetPageActionCommand(const std::string& extension_id,
+                            QueryType type,
+                            extensions::Command* command,
+                            bool* active);
 
   // Gets the active keybinding (if any) for the named commands of an extension
   // given its |extension_id|. The function consults the master list to see if
   // the keybinding is active. Returns an empty map if the extension has no
   // named commands or no active named commands when |type| requested is
   // ACTIVE_ONLY.
-  extensions::CommandMap GetNamedCommands(
-      const std::string& extension_id, QueryType type);
-
-  // Checks to see if a keybinding |accelerator| for a given |command_name| in
-  // an extension with id |extension_id| is registered as active (by consulting
-  // the master list in |user_prefs|).
-  bool IsKeybindingActive(const ui::Accelerator& accelerator,
-                          const std::string& extension_id,
-                          const std::string& command_name) const;
+  bool GetNamedCommands(const std::string& extension_id,
+                        QueryType type,
+                        extensions::CommandMap* command_map);
 
   // Records a keybinding |accelerator| as active for an extension with id
   // |extension_id| and command with the name |command_name|. If
@@ -93,6 +93,20 @@ class CommandService : public ProfileKeyedService,
                          std::string command_name,
                          bool allow_overrides);
 
+  // Update the keybinding prefs (for a command with a matching |extension_id|
+  // and |command_name|) to |keystroke|. If the command had another key assigned
+  // that key assignment will be removed.
+  void UpdateKeybindingPrefs(const std::string& extension_id,
+                             const std::string& command_name,
+                             const std::string& keystroke);
+
+  // Finds the shortcut assigned to a command with the name |command_name|
+  // within an extension with id |extension_id|. Returns an empty Accelerator
+  // object (with keycode VKEY_UNKNOWN) if no shortcut is assigned or the
+  // command is not found.
+  ui::Accelerator FindShortcutForCommand(const std::string& extension_id,
+                                         const std::string& command);
+
   // Overridden from content::NotificationObserver.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -107,7 +121,16 @@ class CommandService : public ProfileKeyedService,
   void AssignInitialKeybindings(const extensions::Extension* extension);
 
   // Removes all keybindings for a given extension by its |extension_id|.
-  void RemoveKeybindingPrefs(std::string extension_id);
+  // |command_name| is optional and if specified, causes only the command with
+  // the name |command_name| to be removed.
+  void RemoveKeybindingPrefs(const std::string& extension_id,
+                             const std::string& command_name);
+
+  bool GetExtensionActionCommand(const std::string& extension_id,
+                                 QueryType type,
+                                 extensions::Command* command,
+                                 bool* active,
+                                 bool browser_action);
 
   // The content notification registrar for listening to extension events.
   content::NotificationRegistrar registrar_;