This adds a webui overlay on the extensions page for showing what Extension keybindings are active.…

chrome/browser/extensions/api/commands/extension_command_service.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_EXTENSIONS_API_COMMANDS_EXTENSION_COMMAND_SERVICE_H_
 #define CHROME_BROWSER_EXTENSIONS_API_COMMANDS_EXTENSION_COMMAND_SERVICE_H_
 #pragma once
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_commands.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 PrefService;
 class Profile;
 
+namespace base {
+  class DictionaryValue;
+}
+
 namespace ui {
   class Accelerator;
 }
 
 // This service keeps track of preferences related to extension commands
 // (assigning initial keybindings on install and removing them on deletion
 // and answers questions related to which commands are active.
 class ExtensionCommandService : public ProfileKeyedService,

[Aaron Boodman, 2012/05/19 05:33:26]

I may have already mentioned this, and it's a low priority, but:

extensions::CommandService - then you rename the file to be shorter, and whoever
owns this can call it just command_service.

[Finnur, 2012/05/21 21:28:43]

How about a separate changelist, so as to not mess up the review diffs?

On 2012/05/19 05:33:26, Aaron Boodman wrote:

                                 public content::NotificationObserver {
  public:
+  // An enum specifying whether to fetch all extension commands or only active
+  // ones.
+  enum ExtensionCommandQueryType {

[Aaron Boodman, 2012/05/19 05:33:26]

Seems like QueryType is sufficient.

[Finnur, 2012/05/21 21:28:43]

Done.

+    ALL,
+    ACTIVE_ONLY,
+  };
+
   // Register prefs for keybinding.
   static void RegisterUserPrefs(PrefService* user_prefs);
 
   // Constructs an ExtensionCommandService object for the given profile.
   explicit ExtensionCommandService(Profile* profile);
   virtual ~ExtensionCommandService();
 
-  // Gets the active keybinding (if any) for the browser action of an extension
-  // given its |extension_id|. The function consults the master list to see if
+  // 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 or no active keybinding for it.
-  const extensions::Command* GetActiveBrowserActionCommand(
-      const std::string& extension_id);
+  // action. Returns NULL if the keybinding is not active and |type| requested
+  // is ACTIVE_ONLY.
+  const extensions::Command* GetBrowserActionCommand(
+      const std::string& extension_id, ExtensionCommandQueryType type);
 
-  // Gets the active 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
-  // or no active keybinding for it.
-  const extensions::Command* GetActivePageActionCommand(
-      const std::string& extension_id);
+  // Gets the keybinding (if any) for the browser action of an extension given

[Aaron Boodman, 2012/05/19 05:33:26]

Did you mean to change this? Also, we're killing page actions. It probably makes
sense to skip supporting them with keybindings. Sorry I didn't realize this
earlier.

[Finnur, 2012/05/21 21:28:43]

Did not mean to change it. I'm hesitant to remove the code on the basis that we
might remove support for page actions at some point in the future. It would be
kind of weird to only support browser actions in the interim.

On 2012/05/19 05:33:26, Aaron Boodman wrote:

+  // 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, ExtensionCommandQueryType type);
 
   // 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 keybinding for the commands.
-  extensions::CommandMap GetActiveNamedCommands(
-      const std::string& extension_id);
+  // named commands or no active named commands when |type| requested is
+  // ACTIVE_ONLY.
+  extensions::CommandMap GetNamedCommands(
+      const std::string& extension_id, ExtensionCommandQueryType type);
+
+  // Fetches all known commands, active and inactive and returns them through
+  // |commands|.
+  void GetAllCommands(base::DictionaryValue* commands);

[Aaron Boodman, 2012/05/19 05:33:26]

Weird to have an interface that deals in Values here. Seems like the UI layer
should just call GetNamedCommands() and GetBrowserActionCommand() directly, and
handle the construction of the DictionaryValue itself.

[Finnur, 2012/05/21 21:28:43]

Done.

 
   // 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,
                           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
@@ skipping 16 matching lines @@
   // Assigns initial 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.
   void AssignInitialKeybindings(const Extension* extension);
 
   // Removes all keybindings for a given extension by its |extension_id|.
   void RemoveKeybindingPrefs(std::string extension_id);
 
+  // Creates a DictionaryValue containing the details of the extension and
+  // its commands. |extension| and |command| are pointers to the objects this
+  // function should describe.
+  base::DictionaryValue* CreateExtensionDetailValue(

[Aaron Boodman, 2012/05/19 05:33:26]

Seems like this should be Command::ToValue().

[Finnur, 2012/05/21 21:28:43]

Done.

+      const Extension* extension,
+      const extensions::Command* command);
+
   // The content notification registrar for listening to extension events.
   content::NotificationRegistrar registrar_;
 
   // A weak pointer to the profile we are associated with. Not owned by us.
   Profile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionCommandService);
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_COMMANDS_EXTENSION_COMMAND_SERVICE_H_