Pull browser action click logic into ToolbarModelController, so that when I

chrome/browser/extensions/extension_toolbar_model.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_EXTENSION_TOOLBAR_MODEL_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_
 #pragma once
 
 #include "base/compiler_specific.h"
 #include "base/observer_list.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
 class Browser;
 class ExtensionService;
 class PrefService;
 
 // Model for the browser actions toolbar.
 class ExtensionToolbarModel : public content::NotificationObserver {
  public:
   explicit ExtensionToolbarModel(ExtensionService* service);
   virtual ~ExtensionToolbarModel();
 
+  // The action that should be taken as a result of clicking a browser action.
+  enum Action {
+    ACTION_NONE,
+    ACTION_SHOW_POPUP,
+    // Unlike ActionBoxController there is no ACTION_SHOW_CONTEXT_MENU, because
+    // UI implementations tend to handle this themselves at a higher level.
+  };
+
   // A class which is informed of changes to the model; represents the view of
   // MVC.
   class Observer {
    public:
     // An extension with a browser action button has been added, and should go
     // in the toolbar at |index|.
     virtual void BrowserActionAdded(const extensions::Extension* extension,
                                     int index) {}
 
     // The browser action button for |extension| should no longer show.
     virtual void BrowserActionRemoved(const extensions::Extension* extension) {}
 
     // The browser action button for |extension| has been moved to |index|.
     virtual void BrowserActionMoved(const extensions::Extension* extension,
                                     int index) {}
 
-    // The browser action button for |extension_id| (which was not a popup) was
-    // clicked, executing it.
-    virtual void BrowserActionExecuted(const std::string& extension_id,
-                                       Browser* browser) {}
-
     // Called when the model has finished loading.
     virtual void ModelLoaded() {}
 
    protected:
     virtual ~Observer() {}
   };
 
   // Functions called by the view.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
   void MoveBrowserAction(const extensions::Extension* extension, int index);
-  void ExecuteBrowserAction(const std::string& extension_id, Browser* browser);
+  // Executes the browser action for an extension and returns the action that
+  // the UI should perform in response.
+  // |popup_url_out| will be set if the extension should show a popup, with
+  // the URL that should be shown, if non-NULL.
+  Action ExecuteBrowserAction(const extensions::Extension* extension,
+                              Browser* browser,
+                              GURL* popup_url_out);
   // If count == size(), this will set the visible icon count to -1, meaning
   // "show all actions".
   void SetVisibleIconCount(int count);
   // As above, a return value of -1 represents "show all actions".
   int GetVisibleIconCount() { return visible_icon_count_; }

[tfarina, 2012/05/24 18:44:55]

nit: make this function const.

[not at google - send to devlin, 2012/05/25 00:55:40]

Done.

 
   bool extensions_initialized() const { return extensions_initialized_; }
 
   size_t size() const {
     return toolitems_.size();
   }
 
   extensions::ExtensionList::iterator begin() {
     return toolitems_.begin();
   }
@@ skipping 44 matching lines @@
   std::string last_extension_removed_;
 
   // Keeps track of where the last extension to get disabled was in the list.
   size_t last_extension_removed_index_;
 
   // The number of icons visible (the rest should be hidden in the overflow
   // chevron).
   int visible_icon_count_;
 
   content::NotificationRegistrar registrar_;
 };

[tfarina, 2012/05/24 18:44:55]

nit: if we don't want copy and assign semantics, probably worth adding
DISALLOW_COPY_AND_ASSIGN here.

[not at google - send to devlin, 2012/05/25 00:55:40]

Done.

 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_