Consolidate ExtensionToolbarModel::Action and LocationBarController::Action

BUG=403083

Review URL: https://codereview.chromium.org/454053005

Cr-Commit-Position: refs/heads/master@{#289654}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289654 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/active_script_controller.cc

@@ -13,7 +13,6 @@
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_util.h"
-#include "chrome/browser/extensions/location_bar_controller.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_id.h"
@@ -121,11 +120,11 @@ ExtensionAction* ActiveScriptController::GetActionForExtension(
   return action.get();
 }
 
-LocationBarController::Action ActiveScriptController::OnClicked(
+ExtensionAction::ShowAction ActiveScriptController::OnClicked(
     const Extension* extension) {
   DCHECK(ContainsKey(pending_requests_, extension->id()));
   RunPendingForExtension(extension);
-  return LocationBarController::ACTION_NONE;
+  return ExtensionAction::ACTION_NONE;
 }
 
 void ActiveScriptController::OnNavigated() {

chrome/browser/extensions/active_script_controller.h

@@ -57,7 +57,7 @@ class ActiveScriptController : public LocationBarController::ActionProvider,
   // LocationBarControllerProvider implementation.
   virtual ExtensionAction* GetActionForExtension(
       const Extension* extension) OVERRIDE;
-  virtual LocationBarController::Action OnClicked(
+  virtual ExtensionAction::ShowAction OnClicked(
       const Extension* extension) OVERRIDE;
   virtual void OnNavigated() OVERRIDE;
   virtual void OnExtensionUnloaded(const Extension* extension) OVERRIDE;

chrome/browser/extensions/extension_action.h

@@ -38,6 +38,14 @@ class Size;
 // a per-tab value, the global value is used instead.
 class ExtensionAction {
  public:
+  // The action that the UI should take after the ExtensionAction is clicked.
+  enum ShowAction {
+    ACTION_NONE,
+    ACTION_SHOW_POPUP,
+    // We don't need a SHOW_CONTEXT_MENU because that's handled separately in
+    // the UI.
+  };
+
   // Use this ID to indicate the default state for properties that take a tab_id
   // parameter.
   static const int kDefaultTabId;

chrome/browser/extensions/extension_toolbar_model.cc

@@ -11,7 +11,6 @@
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
-#include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_toolbar_model_factory.h"
@@ -122,23 +121,22 @@ void ExtensionToolbarModel::MoveBrowserAction(const Extension* extension,
   UpdatePrefs();
 }
 
-ExtensionToolbarModel::Action ExtensionToolbarModel::ExecuteBrowserAction(
+ExtensionAction::ShowAction ExtensionToolbarModel::ExecuteBrowserAction(
     const Extension* extension,
     Browser* browser,
     GURL* popup_url_out,
     bool should_grant) {
   content::WebContents* web_contents = NULL;
   int tab_id = 0;
-  if (!ExtensionTabUtil::GetDefaultTab(browser, &web_contents, &tab_id)) {
-    return ACTION_NONE;
-  }
+  if (!ExtensionTabUtil::GetDefaultTab(browser, &web_contents, &tab_id))
+    return ExtensionAction::ACTION_NONE;
 
   ExtensionAction* browser_action =
       ExtensionActionManager::Get(profile_)->GetBrowserAction(*extension);
 
   // For browser actions, visibility == enabledness.
   if (!browser_action->GetIsVisible(tab_id))
-    return ACTION_NONE;
+    return ExtensionAction::ACTION_NONE;
 
   if (should_grant) {
     TabHelper::FromWebContents(web_contents)
@@ -149,12 +147,12 @@ ExtensionToolbarModel::Action ExtensionToolbarModel::ExecuteBrowserAction(
   if (browser_action->HasPopup(tab_id)) {
     if (popup_url_out)
       *popup_url_out = browser_action->GetPopupUrl(tab_id);
-    return ACTION_SHOW_POPUP;
+    return ExtensionAction::ACTION_SHOW_POPUP;
   }
 
   ExtensionActionAPI::BrowserActionExecuted(
       browser->profile(), *browser_action, web_contents);
-  return ACTION_NONE;
+  return ExtensionAction::ACTION_NONE;
 }
 
 void ExtensionToolbarModel::SetVisibleIconCount(int count) {

chrome/browser/extensions/extension_toolbar_model.h

@@ -9,6 +9,7 @@
 #include "base/observer_list.h"
 #include "base/prefs/pref_change_registrar.h"
 #include "base/scoped_observer.h"
+#include "chrome/browser/extensions/extension_action.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -32,15 +33,6 @@ class ExtensionToolbarModel : public content::NotificationObserver,
   ExtensionToolbarModel(Profile* profile, ExtensionPrefs* extension_prefs);
   virtual ~ExtensionToolbarModel();
 
-  // The action that should be taken as a result of clicking a browser action.
-  enum Action {
-    ACTION_NONE,
-    ACTION_SHOW_POPUP,
-    // Unlike LocationBarController 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. Also used for signaling view changes such as showing extension popups.
   class Observer {
@@ -90,10 +82,10 @@ class ExtensionToolbarModel : public content::NotificationObserver,
   // the extension should be granted page tab permissions, which is what happens
   // when the user clicks the browser action, but not, for example, when the
   // showPopup API is called.
-  Action ExecuteBrowserAction(const Extension* extension,
-                              Browser* browser,
-                              GURL* popup_url_out,
-                              bool should_grant);
+  ExtensionAction::ShowAction ExecuteBrowserAction(const Extension* extension,
+                                                   Browser* browser,
+                                                   GURL* popup_url_out,
+                                                   bool should_grant);
   // If count == size(), this will set the visible icon count to -1, meaning
   // "show all actions".
   void SetVisibleIconCount(int count);

chrome/browser/extensions/location_bar_controller.cc

@@ -6,7 +6,6 @@
 
 #include "base/logging.h"
 #include "chrome/browser/extensions/active_script_controller.h"
-#include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/page_action_controller.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "content/public/browser/invalidate_type.h"
@@ -52,24 +51,25 @@ std::vector<ExtensionAction*> LocationBarController::GetCurrentActions() {
   return current_actions;
 }
 
-LocationBarController::Action LocationBarController::OnClicked(
+ExtensionAction::ShowAction LocationBarController::OnClicked(
     const ExtensionAction* action) {
   const Extension* extension =
       ExtensionRegistry::Get(web_contents_->GetBrowserContext())
           ->enabled_extensions().GetByID(action->extension_id());
   CHECK(extension) << action->extension_id();
 
-  Action page_action =
+  ExtensionAction::ShowAction page_action =
       page_action_controller_->GetActionForExtension(extension) ?
           page_action_controller_->OnClicked(extension) :
-          ACTION_NONE;
-  Action active_script_action =
+          ExtensionAction::ACTION_NONE;
+  ExtensionAction::ShowAction active_script_action =
       active_script_controller_->GetActionForExtension(extension) ?
           active_script_controller_->OnClicked(extension) :
-          ACTION_NONE;
+          ExtensionAction::ACTION_NONE;
 
   // PageAction response takes priority.
-  return page_action != ACTION_NONE ? page_action : active_script_action;
+  return page_action != ExtensionAction::ACTION_NONE ? page_action :
+                                                       active_script_action;
 }
 
 // static

chrome/browser/extensions/location_bar_controller.h

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_observer.h"
+#include "chrome/browser/extensions/extension_action.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "extensions/browser/extension_registry_observer.h"
 
@@ -17,8 +18,6 @@ namespace content {
 class WebContents;
 }
 
-class ExtensionAction;
-
 namespace extensions {
 
 class ActiveScriptController;
@@ -32,13 +31,6 @@ class PageActionController;
 class LocationBarController : public content::WebContentsObserver,
                               public ExtensionRegistryObserver {
  public:
-  // The action that the UI should take after executing |OnClicked|.
-  enum Action {
-    ACTION_NONE,
-    ACTION_SHOW_POPUP,
-    ACTION_SHOW_CONTEXT_MENU,
-  };
-
   class ActionProvider {
    public:
     // Returns the action for the given extension, or NULL if there isn't one.
@@ -46,7 +38,7 @@ class LocationBarController : public content::WebContentsObserver,
         const Extension* extension) = 0;
 
     // Handles a click on an extension action.
-    virtual LocationBarController::Action OnClicked(
+    virtual ExtensionAction::ShowAction OnClicked(
         const Extension* extension) = 0;
 
     // A notification that the WebContents has navigated in the main frame (and
@@ -69,7 +61,7 @@ class LocationBarController : public content::WebContentsObserver,
 
   // Notifies this that an ExtensionAction has been clicked, and returns the
   // action which should be taken in response (if any).
-  Action OnClicked(const ExtensionAction* action);
+  ExtensionAction::ShowAction OnClicked(const ExtensionAction* action);
 
   // Notifies the window that the actions have changed.
   static void NotifyChange(content::WebContents* web_contents);

chrome/browser/extensions/page_action_controller.cc

@@ -37,7 +37,7 @@ ExtensionAction* PageActionController::GetActionForExtension(
   return ExtensionActionManager::Get(GetProfile())->GetPageAction(*extension);
 }
 
-LocationBarController::Action PageActionController::OnClicked(
+ExtensionAction::ShowAction PageActionController::OnClicked(
     const Extension* extension) {
   ExtensionAction* page_action =
       ExtensionActionManager::Get(GetProfile())->GetPageAction(*extension);
@@ -48,7 +48,7 @@ LocationBarController::Action PageActionController::OnClicked(
       active_tab_permission_granter()->GrantIfRequested(extension);
 
   if (page_action->HasPopup(tab_id))
-    return LocationBarController::ACTION_SHOW_POPUP;
+    return ExtensionAction::ACTION_SHOW_POPUP;
 
   ExtensionActionAPI::PageActionExecuted(
       web_contents_->GetBrowserContext(),
@@ -57,7 +57,7 @@ LocationBarController::Action PageActionController::OnClicked(
       web_contents_->GetLastCommittedURL().spec(),
       1 /* Button indication. We only ever pass left-click. */);
 
-  return LocationBarController::ACTION_NONE;
+  return ExtensionAction::ACTION_NONE;
 }
 
 void PageActionController::OnNavigated() {

chrome/browser/extensions/page_action_controller.h

@@ -25,7 +25,7 @@ class PageActionController : public LocationBarController::ActionProvider {
   // LocationBarController::Provider implementation.
   virtual ExtensionAction* GetActionForExtension(const Extension* extension)
       OVERRIDE;
-  virtual LocationBarController::Action OnClicked(
+  virtual ExtensionAction::ShowAction OnClicked(
       const Extension* extension) OVERRIDE;
   virtual void OnNavigated() OVERRIDE;
 

chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm

@@ -746,9 +746,9 @@ class ExtensionServiceObserverBridge
   GURL popupUrl;
   switch (toolbarModel_->ExecuteBrowserAction(extension, browser_, &popupUrl,
                                               shouldGrant)) {
-    case extensions::ExtensionToolbarModel::ACTION_NONE:
+    case ExtensionAction::ACTION_NONE:
       break;
-    case extensions::ExtensionToolbarModel::ACTION_SHOW_POPUP: {
+    case ExtensionAction::ACTION_SHOW_POPUP: {
       NSPoint arrowPoint = [self popupPointForBrowserAction:extension];
       [ExtensionPopupController showURL:popupUrl
                               inBrowser:browser_

chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm

@@ -107,20 +107,12 @@ bool PageActionDecoration::ActivatePageAction(NSRect frame) {
           location_bar_controller();
 
   switch (controller->OnClicked(page_action_)) {
-    case LocationBarController::ACTION_NONE:
+    case ExtensionAction::ACTION_NONE:
       break;
 
-    case LocationBarController::ACTION_SHOW_POPUP:
+    case ExtensionAction::ACTION_SHOW_POPUP:
       ShowPopup(frame, page_action_->GetPopupUrl(current_tab_id_));
       break;
-
-    case LocationBarController::ACTION_SHOW_CONTEXT_MENU:
-      // We are never passing OnClicked a right-click button, so assume that
-      // we're never going to be asked to show a context menu.
-      // TODO(kalman): if this changes, update this class to pass the real
-      // mouse button through to the LocationBarController.
-      NOTREACHED();
-      break;
   }
 
   return true;

chrome/browser/ui/views/extensions/extension_action_view_controller.cc

@@ -78,7 +78,7 @@ bool ExtensionActionViewController::ExecuteAction(
         extensions::ExtensionToolbarModel::Get(browser_->profile());
     show_popup = toolbar_model->ExecuteBrowserAction(
                      extension_, browser_, &popup_url, grant_tab_permissions) ==
-                 extensions::ExtensionToolbarModel::ACTION_SHOW_POPUP;
+                 ExtensionAction::ACTION_SHOW_POPUP;
   } else {  // PageAction
     content::WebContents* web_contents = delegate_->GetCurrentWebContents();
     if (!web_contents)
@@ -87,19 +87,12 @@ bool ExtensionActionViewController::ExecuteAction(
         extensions::TabHelper::FromWebContents(web_contents)->
             location_bar_controller();
     switch (controller->OnClicked(extension_action_)) {
-      case extensions::LocationBarController::ACTION_NONE:
+      case ExtensionAction::ACTION_NONE:
         break;
-      case extensions::LocationBarController::ACTION_SHOW_POPUP:
+      case ExtensionAction::ACTION_SHOW_POPUP:
         popup_url = extension_action_->GetPopupUrl(GetCurrentTabId());
         show_popup = true;
         break;
-      case extensions::LocationBarController::ACTION_SHOW_CONTEXT_MENU:
-        // We are never passing OnClicked a right-click button, so assume that
-        // we're never going to be asked to show a context menu.
-        // TODO(kalman): if this changes, update this class to pass the real
-        // mouse button through to the LocationBarController.
-        NOTREACHED();
-        break;
     }
   }