chrome: Extract CommandUpdaterDelegate into its own header file.

R=sky@chromium.org


Review URL: https://chromiumcodereview.appspot.com/11308259

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

11 files changed, 87 insertions, 79 deletions

diff --git a/chrome/browser/chromeos/login/simple_web_view_dialog.cc b/chrome/browser/chromeos/login/simple_web_view_dialog.cc
index 607a1c6..57eb874 100644
--- a/chrome/browser/chromeos/login/simple_web_view_dialog.cc
+++ b/chrome/browser/chromeos/login/simple_web_view_dialog.cc
@@ -10,6 +10,7 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chromeos/login/captive_portal_window_proxy.h"
+#include "chrome/browser/command_updater.h"
 #include "chrome/browser/password_manager/password_manager.h"
 #include "chrome/browser/password_manager/password_manager_delegate_impl.h"
 #include "chrome/browser/profiles/profile.h"
diff --git a/chrome/browser/chromeos/login/simple_web_view_dialog.h b/chrome/browser/chromeos/login/simple_web_view_dialog.h
index 785c5f3..1298eba 100644
--- a/chrome/browser/chromeos/login/simple_web_view_dialog.h
+++ b/chrome/browser/chromeos/login/simple_web_view_dialog.h
@@ -7,7 +7,7 @@
 
 #include <string>
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/command_updater.h"
+#include "chrome/browser/command_updater_delegate.h"
 #include "chrome/browser/ui/toolbar/toolbar_model_delegate.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "content/public/browser/page_navigator.h"
@@ -16,6 +16,7 @@
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/widget/widget_delegate.h"
 
+class CommandUpdater;
 class Profile;
 class ReloadButton;
 class ToolbarModel;
@@ -38,7 +39,7 @@ class SimpleWebViewDialog : public views::ButtonListener,
                             public views::WidgetDelegateView,
                             public LocationBarView::Delegate,
                             public ToolbarModelDelegate,
