[Extensions] Refactor extension action execution

Move most of the logic for extension action execution into the
ExtensionActionRunner (instead of the ExtensionActionAPI), which
eliminates some of the back-and-forth and makes a more common place
for execution logic and adds slightly finer-grained control for
what to execute. This also becomes more important with upcoming
changes for webRequest/document_start handling.

BUG=595087

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

Cr-Commit-Position: refs/heads/master@{#381603}

chrome/browser/extensions/active_tab_apitest.cc

@@ -3,9 +3,10 @@
 // found in the LICENSE file.
 
 #include "base/logging.h"
-#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
+#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/result_catcher.h"
@@ -40,8 +41,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_ActiveTab) {
   // Granting to the extension should give it access to page.html.
   {
     ResultCatcher catcher;
-    ExtensionActionAPI::Get(browser()->profile())->ExecuteExtensionAction(
-        extension, browser(), true);
+    ExtensionActionRunner::GetForWebContents(
+        browser()->tab_strip_model()->GetActiveWebContents())
+        ->RunAction(extension, true);
     EXPECT_TRUE(catcher.GetNextResult()) << message_;
   }
 

chrome/browser/extensions/api/extension_action/browser_action_apitest.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_icon_factory.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
+#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ -52,6 +53,12 @@ using content::WebContents;
 namespace extensions {
 namespace {
 
+void ExecuteExtensionAction(Browser* browser, const Extension* extension) {
+  ExtensionActionRunner::GetForWebContents(
+      browser->tab_strip_model()->GetActiveWebContents())
+      ->RunAction(extension, true);
+}
+
 // An ImageSkia source that will do nothing (i.e., have a blank skia). We need
 // this because we need a blank canvas at a certain size, and that can't be done
 // by just using a null ImageSkia.
@@ -147,8 +154,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, Basic) {
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL("/extensions/test_file.txt"));
 
-  ExtensionActionAPI::Get(browser()->profile())->ExecuteExtensionAction(
-      extension, browser(), true);
+  ExecuteExtensionAction(browser(), extension);
 
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
@@ -550,16 +556,12 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, IncognitoSplit) {
             BrowserActionTestUtil(incognito_browser).NumberOfBrowserActions());
 
   // A click in the regular profile should open a tab in the regular profile.
-  ExtensionActionAPI* extension_action_api =
-      ExtensionActionAPI::Get(browser()->profile());
-  extension_action_api->ExecuteExtensionAction(
-      extension, browser(), true);
+  ExecuteExtensionAction(browser(), extension);
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 
   // A click in the incognito profile should open a tab in the
   // incognito profile.
-  extension_action_api->ExecuteExtensionAction(
-      extension, incognito_browser, true);
+  ExecuteExtensionAction(incognito_browser, extension);
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
@@ -582,8 +584,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DISABLED_CloseBackgroundPage) {
       content::NotificationService::AllSources());
 
   // Click the browser action.
-  ExtensionActionAPI::Get(browser()->profile())->ExecuteExtensionAction(
-      extension, browser(), true);
+  ExecuteExtensionAction(browser(), extension);
 
   // It can take a moment for the background page to actually get destroyed
   // so we wait for the notification before checking that it's really gone

chrome/browser/extensions/api/extension_action/extension_action_api.cc

@@ -170,51 +170,6 @@ void ExtensionActionAPI::SetBrowserActionVisibility(
       extension_id, visible));
 }
 
