Conflict detection for Extension Keybinding.

chrome/browser/extensions/extension_keybinding_registry.h

Index: chrome/browser/extensions/extension_keybinding_registry.h
===================================================================
--- chrome/browser/extensions/extension_keybinding_registry.h	(revision 133458)
+++ chrome/browser/extensions/extension_keybinding_registry.h	(working copy)
@@ -9,14 +9,19 @@
 #include <string>
 
 #include "base/compiler_specific.h"
+#include "chrome/common/extensions/extension.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
 
 class Extension;
+class PrefService;
 class Profile;
 
+namespace ui {
+class Accelerator;
+}
 // The ExtensionKeybindingRegistry is a class that handles the cross-platform
 // logic for keyboard accelerators. See platform-specific implementations for
 // implementation details for each platform.
@@ -25,6 +30,64 @@
   explicit ExtensionKeybindingRegistry(Profile* profile);
   virtual ~ExtensionKeybindingRegistry();
 
+  // Register prefs for keybinding.
+  static void RegisterUserPrefs(PrefService* user_prefs);
+
+  // Gets the active keybinding (if any) for the browser action of an extension
+  // given its |extension_id|. The function consults the master list (in the
+  // |profile|) to see if the keybinding is active. Returns NULL if the
+  // extension has no browser action or no active keybinding for it.
+  static const Extension::ExtensionKeybinding* GetActiveBrowserActionCommand(

[Aaron Boodman, 2012/04/25 19:14:38]

The names being used here are making this code more confusing that it needs to
be.

What do you think about changing Extension::ExtensionKeybinding to
Extension::Command? That makes more sense since it more represent an abstract
named command than a keybinding.

Also, is Extension::Keybinding::accelerator the assigned accelerator, or only
the suggested accelerator, from the manifest. If the latter, renaming it
suggested or default accelerator would be more clear.

Then the comment would be updated to something like:

Gets the command for |extension_id|'s browser action if it has an active
keybinding. Otherwise, returns NULL.

The one for command map would be:

Gets the commands for |extension_id| that have active keybindings. If the
extension has commands that do not have active keybindings, those commands are
not included in the returned map.

All of these naming cleanup changes can be done in a separate CL.

[Finnur, 2012/04/26 13:12:35]

Yup. Sounds good.

On 2012/04/25 19:14:38, Aaron Boodman wrote:

+      Profile* profile, const std::string& extension_id);
+
+  // Gets the active keybinding (if any) for the page action of an extension
+  // given its |extension_id|. The function consults the master list (in the
+  // |profile|) to see if the keybinding is active. Returns NULL if the
+  // extension has no page action or no active keybinding for it.
+  static const Extension::ExtensionKeybinding* GetActivePageActionCommand(
+      Profile* profile, const std::string& extension_id);
+
+  // Gets the active keybinding (if any) for the named commands of an extension
+  // given its |extension_id|. The function consults the master list (in the
+  // |profile|) to see if the keybinding is active. Returns an empty map if the
+  // extension has no named commands or no active keybinding for the commands.
+  static Extension::CommandMap GetActiveNamedCommands(
+      Profile* profile, const std::string& extension_id);
+
+  // Resolves any keybinding for a given |extension|'s page action, browser
+  // action and named commands. In each case, if the suggested keybinding is
+  // free, it will be taken by this extension. If not, that keybinding request
+  // is ignored. |user_pref| is the PrefService used to record the new
+  // keybinding assignment.
+  static void ResolveKeyBindings(PrefService* user_prefs,

[Aaron Boodman, 2012/04/25 19:14:38]

You are using Keybinding (not KeyBinding) everywhere else.

[Aaron Boodman, 2012/04/25 19:14:38]

AssignInitialKeybindings?

+                                 const Extension* extension);
+
+  // 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|).
+  static bool KeybindingActive(PrefService* user_prefs,

[Aaron Boodman, 2012/04/25 19:14:38]

Verbify: IsKeybindingActive

+                               const ui::Accelerator& accelerator,
+                               std::string extension_id,
+                               std::string command_name);
+
+  // Records a keybinding |accelerator| as active for an extension with id
+  // |extension_id| and command with the name |command_name|. If
+  // |allow_overrides| is false, the keybinding must be free for the change to
+  // be recorded (as determined by the master list in |user_prefs|). If
+  // |allow_overwrites| is true, any previously recorded keybinding for this
+  // |accelerator| will be overwritten. Returns true if the change was
+  // successfully recorded.
+  static bool AddKeybindingPref(PrefService* user_prefs,
+                                const ui::Accelerator& accelerator,
+                                std::string extension_id,
+                                std::string command_name,
+                                bool allow_overrides);
+
+  // Removes all keybindings for a given extension by its |extension_id|.
+  // |user_pref| is the PrefService used to record the keybinding deletion.
+  static void RemoveKeybindingPref(PrefService* user_prefs,
+                                   std::string extension_id);
+
   // Overridden from content::NotificationObserver:
   virtual void Observe(int type,
                        const content::NotificationSource& source,