-                            public CommandUpdater::CommandUpdaterDelegate,
+                            public CommandUpdaterDelegate,
                             public content::PageNavigator,
                             public content::WebContentsDelegate {
  public:
diff --git a/chrome/browser/command_observer.h b/chrome/browser/command_observer.h
index 8c14646..8a81968 100644
--- a/chrome/browser/command_observer.h
+++ b/chrome/browser/command_observer.h
@@ -14,7 +14,7 @@ class CommandObserver {
   virtual void EnabledStateChangedForCommand(int id, bool enabled) = 0;
 
  protected:
-  virtual ~CommandObserver();
+  virtual ~CommandObserver() {}
 };
 
 #endif  // CHROME_BROWSER_COMMAND_OBSERVER_H_
diff --git a/chrome/browser/command_updater.cc b/chrome/browser/command_updater.cc
index a92b8cb..4eb8391 100644
--- a/chrome/browser/command_updater.cc
+++ b/chrome/browser/command_updater.cc
@@ -10,9 +10,7 @@
 #include "base/observer_list.h"
 #include "base/stl_util.h"
 #include "chrome/browser/command_observer.h"
-
-CommandUpdater::CommandUpdaterDelegate::~CommandUpdaterDelegate() {
-}
+#include "chrome/browser/command_updater_delegate.h"
 
 class CommandUpdater::Command {
  public:
@@ -22,14 +20,18 @@ class CommandUpdater::Command {
   Command() : enabled(true) {}
 };
 
-CommandUpdater::CommandUpdater(CommandUpdaterDelegate* handler)
-    : delegate_(handler) {
+CommandUpdater::CommandUpdater(CommandUpdaterDelegate* delegate)
+    : delegate_(delegate) {
 }
 
 CommandUpdater::~CommandUpdater() {
   STLDeleteContainerPairSecondPointers(commands_.begin(), commands_.end());
 }
 
+bool CommandUpdater::SupportsCommand(int id) const {
+  return commands_.find(id) != commands_.end();
+}
+
 bool CommandUpdater::IsCommandEnabled(int id) const {
   const CommandMap::const_iterator command(commands_.find(id));
   if (command == commands_.end())
@@ -37,10 +39,6 @@ bool CommandUpdater::IsCommandEnabled(int id) const {
   return command->second->enabled;
 }
 
-bool CommandUpdater::SupportsCommand(int id) const {
-  return commands_.find(id) != commands_.end();
-}
-
 bool CommandUpdater::ExecuteCommand(int id) {
   return ExecuteCommandWithDisposition(id, CURRENT_TAB);
 }
@@ -55,7 +53,22 @@ bool CommandUpdater::ExecuteCommandWithDisposition(
   return false;
 }
 
-CommandObserver::~CommandObserver() {
+void CommandUpdater::AddCommandObserver(int id, CommandObserver* observer) {
+  GetCommand(id, true)->observers.AddObserver(observer);
+}
+
+void CommandUpdater::RemoveCommandObserver(int id, CommandObserver* observer) {
+  GetCommand(id, false)->observers.RemoveObserver(observer);
+}
+
+void CommandUpdater::RemoveCommandObserver(CommandObserver* observer) {
+  for (CommandMap::const_iterator it = commands_.begin();
+       it != commands_.end();
+       ++it) {
+    Command* command = it->second;
+    if (command)
+      command->observers.RemoveObserver(observer);
+  }
 }
 
 void CommandUpdater::UpdateCommandEnabled(int id, bool enabled) {
@@ -76,21 +89,3 @@ CommandUpdater::Command* CommandUpdater::GetCommand(int id, bool create) {
   commands_[id] = command;
   return command;
 }
-
-void CommandUpdater::AddCommandObserver(int id, CommandObserver* observer) {
-  GetCommand(id, true)->observers.AddObserver(observer);
-}
-
-void CommandUpdater::RemoveCommandObserver(int id, CommandObserver* observer) {
-  GetCommand(id, false)->observers.RemoveObserver(observer);
-}
-
-void CommandUpdater::RemoveCommandObserver(CommandObserver* observer) {
-  for (CommandMap::const_iterator it = commands_.begin();
-       it != commands_.end();
-       ++it) {
-    Command* command = it->second;
-    if (command)
-      command->observers.RemoveObserver(observer);
-  }
-}
diff --git a/chrome/browser/command_updater.h b/chrome/browser/command_updater.h
index 0bbbb36..840cb03 100644
--- a/chrome/browser/command_updater.h
+++ b/chrome/browser/command_updater.h
@@ -10,6 +10,7 @@
 #include "webkit/glue/window_open_disposition.h"
 
 class CommandObserver;
+class CommandUpdaterDelegate;
 
 ////////////////////////////////////////////////////////////////////////////////
 //
@@ -21,24 +22,10 @@ class CommandObserver;
 //
 class CommandUpdater {
  public:
-  // A Delegate object implements this interface so that it can execute commands
-  // when needed.
-  class CommandUpdaterDelegate {
-   public:
-    // Performs the action associated with the command with the specified ID and
-    // using the given disposition.
-    virtual void ExecuteCommandWithDisposition(
-        int id,
-        WindowOpenDisposition disposition) = 0;
-
-   protected:
-    virtual ~CommandUpdaterDelegate();
-  };
-
-  // Create a CommandUpdater with a CommandUpdaterDelegate to handle execution
-  // of specific commands.
-  explicit CommandUpdater(CommandUpdaterDelegate* handler);
-  virtual ~CommandUpdater();
+  // Create a CommandUpdater with |delegate| to handle the execution of specific
+  // commands.
+  explicit CommandUpdater(CommandUpdaterDelegate* delegate);
+  ~CommandUpdater();
 
   // Returns true if the specified command ID is supported.
   bool SupportsCommand(int id) const;
@@ -91,7 +78,6 @@ class CommandUpdater {
   typedef base::hash_map<int, Command*> CommandMap;
   CommandMap commands_;
 
-  CommandUpdater();
   DISALLOW_COPY_AND_ASSIGN(CommandUpdater);
 };
 
diff --git a/chrome/browser/command_updater_delegate.h b/chrome/browser/command_updater_delegate.h
new file mode 100644
index 0000000..3c63836
--- /dev/null
+++ b/chrome/browser/command_updater_delegate.h
@@ -0,0 +1,24 @@
+// Copyright 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_COMMAND_UPDATER_DELEGATE_H_
+#define CHROME_BROWSER_COMMAND_UPDATER_DELEGATE_H_
+
+#include "webkit/glue/window_open_disposition.h"
+
+// Implement this interface so that your object can execute commands when
+// needed.
+class CommandUpdaterDelegate {
+ public:
+  // Performs the action associated with the command with the specified ID and
+  // using the given disposition.
+  virtual void ExecuteCommandWithDisposition(
+      int id,
+      WindowOpenDisposition disposition) = 0;
+
+ protected:
+  virtual ~CommandUpdaterDelegate() {}
+};
+
+#endif  // CHROME_BROWSER_COMMAND_UPDATER_DELEGATE_H_
diff --git a/chrome/browser/command_updater_unittest.cc b/chrome/browser/command_updater_unittest.cc
index c72d5f1..fa7f3ab 100644
--- a/chrome/browser/command_updater_unittest.cc
+++ b/chrome/browser/command_updater_unittest.cc
@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/command_updater.h"
+
 #include "base/compiler_specific.h"
 #include "chrome/browser/command_observer.h"
-#include "chrome/browser/command_updater.h"
+#include "chrome/browser/command_updater_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class TestingCommandHandlerMock
-    : public CommandUpdater::CommandUpdaterDelegate {
+class FakeCommandUpdaterDelegate : public CommandUpdaterDelegate {
  public:
   virtual void ExecuteCommandWithDisposition(int id,
                                              WindowOpenDisposition) OVERRIDE {
@@ -16,14 +17,11 @@ class TestingCommandHandlerMock
   }
 };
 
-class CommandUpdaterTest : public testing::Test {
-};
-
-class TestingCommandObserverMock : public CommandObserver {
+class FakeCommandObserver : public CommandObserver {
  public:
-  TestingCommandObserverMock() : enabled_(true) {}
+  FakeCommandObserver() : enabled_(true) {}
 
-  virtual void EnabledStateChangedForCommand(int id, bool enabled) {
+  virtual void EnabledStateChangedForCommand(int id, bool enabled) OVERRIDE {
     enabled_ = enabled;
   }
 
@@ -33,14 +31,14 @@ class TestingCommandObserverMock : public CommandObserver {
   bool enabled_;
 };
 
-TEST_F(CommandUpdaterTest, TestBasicAPI) {
-  TestingCommandHandlerMock handler;
-  CommandUpdater command_updater(&handler);
+TEST(CommandUpdaterTest, TestBasicAPI) {
+  FakeCommandUpdaterDelegate delegate;
+  CommandUpdater command_updater(&delegate);
 
   // Unsupported command
   EXPECT_FALSE(command_updater.SupportsCommand(0));
   EXPECT_FALSE(command_updater.IsCommandEnabled(0));
-  // TestingCommandHandlerMock::ExecuteCommand should not be called, since
+  // FakeCommandUpdaterDelegate::ExecuteCommand should not be called, since
   // the command is not supported.
   command_updater.ExecuteCommand(0);
 
@@ -54,18 +52,18 @@ TEST_F(CommandUpdaterTest, TestBasicAPI) {
   command_updater.UpdateCommandEnabled(2, false);
   EXPECT_TRUE(command_updater.SupportsCommand(2));
   EXPECT_FALSE(command_updater.IsCommandEnabled(2));
-  // TestingCommandHandlerMock::ExecuteCommmand should not be called, since
+  // FakeCommandUpdaterDelegate::ExecuteCommmand should not be called, since
   // the command_updater is disabled
   command_updater.ExecuteCommand(2);
 }
 
-TEST_F(CommandUpdaterTest, TestObservers) {
-  TestingCommandHandlerMock handler;
-  CommandUpdater command_updater(&handler);
+TEST(CommandUpdaterTest, TestObservers) {
+  FakeCommandUpdaterDelegate delegate;
+  CommandUpdater command_updater(&delegate);
 
   // Create an observer for the command 2 and add it to the controller, then
   // update the command.
-  TestingCommandObserverMock observer;
+  FakeCommandObserver observer;
   command_updater.AddCommandObserver(2, &observer);
   command_updater.UpdateCommandEnabled(2, true);
   EXPECT_TRUE(observer.enabled());
@@ -78,16 +76,16 @@ TEST_F(CommandUpdaterTest, TestObservers) {
   EXPECT_FALSE(observer.enabled());
 }
 
-TEST_F(CommandUpdaterTest, TestObserverRemovingAllCommands) {
-  TestingCommandHandlerMock handler;
-  CommandUpdater command_updater(&handler);
+TEST(CommandUpdaterTest, TestObserverRemovingAllCommands) {
+  FakeCommandUpdaterDelegate delegate;
+  CommandUpdater command_updater(&delegate);
 
   // Create two observers for the commands 1-3 as true, remove one using the
   // single remove command, then set the command to false. Ensure that the
   // removed observer still thinks all commands are true and the one left
   // observing picked up the change.
 
-  TestingCommandObserverMock observer_remove, observer_keep;
+  FakeCommandObserver observer_remove, observer_keep;
   command_updater.AddCommandObserver(1, &observer_remove);
   command_updater.AddCommandObserver(2, &observer_remove);
   command_updater.AddCommandObserver(3, &observer_remove);
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
index 25ad1c7..930e037 100644
--- a/chrome/browser/ui/browser_command_controller.cc
+++ b/chrome/browser/ui/browser_command_controller.cc
@@ -306,11 +306,11 @@ void BrowserCommandController::LoadingStateChanged(bool is_loading,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// BrowserCommandController,
-//     CommandUpdater::CommandUpdaterDelegate implementation:
+// BrowserCommandController, CommandUpdaterDelegate implementation:
 
 void BrowserCommandController::ExecuteCommandWithDisposition(
-  int id, WindowOpenDisposition disposition) {
+    int id,
+    WindowOpenDisposition disposition) {
   // No commands are enabled if there is not yet any selected tab.
   // TODO(pkasting): It seems like we should not need this, because either
   // most/all commands should not have been enabled yet anyway or the ones that
diff --git a/chrome/browser/ui/browser_command_controller.h b/chrome/browser/ui/browser_command_controller.h
index e3f6afa..af6ef91 100644
--- a/chrome/browser/ui/browser_command_controller.h
+++ b/chrome/browser/ui/browser_command_controller.h
@@ -8,6 +8,7 @@
 #include "base/prefs/public/pref_change_registrar.h"
 #include "chrome/browser/api/sync/profile_sync_service_observer.h"
 #include "chrome/browser/command_updater.h"
+#include "chrome/browser/command_updater_delegate.h"
 #include "chrome/browser/sessions/tab_restore_service_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "content/public/browser/notification_observer.h"
@@ -25,7 +26,7 @@ struct NativeWebKeyboardEvent;
 
 namespace chrome {
 
-class BrowserCommandController : public CommandUpdater::CommandUpdaterDelegate,
+class BrowserCommandController : public CommandUpdaterDelegate,
                                  public content::NotificationObserver,
                                  public TabStripModelObserver,
                                  public TabRestoreServiceObserver,
@@ -76,7 +77,7 @@ class BrowserCommandController : public CommandUpdater::CommandUpdaterDelegate,
     FULLSCREEN_METRO_SNAP,
   };
 
-  // Overridden from CommandUpdater::CommandUpdaterDelegate:
+  // Overridden from CommandUpdaterDelegate:
   virtual void ExecuteCommandWithDisposition(
       int id,
       WindowOpenDisposition disposition) OVERRIDE;
diff --git a/chrome/browser/ui/panels/panel.h b/chrome/browser/ui/panels/panel.h
index 6264e97..fc1d7d5 100644
--- a/chrome/browser/ui/panels/panel.h
+++ b/chrome/browser/ui/panels/panel.h
@@ -11,6 +11,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
 #include "chrome/browser/command_updater.h"
+#include "chrome/browser/command_updater_delegate.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/base_window.h"
@@ -47,7 +48,7 @@ class WindowController;
 // - Invoke an appropriate PanelManager function to do stuff that might affect
 //   other Panels. For example deleting a panel would rearrange other panels.
 class Panel : public BaseWindow,
-              public CommandUpdater::CommandUpdaterDelegate,
+              public CommandUpdaterDelegate,
               public content::NotificationObserver,
               public ImageLoadingTracker::Observer {
  public:
@@ -134,7 +135,7 @@ class Panel : public BaseWindow,
   virtual void FlashFrame(bool flash) OVERRIDE;
   virtual bool IsAlwaysOnTop() const OVERRIDE;
 
-  // Overridden from CommandUpdater::CommandUpdaterDelegate:
+  // Overridden from CommandUpdaterDelegate:
   virtual void ExecuteCommandWithDisposition(
       int id,
       WindowOpenDisposition disposition) OVERRIDE;
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 92f9389..e9d33b0 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -450,6 +450,7 @@
         'browser/command_observer.h',
         'browser/command_updater.cc',
         'browser/command_updater.h',
+        'browser/command_updater_delegate.h',
         'browser/common/cancelable_request.cc',
         'browser/common/cancelable_request.h',
         'browser/component_updater/component_updater_configurator.cc',