-ExtensionAction::ShowAction ExtensionActionAPI::ExecuteExtensionAction(
-    const Extension* extension,
-    Browser* browser,
-    bool grant_active_tab_permissions) {
-  content::WebContents* web_contents =
-      browser->tab_strip_model()->GetActiveWebContents();
-  if (!web_contents)
-    return ExtensionAction::ACTION_NONE;
-
-  int tab_id = SessionTabHelper::IdForTab(web_contents);
-
-  ExtensionActionRunner* action_runner =
-      ExtensionActionRunner::GetForWebContents(web_contents);
-  bool has_pending_scripts = false;
-  if (action_runner && action_runner->WantsToRun(extension))
-    has_pending_scripts = true;
-
-  // Grant active tab if appropriate.
-  if (grant_active_tab_permissions) {
-    TabHelper::FromWebContents(web_contents)->active_tab_permission_granter()->
-        GrantIfRequested(extension);
-  }
-
-  // If this was a request to run a script, it will have been run once active
-  // tab was granted. Return without executing the action, since we should only
-  // run pending scripts OR the extension action, not both.
-  if (has_pending_scripts)
-    return ExtensionAction::ACTION_NONE;
-
-  ExtensionAction* extension_action =
-      ExtensionActionManager::Get(browser_context_)->GetExtensionAction(
-          *extension);
-
-  // Anything that gets here should have a page or browser action.
-  DCHECK(extension_action);
-  if (!extension_action->GetIsVisible(tab_id))
-    return ExtensionAction::ACTION_NONE;
-
-  if (extension_action->HasPopup(tab_id))
-    return ExtensionAction::ACTION_SHOW_POPUP;
-
-  ExtensionActionExecuted(*extension_action, web_contents);
-  return ExtensionAction::ACTION_NONE;
-}
-
 bool ExtensionActionAPI::ShowExtensionActionPopup(
     const Extension* extension,
     Browser* browser,
@@ -246,22 +201,6 @@ bool ExtensionActionAPI::ShowExtensionActionPopup(
              extension->id(), grant_active_tab_permissions);
 }
 
-bool ExtensionActionAPI::PageActionWantsToRun(
-    const Extension* extension,
-    content::WebContents* web_contents) {
-  ExtensionAction* page_action =
-      ExtensionActionManager::Get(browser_context_)->GetPageAction(*extension);
-  return page_action &&
-         page_action->GetIsVisible(SessionTabHelper::IdForTab(web_contents));
-}
-
-bool ExtensionActionAPI::HasBeenBlocked(const Extension* extension,
-                                        content::WebContents* web_contents) {
-  ExtensionActionRunner* action_runner =
-      ExtensionActionRunner::GetForWebContents(web_contents);
-  return action_runner && action_runner->WantsToRun(extension);
-}
-
 void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action,
                                       content::WebContents* web_contents,
                                       content::BrowserContext* context) {
@@ -274,6 +213,37 @@ void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action,
     NotifyPageActionsChanged(web_contents);
 }
 
