Refactor GlobalShortcutListener.

chrome/browser/extensions/global_shortcut_listener.h

Index: chrome/browser/extensions/global_shortcut_listener.h
diff --git a/chrome/browser/extensions/global_shortcut_listener.h b/chrome/browser/extensions/global_shortcut_listener.h
index c61abe65f0c16b91c722c33a1c9b165583e20af3..f79187440749231c72a06b65234fbf9e8b782ae0 100644
--- a/chrome/browser/extensions/global_shortcut_listener.h
+++ b/chrome/browser/extensions/global_shortcut_listener.h
@@ -18,7 +18,7 @@ class Accelerator;
 namespace extensions {
 
 // Platform-neutral implementation of a class that keeps track of observers and
-// monitors keystrokes. It relays messages to the appropriate observers when a
+// monitors keystrokes. It relays messages to the appropriate observer when a
 // global shortcut has been struck by the user.
 class GlobalShortcutListener {
  public:
@@ -32,28 +32,32 @@ class GlobalShortcutListener {
 
   static GlobalShortcutListener* GetInstance();
 
-  // Implemented by platform-specific implementations of this class.
-  virtual void StartListening() = 0;
-  virtual void StopListening() = 0;
-
-  // Register an observer for when a certain |accelerator| is struck.
-  virtual void RegisterAccelerator(
-      const ui::Accelerator& accelerator, Observer* observer);
+  // Register an observer for when a certain |accelerator| is struck. Return
+  // true if register successfully, false if the specificied |accelerator| has
+  // been registered by callers or other native applications.

[Mark Mentovai, 2013/12/19 15:24:13]

We don’t support recognizing that an accelerator has been registered by another
application on all platforms. The comment should specify that that this is a
per-platform consideration.

[zhchbin, 2013/12/20 09:13:12]

Done.

+  bool RegisterAccelerator(const ui::Accelerator& accelerator,
+                           Observer* observer);
   // Stop listening for the given |accelerator|.
-  virtual void UnregisterAccelerator(
-      const ui::Accelerator& accelerator, Observer* observer);
+  void UnregisterAccelerator(const ui::Accelerator& accelerator,
+                             Observer* observer);
 
  protected:
   GlobalShortcutListener();
 
   // Called by platform specific implementations of this class whenever a key
-  // is struck. Only called for keys that have observers registered.
+  // is struck. Only called for keys that have an observer registered.
   void NotifyKeyPressed(const ui::Accelerator& accelerator);
 
+  // Implemented by platform-specific implementations of this class.

[Mark Mentovai, 2013/12/19 15:24:13]

These four can be private. The base class calls them through their vtables, so
it will have access to them. “protected” only gives the subclasses access to the
base class’ version, but since they’re pure virtual, there is no base class
version and “protected” doesn’t mean anything different than “private.”

[zhchbin, 2013/12/20 09:13:12]

Done.

+  virtual void StartListening() = 0;
+  virtual void StopListening() = 0;
+  virtual bool RegisterAcceleratorImpl(const ui::Accelerator& accelerator) = 0;
+  virtual void UnregisterAcceleratorImpl(
+      const ui::Accelerator& accelerator) = 0;
+
   // The map of accelerators that have been successfully registered as global
-  // shortcuts and their observer lists.
-  typedef ObserverList<Observer> Observers;
-  typedef std::map< ui::Accelerator, Observers* > AcceleratorMap;
+  // shortcuts and their observer.
+  typedef std::map< ui::Accelerator, Observer* > AcceleratorMap;

[Mark Mentovai, 2013/12/19 15:24:13]

No spaces inside the angle brackets. See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Horizo...
under “Templates and Casts.”

[zhchbin, 2013/12/20 09:13:12]

Done.

   AcceleratorMap accelerator_map_;
 
  private: