[Extensions Cleanup] Remove ToolbarActionController::WantsToRun()

ToolbarActionViewController::WantsToRun() is only implemented by
ExtensionActionViewController, and is never used outside of tests.
Remove it, instead using only the PageActionWantsToRun() and
HasBeenBlocked() methods (which are isolated to
ExtensionActionViewController instead of exposed on the base class).

Bug: None

Change-Id: I0d8777785d1a67d1e36f00ee8d3e7bc844c6fdd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1132250
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Caroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760152}

chrome/browser/ui/extensions/blocked_action_bubble_browsertest.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/extension_action_test_helper.h"
+#include "chrome/browser/ui/extensions/extension_action_view_controller.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
@@ -91,7 +92,9 @@ void ExtensionBlockedActionsBubbleTest::ShowUi(const std::string& name) {
   ToolbarActionsBar* const toolbar_actions_bar =
       ToolbarActionsBar::FromBrowserWindow(browser()->window());
   ASSERT_EQ(1u, toolbar_actions_bar->GetActions().size());
-  EXPECT_TRUE(toolbar_actions_bar->GetActions()[0]->WantsToRun(tab));
+  auto* view_controller = static_cast<ExtensionActionViewController*>(
+      toolbar_actions_bar->GetActions()[0]);
+  EXPECT_TRUE(view_controller->HasBeenBlockedForTesting(tab));
 
   ExtensionActionTestHelper::Create(browser())->Press(0);
 

chrome/browser/ui/extensions/extension_action_test_helper.h

@@ -73,10 +73,6 @@ class ExtensionActionTestHelper {
   // Hides the given popup and returns whether the hide was successful.
   virtual bool HidePopup() = 0;
 
-  // Tests that the button at the given |index| is displaying that it wants
-  // to run.
-  virtual bool ActionButtonWantsToRun(size_t index) = 0;
-
   // Sets the current width of the browser actions container without resizing
   // the underlying controller. This is to simulate e.g. when the browser window
   // is too small for the preferred width.

chrome/browser/ui/extensions/extension_action_view_controller.cc

@@ -159,12 +159,6 @@ bool ExtensionActionViewController::IsEnabled(
              PageInteractionStatus::kPending;
 }
 
-bool ExtensionActionViewController::WantsToRun(
-    content::WebContents* web_contents) const {
-  return ExtensionIsValid() &&
-         (PageActionWantsToRun(web_contents) || HasBeenBlocked(web_contents));
-}
-
 bool ExtensionActionViewController::HasPopup(
     content::WebContents* web_contents) const {
   if (!ExtensionIsValid())
@@ -374,6 +368,11 @@ ExtensionActionViewController::GetIconImageSourceForTesting(
   return GetIconImageSource(web_contents, size);
 }
 
+bool ExtensionActionViewController::HasBeenBlockedForTesting(
+    content::WebContents* web_contents) const {
+  return HasBeenBlocked(web_contents);
+}
+
 ExtensionActionViewController*
 ExtensionActionViewController::GetPreferredPopupViewController() {
   return static_cast<ExtensionActionViewController*>(

chrome/browser/ui/extensions/extension_action_view_controller.h

@@ -62,7 +62,6 @@ class ExtensionActionViewController
   PageInteractionStatus GetPageInteractionStatus(
       content::WebContents* web_contents) const override;
   bool IsEnabled(content::WebContents* web_contents) const override;
-  bool WantsToRun(content::WebContents* web_contents) const override;
   bool HasPopup(content::WebContents* web_contents) const override;
   bool IsShowingPopup() const override;
   void HidePopup() override;
@@ -98,6 +97,7 @@ class ExtensionActionViewController
   std::unique_ptr<IconWithBadgeImageSource> GetIconImageSourceForTesting(
       content::WebContents* web_contents,
       const gfx::Size& size);
+  bool HasBeenBlockedForTesting(content::WebContents* web_contents) const;
 
  private:
   // ExtensionActionIconFactory::Observer:

chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc

@@ -286,56 +286,6 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, Visibility) {
   EXPECT_TRUE(toolbar_actions_bar->NeedsOverflow());
 }
 
-// Test that, with the toolbar action redesign, actions that want to run have
-// the proper appearance.
-IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest,
-                       TestUiForActionsWantToRun) {
-  LoadExtensions();
-  EXPECT_EQ(3, browser_actions_bar()->VisibleBrowserActions());
-
-  // Load an extension with a page action.
-  scoped_refptr<const extensions::Extension> page_action_extension =
-      extensions::ExtensionBuilder("page action")
-          .SetAction(extensions::ExtensionBuilder::ActionType::PAGE_ACTION)
-          .SetLocation(extensions::Manifest::INTERNAL)
-          .Build();
-  extension_service()->AddExtension(page_action_extension.get());
-
-  // Verify that the extension was added at the last index.
-  EXPECT_EQ(4, browser_actions_bar()->VisibleBrowserActions());
-  EXPECT_EQ(page_action_extension->id(),
-            browser_actions_bar()->GetExtensionId(3));
-  EXPECT_FALSE(browser_actions_bar()->ActionButtonWantsToRun(3));
-
-  // Make the extension want to run on the current page.
-  ExtensionAction* action = extensions::ExtensionActionManager::Get(profile())->
-      GetExtensionAction(*page_action_extension);
-  ASSERT_TRUE(action);
-  content::WebContents* web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  int tab_id = sessions::SessionTabHelper::IdForTab(web_contents).id();
-  action->SetIsVisible(tab_id, true);
-  extensions::ExtensionActionAPI* extension_action_api =
-      extensions::ExtensionActionAPI::Get(profile());
-  extension_action_api->NotifyChange(action, web_contents, profile());
-  // Verify that the extension's button has the proper UI.
-  EXPECT_TRUE(browser_actions_bar()->ActionButtonWantsToRun(3));
-
-  // Make the extension not want to run, and check that the special UI goes
-  // away.
-  action->SetIsVisible(tab_id, false);
-  extension_action_api->NotifyChange(action, web_contents, profile());
-  EXPECT_FALSE(browser_actions_bar()->ActionButtonWantsToRun(3));
-
-  // Reduce the visible icon count so that the extension is hidden.
-  toolbar_model()->SetVisibleIconCount(3);
-
-  // The extension should want to run whether or not it's hidden.
-  action->SetIsVisible(tab_id, true);
-  extension_action_api->NotifyChange(action, web_contents, profile());
-  EXPECT_TRUE(browser_actions_bar()->ActionButtonWantsToRun(3));
-}
-
 IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest,
                        BrowserActionPopupTest) {
   // Load up two extensions that have browser action popups.
@@ -765,7 +715,13 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarRuntimeHostPermissionsBrowserTest,
   std::vector<ToolbarActionViewController*> actions = actions_bar->GetActions();
   ASSERT_EQ(1u, actions.size());
 
-  EXPECT_TRUE(browser_actions_bar()->ActionButtonWantsToRun(0));
+  auto extension_has_been_blocked = [this, web_contents]() {
+    ToolbarActionsBar* toolbar = browser_actions_bar()->GetToolbarActionsBar();
+    auto* view_controller =
+        static_cast<ExtensionActionViewController*>(toolbar->GetActions()[0]);
+    return view_controller->HasBeenBlockedForTesting(web_contents);
+  };
+  EXPECT_TRUE(extension_has_been_blocked());
 
   {
     // Simulate clicking on the extension icon to allow it to run via a page
@@ -782,7 +738,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarRuntimeHostPermissionsBrowserTest,
   // The extension should have run on page reload, so the button shouldn't
   // indicate the extension wants to run.
   ASSERT_TRUE(injection_listener.WaitUntilSatisfied());
-  EXPECT_FALSE(browser_actions_bar()->ActionButtonWantsToRun(0));
+  EXPECT_FALSE(extension_has_been_blocked());
 }
 
 // Tests page access modifications through the context menu which require a page

chrome/browser/ui/toolbar/test_toolbar_action_view_controller.cc

@@ -54,11 +54,6 @@ bool TestToolbarActionViewController::IsEnabled(
   return is_enabled_;
 }
 
-bool TestToolbarActionViewController::WantsToRun(
-    content::WebContents* web_contents) const {
-  return wants_to_run_;
-}
-
 bool TestToolbarActionViewController::HasPopup(
     content::WebContents* web_contents) const {
   return true;
@@ -128,11 +123,6 @@ void TestToolbarActionViewController::SetEnabled(bool is_enabled) {
   UpdateDelegate();
 }
 
-void TestToolbarActionViewController::SetWantsToRun(bool wants_to_run) {
-  wants_to_run_ = wants_to_run;
-  UpdateDelegate();
-}
-
 void TestToolbarActionViewController::SetDisabledClickOpensMenu(
     bool disabled_click_opens_menu) {
   disabled_click_opens_menu_ = disabled_click_opens_menu;

chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h

@@ -26,7 +26,6 @@ class TestToolbarActionViewController : public ToolbarActionViewController {
   base::string16 GetTooltip(content::WebContents* web_contents)
       const override;
   bool IsEnabled(content::WebContents* web_contents) const override;
-  bool WantsToRun(content::WebContents* web_contents) const override;
   bool HasPopup(content::WebContents* web_contents) const override;
   bool IsShowingPopup() const override;
   void HidePopup() override;
@@ -46,7 +45,6 @@ class TestToolbarActionViewController : public ToolbarActionViewController {
   void SetAccessibleName(const base::string16& name);
   void SetTooltip(const base::string16& tooltip);
   void SetEnabled(bool is_enabled);
-  void SetWantsToRun(bool wants_to_run);
   void SetDisabledClickOpensMenu(bool disabled_click_opens_menu);
 
   int execute_action_count() const { return execute_action_count_; }
@@ -71,9 +69,6 @@ class TestToolbarActionViewController : public ToolbarActionViewController {
   // Whether or not the action is enabled.
   bool is_enabled_ = true;
 
-  // Whether or not the action wants to run.
-  bool wants_to_run_ = false;
-
   // Whether or not a click on a disabled action should open the context menu.
   bool disabled_click_opens_menu_ = false;
 

chrome/browser/ui/toolbar/toolbar_action_view_controller.h

@@ -69,10 +69,6 @@ class ToolbarActionViewController {
   // Returns true if the action should be enabled on the given |web_contents|.
   virtual bool IsEnabled(content::WebContents* web_contents) const = 0;
 
-  // Returns true if the action wants to run, and should be popped out of the
-  // overflow menu on the given |web_contents|.
-  virtual bool WantsToRun(content::WebContents* web_contents) const = 0;
-
   // Returns true if the action has a popup for the given |web_contents|.
   virtual bool HasPopup(content::WebContents* web_contents) const = 0;
 

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

@@ -134,11 +134,6 @@ bool ExtensionActionTestHelperViews::HidePopup() {
   return !HasPopup();
 }
 
-bool ExtensionActionTestHelperViews::ActionButtonWantsToRun(size_t index) {
-  return browser_actions_container_->GetToolbarActionViewAt(index)
-      ->wants_to_run_for_testing();
-}
-
 void ExtensionActionTestHelperViews::SetWidth(int width) {
   browser_actions_container_->SetSize(
       gfx::Size(width, browser_actions_container_->height()));

chrome/browser/ui/views/extensions/extension_action_test_helper_views.h

@@ -31,7 +31,6 @@ class ExtensionActionTestHelperViews : public ExtensionActionTestHelper {
   gfx::NativeView GetPopupNativeView() override;
   bool HasPopup() override;
   bool HidePopup() override;
-  bool ActionButtonWantsToRun(size_t index) override;
   void SetWidth(int width) override;
   ToolbarActionsBar* GetToolbarActionsBar() override;
   ExtensionsContainer* GetExtensionsContainer() override;

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

@@ -150,12 +150,6 @@ bool ExtensionsMenuTestUtil::HidePopup() {
   return !HasPopup();
 }
 
-bool ExtensionsMenuTestUtil::ActionButtonWantsToRun(size_t index) {
-  // TODO(devlin): Investigate if wants-to-run behavior is still necessary.
-  NOTREACHED();
-  return false;
-}
-
 void ExtensionsMenuTestUtil::SetWidth(int width) {
   extensions_container_->SetSize(
       gfx::Size(width, extensions_container_->height()));

chrome/browser/ui/views/extensions/extensions_menu_test_util.h

@@ -36,7 +36,6 @@ class ExtensionsMenuTestUtil : public ExtensionActionTestHelper {
   gfx::NativeView GetPopupNativeView() override;
   bool HasPopup() override;
   bool HidePopup() override;
-  bool ActionButtonWantsToRun(size_t index) override;
   void SetWidth(int width) override;
   ToolbarActionsBar* GetToolbarActionsBar() override;
   ExtensionsContainer* GetExtensionsContainer() override;

chrome/browser/ui/views/toolbar/toolbar_action_view.cc

@@ -166,8 +166,6 @@ void ToolbarActionView::UpdateState() {
     SetState(views::Button::STATE_NORMAL);
   }
 
-  wants_to_run_ = view_controller_->WantsToRun(web_contents);
-
   gfx::ImageSkia icon(
       view_controller_->GetIcon(web_contents, GetPreferredSize())
           .AsImageSkia());

chrome/browser/ui/views/toolbar/toolbar_action_view.h

@@ -88,12 +88,9 @@ class ToolbarActionView : public views::MenuButton,
 
   bool IsMenuRunningForTesting() const;
 
-  bool wants_to_run_for_testing() const { return wants_to_run_; }
-
   ExtensionContextMenuController* context_menu_controller_for_testing() const {
     return context_menu_controller_.get();
   }
-
   static const char kClassName[];
 
  private:
@@ -133,9 +130,6 @@ class ToolbarActionView : public views::MenuButton,
   // doesn't hide on mouse press and immediately reshow on mouse release.
   bool suppress_next_release_ = false;
 
-  // The cached value of whether or not the action wants to run on the current
-  // tab.
-  bool wants_to_run_ = false;
 
   // This controller is responsible for showing the context menu for an
   // extension.

chrome/browser/ui/views/toolbar/toolbar_action_view_unittest.cc

@@ -260,12 +260,6 @@ TEST_F(ToolbarActionViewUnitTest, MAYBE_BasicToolbarActionViewTest) {
     EXPECT_EQ(old_execute_action_count, controller.execute_action_count());
   }
 
-  // Ensure that the button's want-to-run state reflects that of the controller.
-  controller.SetWantsToRun(true);
-  EXPECT_TRUE(view.wants_to_run_for_testing());
-  controller.SetWantsToRun(false);
-  EXPECT_FALSE(view.wants_to_run_for_testing());
-
   // Create an overflow button.
   views::MenuButton overflow_button(base::string16(), nullptr);
   overflow_button.set_owned_by_client();