+void ExtensionActionAPI::DispatchExtensionActionClicked(
+    const ExtensionAction& extension_action,
+    WebContents* web_contents) {
+  events::HistogramValue histogram_value = events::UNKNOWN;
+  const char* event_name = NULL;
+  switch (extension_action.action_type()) {
+    case ActionInfo::TYPE_BROWSER:
+      histogram_value = events::BROWSER_ACTION_ON_CLICKED;
+      event_name = "browserAction.onClicked";
+      break;
+    case ActionInfo::TYPE_PAGE:
+      histogram_value = events::PAGE_ACTION_ON_CLICKED;
+      event_name = "pageAction.onClicked";
+      break;
+    case ActionInfo::TYPE_SYSTEM_INDICATOR:
+      // The System Indicator handles its own clicks.
+      NOTREACHED();
+      break;
+  }
+
+  if (event_name) {
+    scoped_ptr<base::ListValue> args(new base::ListValue());
+    args->Append(
+        ExtensionTabUtil::CreateTabObject(web_contents)->ToValue().release());
+
+    DispatchEventToExtension(web_contents->GetBrowserContext(),
+                             extension_action.extension_id(), histogram_value,
+                             event_name, std::move(args));
+  }
+}
+
 void ExtensionActionAPI::ClearAllValuesForTab(
     content::WebContents* web_contents) {
   DCHECK(web_contents);
@@ -321,37 +291,6 @@ void ExtensionActionAPI::DispatchEventToExtension(
       ->DispatchEventToExtension(extension_id, std::move(event));
 }
 
-void ExtensionActionAPI::ExtensionActionExecuted(
-    const ExtensionAction& extension_action,
-    WebContents* web_contents) {
-  events::HistogramValue histogram_value = events::UNKNOWN;
-  const char* event_name = NULL;
-  switch (extension_action.action_type()) {
-    case ActionInfo::TYPE_BROWSER:
-      histogram_value = events::BROWSER_ACTION_ON_CLICKED;
-      event_name = "browserAction.onClicked";
-      break;
-    case ActionInfo::TYPE_PAGE:
-      histogram_value = events::PAGE_ACTION_ON_CLICKED;
-      event_name = "pageAction.onClicked";
-      break;
-    case ActionInfo::TYPE_SYSTEM_INDICATOR:
-      // The System Indicator handles its own clicks.
-      NOTREACHED();
-      break;
-  }
-
-  if (event_name) {
-    scoped_ptr<base::ListValue> args(new base::ListValue());
-    args->Append(
-        ExtensionTabUtil::CreateTabObject(web_contents)->ToValue().release());
-
-    DispatchEventToExtension(web_contents->GetBrowserContext(),
-                             extension_action.extension_id(), histogram_value,
-                             event_name, std::move(args));
-  }
-}
-
 void ExtensionActionAPI::NotifyPageActionsChanged(
     content::WebContents* web_contents) {
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);

chrome/browser/extensions/api/extension_action/extension_action_api.h

@@ -78,15 +78,6 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI {
   void SetBrowserActionVisibility(const std::string& extension_id,
                                   bool visible);
 
-  // Executes the action of the given |extension| on the |browser|'s active
-  // web contents. If |grant_tab_permissions| is true, this will also grant
-  // activeTab to the extension (so this should only be done if this is through
-  // a direct user action). Returns the action that should be taken.
-  ExtensionAction::ShowAction ExecuteExtensionAction(
-      const Extension* extension,
-      Browser* browser,
-      bool grant_active_tab_permissions);
-
   // Opens the popup for the given |extension| in the given |browser|'s window.
   // If |grant_active_tab_permissions| is true, this grants the extension
   // activeTab (so this should only be done if this is through a direct user
@@ -95,21 +86,15 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI {
                                 Browser* browser,
                                 bool grant_active_tab_permissions);
 
-  // Returns true if the |extension| has a visible page action on the given
-  // |web_contents|.
-  bool PageActionWantsToRun(const Extension* extension,
-                            content::WebContents* web_contents);
-
-  // Returns true if the |extension| has a blocked script that wants to inject
-  // on the given |web_contents|.
-  bool HasBeenBlocked(const Extension* extension,
-                      content::WebContents* web_contents);
-
   // Notifies that there has been a change in the given |extension_action|.
   void NotifyChange(ExtensionAction* extension_action,
                     content::WebContents* web_contents,
                     content::BrowserContext* browser_context);
 
+  // Dispatches the onClicked event for extension that owns the given action.
+  void DispatchExtensionActionClicked(const ExtensionAction& extension_action,
+                                      content::WebContents* web_contents);
+
   // Clears the values for all ExtensionActions for the tab associated with the
   // given |web_contents| (and signals that page actions changed).
   void ClearAllValuesForTab(content::WebContents* web_contents);
@@ -135,11 +120,6 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI {
                                 const std::string& event_name,
                                 scoped_ptr<base::ListValue> event_args);
 
-  // Called when either a browser or page action is executed. Figures out which
-  // event to send based on what the extension wants.
-  void ExtensionActionExecuted(const ExtensionAction& extension_action,
-                               content::WebContents* web_contents);
-
   // BrowserContextKeyedAPI implementation.
   void Shutdown() override;
   static const char* service_name() { return "ExtensionActionAPI"; }

chrome/browser/extensions/api/extension_action/page_action_apitest.cc

@@ -6,6 +6,7 @@
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_icon_factory.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
+#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_action_test_util.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -60,8 +61,9 @@ IN_PROC_BROWSER_TEST_F(PageActionApiTest, Basic) {
   {
     // Simulate the page action being clicked.
     ResultCatcher catcher;
-    ExtensionActionAPI::Get(browser()->profile())->ExecuteExtensionAction(
-        extension, browser(), true);
+    ExtensionActionRunner::GetForWebContents(
+        browser()->tab_strip_model()->GetActiveWebContents())
+        ->RunAction(extension, true);
     EXPECT_TRUE(catcher.GetNextResult());
   }
 
@@ -103,8 +105,9 @@ IN_PROC_BROWSER_TEST_F(PageActionApiTest, AddPopup) {
   // install a page action popup.
   {
     ResultCatcher catcher;
-    ExtensionActionAPI::Get(browser()->profile())->ExecuteExtensionAction(
-        extension, browser(), true);
+    ExtensionActionRunner::GetForWebContents(
+        browser()->tab_strip_model()->GetActiveWebContents())
+        ->RunAction(extension, true);
     ASSERT_TRUE(catcher.GetNextResult());
   }
 
@@ -229,8 +232,9 @@ IN_PROC_BROWSER_TEST_F(PageActionApiTest, TestTriggerPageAction) {
   {
     // Simulate the page action being clicked.
     ResultCatcher catcher;
-    ExtensionActionAPI::Get(browser()->profile())->ExecuteExtensionAction(
-        extension, browser(), true);
+    ExtensionActionRunner::GetForWebContents(
+        browser()->tab_strip_model()->GetActiveWebContents())
+        ->RunAction(extension, true);
     EXPECT_TRUE(catcher.GetNextResult());
   }
 

chrome/browser/extensions/api/web_request/web_request_apitest.cc

@@ -527,7 +527,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
   // Grant activeTab permission, and perform another XHR. The extension should
   // receive the event.
   EXPECT_EQ(BLOCKED_ACTION_WEB_REQUEST, runner->GetBlockedActions(extension));
-  runner->OnClicked(extension);
+  runner->RunAction(extension, true);
   EXPECT_EQ(BLOCKED_ACTION_NONE, runner->GetBlockedActions(extension));
   PerformXhrInPage(web_contents, kHost, port, kXhrPath);
   EXPECT_EQ(1, GetWebRequestCountFromBackgroundPage(extension, profile()));

chrome/browser/extensions/error_console/error_console_browsertest.cc

@@ -12,9 +12,10 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
-#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
+#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -251,8 +252,9 @@ class ErrorConsoleBrowserTest : public ExtensionBrowserTest {
         break;
       }
       case ACTION_BROWSER_ACTION: {
-        ExtensionActionAPI::Get(profile())->ExecuteExtensionAction(
-            *extension, browser(), true);
+        ExtensionActionRunner::GetForWebContents(
+            browser()->tab_strip_model()->GetActiveWebContents())
+            ->RunAction(*extension, true);
         break;
       }
       case ACTION_NEW_TAB: {

chrome/browser/extensions/extension_action_runner.cc

@@ -68,13 +68,40 @@ ExtensionActionRunner* ExtensionActionRunner::GetForWebContents(
   return tab_helper ? tab_helper->extension_action_runner() : NULL;
 }
 
-void ExtensionActionRunner::OnActiveTabPermissionGranted(
-    const Extension* extension) {
-  if (WantsToRun(extension))
-    OnClicked(extension);
+ExtensionAction::ShowAction ExtensionActionRunner::RunAction(
+    const Extension* extension,
+    bool grant_tab_permissions) {
+  bool has_pending_scripts = WantsToRun(extension);
+  if (grant_tab_permissions) {
+    TabHelper::FromWebContents(web_contents())
+        ->active_tab_permission_granter()
+        ->GrantIfRequested(extension);
+    // If the extension had blocked actions, granting active tab will have
+    // run the extension. Don't execute further since clicking should run
+    // blocked actions *or* the normal extension action, not both.
+    if (has_pending_scripts)
+      return ExtensionAction::ACTION_NONE;
+  }
+
+  ExtensionAction* extension_action =
+      ExtensionActionManager::Get(browser_context_)
+          ->GetExtensionAction(*extension);
+
+  // Anything that gets here should have a page or browser action.
+  DCHECK(extension_action);
+  int tab_id = SessionTabHelper::IdForTab(web_contents());
+  if (!extension_action->GetIsVisible(tab_id))
+    return ExtensionAction::ACTION_NONE;
+
+  if (extension_action->HasPopup(tab_id))
+    return ExtensionAction::ACTION_SHOW_POPUP;
+
+  ExtensionActionAPI::Get(browser_context_)
+      ->DispatchExtensionActionClicked(*extension_action, web_contents());
+  return ExtensionAction::ACTION_NONE;
 }
 
-void ExtensionActionRunner::OnClicked(const Extension* extension) {
+void ExtensionActionRunner::RunBlockedActions(const Extension* extension) {
   DCHECK(ContainsKey(pending_scripts_, extension->id()) ||
          web_request_blocked_.count(extension->id()) != 0);
 
@@ -94,6 +121,12 @@ void ExtensionActionRunner::OnClicked(const Extension* extension) {
   NotifyChange(extension);
 }
 
+void ExtensionActionRunner::OnActiveTabPermissionGranted(
+    const Extension* extension) {
+  if (WantsToRun(extension))
+    RunBlockedActions(extension);
+}
+
 void ExtensionActionRunner::OnWebRequestBlocked(const Extension* extension) {
   web_request_blocked_.insert(extension->id());
 }

chrome/browser/extensions/extension_action_runner.h

@@ -16,6 +16,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/scoped_observer.h"
+#include "chrome/browser/extensions/extension_action.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "extensions/browser/blocked_action_type.h"
 #include "extensions/browser/extension_registry_observer.h"
@@ -31,8 +32,6 @@ namespace IPC {
 class Message;
 }
 
-class ExtensionAction;
-
 namespace extensions {
 class Extension;
 class ExtensionRegistry;
@@ -45,20 +44,28 @@ class ExtensionActionRunner : public content::WebContentsObserver,
   explicit ExtensionActionRunner(content::WebContents* web_contents);
   ~ExtensionActionRunner() override;
 
-  // Returns the ExtensionActionRunner for the given |web_contents|, or NULL
+  // Returns the ExtensionActionRunner for the given |web_contents|, or null
   // if one does not exist.
   static ExtensionActionRunner* GetForWebContents(
       content::WebContents* web_contents);
 
+  // Executes the action for the given |extension| and returns any further
+  // action (like showing a popup) that should be taken. If
+  // |grant_tab_permissions| is true, this will also grant activeTab to the
+  // extension (so this should only be done if this is through a direct user
+  // action).
+  ExtensionAction::ShowAction RunAction(const Extension* extension,
+                                        bool grant_tab_permissions);
+
+  // Runs any actions that were blocked for the given |extension|. As a
+  // requirement, this will grant activeTab permission to the extension.
+  void RunBlockedActions(const Extension* extension);
+
   // Notifies the ExtensionActionRunner that an extension has been granted
   // active tab permissions. This will run any pending injections for that
   // extension.
   void OnActiveTabPermissionGranted(const Extension* extension);
 
-  // Notifies the ExtensionActionRunner that the action for |extension| has
-  // been clicked, running any pending tasks that were previously shelved.
-  void OnClicked(const Extension* extension);
-
   // Called when a webRequest event for the given |extension| was blocked.
   void OnWebRequestBlocked(const Extension* extension);
 

chrome/browser/extensions/extension_action_runner_browsertest.cc

@@ -261,7 +261,7 @@ testing::AssertionResult ActiveScriptTester::Verify() {
   DCHECK(wants_to_run);
 
   // Grant permission by clicking on the extension action.
-  runner->OnClicked(extension_);
+  runner->RunAction(extension_, true);
 
   // Now, the extension should be able to inject the script.
   inject_success_listener_->WaitUntilSatisfied();
@@ -371,7 +371,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionActionRunnerBrowserTest,
       new ExtensionTestMessageListener(kInjectSucceeded,
                                        false /* won't reply */));
   inject_success_listener.set_extension_id(extension1->id());
-  action_runner->OnClicked(extension1);
+  action_runner->RunAction(extension1, true);
   inject_success_listener.WaitUntilSatisfied();
 }
 

chrome/browser/extensions/extension_action_runner_unittest.cc

@@ -199,9 +199,6 @@ TEST_F(ExtensionActionRunnerUnitTest, RequestPermissionAndExecute) {
   ASSERT_EQ(0u, GetExecutionCountForExtension(extension->id()));
   ASSERT_FALSE(runner()->WantsToRun(extension));
 
-  ExtensionActionAPI* extension_action_api = ExtensionActionAPI::Get(profile());
-  ASSERT_FALSE(extension_action_api->HasBeenBlocked(extension, web_contents()));
-
   // Since the extension requests all_hosts, we should require user consent.
   EXPECT_TRUE(RequiresUserConsent(extension));
 
@@ -209,16 +206,14 @@ TEST_F(ExtensionActionRunnerUnitTest, RequestPermissionAndExecute) {
   // executed.
   RequestInjection(extension);
   EXPECT_TRUE(runner()->WantsToRun(extension));
-  EXPECT_TRUE(extension_action_api->HasBeenBlocked(extension, web_contents()));
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
   // Click to accept the extension executing.
-  runner()->OnClicked(extension);
+  runner()->RunAction(extension, true);
 
   // The extension should execute, and the extension shouldn't want to run.
   EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
   EXPECT_FALSE(runner()->WantsToRun(extension));
-  EXPECT_FALSE(extension_action_api->HasBeenBlocked(extension, web_contents()));
 
   // Since we already executed on the given page, we shouldn't need permission
   // for a second time.
@@ -239,7 +234,7 @@ TEST_F(ExtensionActionRunnerUnitTest, RequestPermissionAndExecute) {
 
   // Grant access.
   RequestInjection(extension);
-  runner()->OnClicked(extension);
+  runner()->RunAction(extension, true);
   EXPECT_EQ(2u, GetExecutionCountForExtension(extension->id()));
   EXPECT_FALSE(runner()->WantsToRun(extension));
 
@@ -271,7 +266,7 @@ TEST_F(ExtensionActionRunnerUnitTest, PendingInjectionsRemovedAtNavigation) {
 
   // Request and accept a new injection.
   RequestInjection(extension);
-  runner()->OnClicked(extension);
+  runner()->RunAction(extension, true);
 
   // The extension should only have executed once, even though a grand total
   // of two executions were requested.
@@ -295,7 +290,7 @@ TEST_F(ExtensionActionRunnerUnitTest, MultiplePendingInjection) {
 
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
-  runner()->OnClicked(extension);
+  runner()->RunAction(extension, true);
 
   // All pending injections should have executed.
   EXPECT_EQ(kNumInjections, GetExecutionCountForExtension(extension->id()));
@@ -393,7 +388,7 @@ TEST_F(ExtensionActionRunnerUnitTest, TestAlwaysRun) {
   // Allow the extension to always run on this origin.
   ScriptingPermissionsModifier modifier(profile(), extension);
   modifier.GrantHostPermission(web_contents()->GetLastCommittedURL());
-  runner()->OnClicked(extension);
+  runner()->RunAction(extension, true);
 
   // The extension should execute, and the extension shouldn't want to run.
   EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
@@ -449,7 +444,7 @@ TEST_F(ExtensionActionRunnerUnitTest, TestDifferentScriptRunLocations) {
   EXPECT_EQ(BLOCKED_ACTION_SCRIPT_AT_START | BLOCKED_ACTION_SCRIPT_OTHER,
             runner()->GetBlockedActions(extension));
 
-  runner()->OnClicked(extension);
+  runner()->RunAction(extension, true);
   EXPECT_EQ(BLOCKED_ACTION_NONE, runner()->GetBlockedActions(extension));
 }
 

chrome/browser/extensions/extension_context_menu_model.cc

@@ -455,7 +455,7 @@ void ExtensionContextMenuModel::HandlePageAccessCommand(
     ExtensionActionRunner* runner =
         ExtensionActionRunner::GetForWebContents(web_contents);
     if (runner && runner->WantsToRun(extension))
-      runner->OnClicked(extension);
+      runner->RunBlockedActions(extension);
   }
 }
 

chrome/browser/extensions/location_bar_controller_unittest.cc

@@ -69,6 +69,12 @@ class LocationBarControllerUnitTest : public ChromeRenderViewHostTestHarness {
     return SessionTabHelper::IdForTab(web_contents());
   }
 
+  bool PageActionWantsToRun(const Extension* extension) {
+    ExtensionAction* page_action =
+        ExtensionActionManager::Get(profile())->GetPageAction(*extension);
+    return page_action && page_action->GetIsVisible(tab_id());
+  }
+
   const Extension* AddExtension(bool has_page_actions,
                                 const std::string& name) {
     DictionaryBuilder manifest;
@@ -168,16 +174,12 @@ TEST_F(LocationBarControllerUnitTest, NavigationClearsState) {
   page_action.SetTitle(tab_id(), "Goodbye");
   page_action.SetPopupUrl(tab_id(), extension->GetResourceURL("popup.html"));
 
-  ExtensionActionAPI* extension_action_api =
-      ExtensionActionAPI::Get(profile());
   // By default, extensions shouldn't want to act on a page.
-  EXPECT_FALSE(
-      extension_action_api->PageActionWantsToRun(extension, web_contents()));
+  EXPECT_FALSE(PageActionWantsToRun(extension));
   // Showing the page action should indicate that an extension *does* want to
   // run on the page.
   page_action.SetIsVisible(tab_id(), true);
-  EXPECT_TRUE(
-      extension_action_api->PageActionWantsToRun(extension, web_contents()));
+  EXPECT_TRUE(PageActionWantsToRun(extension));
 
   EXPECT_EQ("Goodbye", page_action.GetTitle(tab_id()));
   EXPECT_EQ(extension->GetResourceURL("popup.html"),
@@ -189,16 +191,14 @@ TEST_F(LocationBarControllerUnitTest, NavigationClearsState) {
   EXPECT_EQ("Goodbye", page_action.GetTitle(tab_id()));
   EXPECT_EQ(extension->GetResourceURL("popup.html"),
             page_action.GetPopupUrl(tab_id()));
-  EXPECT_TRUE(
-      extension_action_api->PageActionWantsToRun(extension, web_contents()));
+  EXPECT_TRUE(PageActionWantsToRun(extension));
 
   // Should discard the settings, and go back to the defaults.
   NavigateAndCommit(GURL("http://www.yahoo.com"));
 
   EXPECT_EQ("Hello", page_action.GetTitle(tab_id()));
   EXPECT_EQ(GURL(), page_action.GetPopupUrl(tab_id()));
-  EXPECT_FALSE(
-      extension_action_api->PageActionWantsToRun(extension, web_contents()));
+  EXPECT_FALSE(PageActionWantsToRun(extension));
 }
 
 }  // namespace

chrome/browser/ui/extensions/extension_action_view_controller.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/extensions/api/commands/command_service.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_runner.h"
 #include "chrome/browser/extensions/extension_view.h"
 #include "chrome/browser/extensions/extension_view_host.h"
 #include "chrome/browser/extensions/extension_view_host_factory.h"
@@ -33,6 +34,7 @@
 
 using extensions::ActionInfo;
 using extensions::CommandService;
+using extensions::ExtensionActionRunner;
 
 ExtensionActionViewController::ExtensionActionViewController(
     const extensions::Extension* extension,
@@ -119,16 +121,13 @@ bool ExtensionActionViewController::IsEnabled(
 
   return extension_action_->GetIsVisible(
              SessionTabHelper::IdForTab(web_contents)) ||
-         extensions::ExtensionActionAPI::Get(browser_->profile())
-             ->HasBeenBlocked(extension(), web_contents);
+         HasBeenBlocked(web_contents);
 }
 
 bool ExtensionActionViewController::WantsToRun(
     content::WebContents* web_contents) const {
-  extensions::ExtensionActionAPI* action_api =
-      extensions::ExtensionActionAPI::Get(browser_->profile());
-  return action_api->PageActionWantsToRun(extension(), web_contents) ||
-         action_api->HasBeenBlocked(extension(), web_contents);
+  return ExtensionIsValid() &&
+         (PageActionWantsToRun(web_contents) || HasBeenBlocked(web_contents));
 }
 
 bool ExtensionActionViewController::HasPopup(
@@ -208,9 +207,13 @@ bool ExtensionActionViewController::ExecuteAction(PopupShowAction show_action,
   if (!ExtensionIsValid())
     return false;
 
-  if (extensions::ExtensionActionAPI::Get(browser_->profile())
-          ->ExecuteExtensionAction(
-              extension_.get(), browser_, grant_tab_permissions) ==
+  content::WebContents* web_contents = view_delegate_->GetCurrentWebContents();
+  ExtensionActionRunner* action_runner =
+      ExtensionActionRunner::GetForWebContents(web_contents);
+  if (!action_runner)
+    return false;
+
+  if (action_runner->RunAction(extension(), grant_tab_permissions) ==
       ExtensionAction::ACTION_SHOW_POPUP) {
     GURL popup_url = extension_action_->GetPopupUrl(
         SessionTabHelper::IdForTab(view_delegate_->GetCurrentWebContents()));
@@ -401,14 +404,27 @@ ExtensionActionViewController::GetIconImageSource(
     bool is_overflow =
         toolbar_actions_bar_ && toolbar_actions_bar_->in_overflow_mode();
 
-    extensions::ExtensionActionAPI* api =
-        extensions::ExtensionActionAPI::Get(browser_->profile());
-    bool has_blocked_actions = api->HasBeenBlocked(extension(), web_contents);
+    bool has_blocked_actions = HasBeenBlocked(web_contents);
     image_source->set_paint_blocked_actions_decoration(has_blocked_actions);
     image_source->set_paint_page_action_decoration(
         !has_blocked_actions && is_overflow &&
-        api->PageActionWantsToRun(extension(), web_contents));
+        PageActionWantsToRun(web_contents));
   }
 
   return image_source;
 }
+
+bool ExtensionActionViewController::PageActionWantsToRun(
+    content::WebContents* web_contents) const {
+  return extension_action_->action_type() ==
+             extensions::ActionInfo::TYPE_PAGE &&
+         extension_action_->GetIsVisible(
+             SessionTabHelper::IdForTab(web_contents));
+}
+
+bool ExtensionActionViewController::HasBeenBlocked(
+    content::WebContents* web_contents) const {
+  ExtensionActionRunner* action_runner =
+      ExtensionActionRunner::GetForWebContents(web_contents);
+  return action_runner && action_runner->WantsToRun(extension());
+}

chrome/browser/ui/extensions/extension_action_view_controller.h

@@ -142,6 +142,14 @@ class ExtensionActionViewController
       content::WebContents* web_contents,
       const gfx::Size& size);
 
+  // Returns true if this extension has a page action and that page action wants
+  // to run on the given |web_contents|.
+  bool PageActionWantsToRun(content::WebContents* web_contents) const;
+
+  // Returns true if this extension has been blocked on the given
+  // |web_contents|.
+  bool HasBeenBlocked(content::WebContents* web_contents) const;
+
   // The extension associated with the action we're displaying.
   scoped_refptr<const extensions::Extension> extension_;
 

chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc

@@ -141,7 +141,7 @@ TEST_P(ToolbarActionsBarRedesignUnitTest, ExtensionActionBlockedActions) {
   EXPECT_FALSE(image_source->paint_page_action_decoration());
   EXPECT_TRUE(image_source->paint_blocked_actions_decoration());
 
-  action_runner->OnClicked(browser_action_ext.get());
+  action_runner->RunBlockedActions(browser_action_ext.get());
   image_source =
       browser_action->GetIconImageSourceForTesting(web_contents, kSize);
   EXPECT_FALSE(image_source->grayscale());
@@ -191,7 +191,7 @@ TEST_P(ToolbarActionsBarRedesignUnitTest, ExtensionActionBlockedActions) {
                            web_contents, false);
   toolbar_model()->SetVisibleIconCount(2u);
 
-  action_runner->OnClicked(page_action_ext.get());
+  action_runner->RunBlockedActions(page_action_ext.get());
   image_source = page_action->GetIconImageSourceForTesting(web_contents, kSize);
   EXPECT_TRUE(image_source->grayscale());
   EXPECT_FALSE(image_source->paint_page_action_decoration());