Block tabs.executeScript() from executing until user grants permission

Prevent extensions with <all_urls> from executing scripts using executeScript()
without user consent if the scripts-require-action switch is on.
Coming up next: Content scripts.

BUG=362353

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

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

chrome/browser/extensions/active_script_controller.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/extensions/active_script_controller.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "chrome/browser/extensions/extension_action.h"
@@ -12,6 +15,7 @@
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
@@ -23,10 +27,25 @@
 
 namespace extensions {
 
+ActiveScriptController::PendingRequest::PendingRequest() :
+    page_id(-1) {
+}
+
+ActiveScriptController::PendingRequest::PendingRequest(
+    const base::Closure& closure,
+    int page_id)
+    : closure(closure),
+      page_id(page_id) {
+}
+
+ActiveScriptController::PendingRequest::~PendingRequest() {
+}
+
 ActiveScriptController::ActiveScriptController(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       enabled_(FeatureSwitch::scripts_require_action()->IsEnabled()) {
+  CHECK(web_contents);
 }
 
 ActiveScriptController::~ActiveScriptController() {
@@ -48,43 +67,51 @@ ActiveScriptController* ActiveScriptController::GetForWebContents(
   return location_bar_controller->active_script_controller();
 }
 
-void ActiveScriptController::NotifyScriptExecuting(
-    const std::string& extension_id, int page_id) {
-  content::NavigationEntry* visible_entry =
-      web_contents()->GetController().GetVisibleEntry();
-  if (!visible_entry ||
-      extensions_executing_scripts_.count(extension_id) ||
-      visible_entry->GetPageID() != page_id) {
-    return;
-  }
+bool ActiveScriptController::RequiresUserConsentForScriptInjection(
+    const Extension* extension) {
+  CHECK(extension);
+  if (!PermissionsData::RequiresActionForScriptExecution(extension))
+    return false;
 
-  const Extension* extension =
-      ExtensionRegistry::Get(web_contents()->GetBrowserContext())
-          ->enabled_extensions().GetByID(extension_id);
-  if (extension &&
-      PermissionsData::RequiresActionForScriptExecution(extension)) {
-    extensions_executing_scripts_.insert(extension_id);
+  // If the feature is not enabled, we automatically allow all extensions to
+  // run scripts.
+  if (!enabled_)
+    permitted_extensions_.insert(extension->id());
+
+  return permitted_extensions_.count(extension->id()) == 0;
+}
+
+void ActiveScriptController::RequestScriptInjection(
+    const Extension* extension,
+    int page_id,
+    const base::Closure& callback) {
+  CHECK(extension);
+  PendingRequestList& list = pending_requests_[extension->id()];
+  list.push_back(PendingRequest(callback, page_id));
+
+  // If this was the first entry, notify the location bar that there's a new
+  // icon.
+  if (list.size() == 1u)
     LocationBarController::NotifyChange(web_contents());
-  }
 }
 
 void ActiveScriptController::OnAdInjectionDetected(
     const std::vector<std::string> ad_injectors) {
   size_t num_preventable_ad_injectors =
       base::STLSetIntersection<std::set<std::string> >(
-          ad_injectors, extensions_executing_scripts_).size();
+          ad_injectors, permitted_extensions_).size();
 
   UMA_HISTOGRAM_COUNTS_100(
       "Extensions.ActiveScriptController.PreventableAdInjectors",
       num_preventable_ad_injectors);
   UMA_HISTOGRAM_COUNTS_100(
-      "Extensions.ActiveScriptController.PreventableAdInjectors",
+      "Extensions.ActiveScriptController.UnpreventableAdInjectors",
       ad_injectors.size() - num_preventable_ad_injectors);
 }
 
 ExtensionAction* ActiveScriptController::GetActionForExtension(
     const Extension* extension) {
-  if (!enabled_ || extensions_executing_scripts_.count(extension->id()) == 0)
+  if (!enabled_ || pending_requests_.count(extension->id()) == 0)
     return NULL;  // No action for this extension.
 
   ActiveScriptMap::iterator existing =
@@ -112,13 +139,70 @@ ExtensionAction* ActiveScriptController::GetActionForExtension(
 
 LocationBarController::Action ActiveScriptController::OnClicked(
     const Extension* extension) {
-  DCHECK(extensions_executing_scripts_.count(extension->id()) > 0);
+  DCHECK(extension);
+  PendingRequestMap::iterator iter =
+      pending_requests_.find(extension->id());
+  DCHECK(iter != pending_requests_.end());
+
+  content::NavigationEntry* visible_entry =
+      web_contents()->GetController().GetVisibleEntry();
+  // Refuse to run if there's no visible entry, because we have no idea of
+  // determining if it's the proper page. This should rarely, if ever, happen.
+  if (!visible_entry)
+    return LocationBarController::ACTION_NONE;
+
+  int page_id = visible_entry->GetPageID();
+
+  // We add this to the list of permitted extensions and erase pending entries
+  // *before* running them to guard against the crazy case where running the
+  // callbacks adds more entries.
+  permitted_extensions_.insert(extension->id());
+  PendingRequestList requests;
+  iter->second.swap(requests);
+  pending_requests_.erase(extension->id());
+
+  // Run all pending injections for the given extension.
+  for (PendingRequestList::iterator request = requests.begin();
+       request != requests.end();
+       ++request) {
+    // Only run if it's on the proper page.
+    if (request->page_id == page_id)
+      request->closure.Run();
+  }
+
+  // Inform the location bar that the action is now gone.
+  LocationBarController::NotifyChange(web_contents());
+
   return LocationBarController::ACTION_NONE;
 }
 
 void ActiveScriptController::OnNavigated() {
   LogUMA();
-  extensions_executing_scripts_.clear();
+  permitted_extensions_.clear();
+  pending_requests_.clear();
+}
+
+void ActiveScriptController::OnNotifyExtensionScriptExecution(
+    const std::string& extension_id,
+    int page_id) {
+  if (!Extension::IdIsValid(extension_id)) {
+    NOTREACHED() << "'" << extension_id << "' is not a valid id.";
+    return;
+  }
+
+  const Extension* extension =
+      ExtensionRegistry::Get(web_contents()->GetBrowserContext())
+          ->enabled_extensions().GetByID(extension_id);
+  // We shouldn't allow extensions which are no longer enabled to run any
+  // scripts. Ignore the request.
+  if (!extension)
+    return;
+
+  // Right now, we allow all content scripts to execute, but notify the
+  // controller of them.
+  // TODO(rdevlin.cronin): Fix this in a future CL.
+  if (RequiresUserConsentForScriptInjection(extension))
+    RequestScriptInjection(extension, page_id, base::Bind(&base::DoNothing));
 }
 
 bool ActiveScriptController::OnMessageReceived(const IPC::Message& message) {
@@ -131,20 +215,21 @@ bool ActiveScriptController::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-void ActiveScriptController::OnNotifyExtensionScriptExecution(
-    const std::string& extension_id,
-    int page_id) {
-  if (!Extension::IdIsValid(extension_id)) {
-    NOTREACHED() << "'" << extension_id << "' is not a valid id.";
-    return;
-  }
-  NotifyScriptExecuting(extension_id, page_id);
-}
-
 void ActiveScriptController::LogUMA() const {
   UMA_HISTOGRAM_COUNTS_100(
       "Extensions.ActiveScriptController.ShownActiveScriptsOnPage",
-      extensions_executing_scripts_.size());
+      pending_requests_.size());
+
+  // We only log the permitted extensions metric if the feature is enabled,
+  // because otherwise the data will be boring (100% allowed).
+  if (enabled_) {
+    UMA_HISTOGRAM_COUNTS_100(
+        "Extensions.ActiveScriptController.PermittedExtensions",
+        permitted_extensions_.size());
+    UMA_HISTOGRAM_COUNTS_100(
+        "Extensions.ActiveScriptController.DeniedExtensions",
+        pending_requests_.size());
+  }
 }
 
 }  // namespace extensions

chrome/browser/extensions/active_script_controller.h

@@ -8,7 +8,9 @@
 #include <map>
 #include <set>
 #include <string>
+#include <vector>
 
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/linked_ptr.h"
 #include "chrome/browser/extensions/location_bar_controller.h"
@@ -42,10 +44,17 @@ class ActiveScriptController : public LocationBarController::ActionProvider,
   static ActiveScriptController* GetForWebContents(
       content::WebContents* web_contents);
 
-  // Notify the ActiveScriptController that an extension is running a script.
-  // TODO(rdevlin.cronin): Soon, this should be ask the user for permission,
-  // rather than simply notifying them.
-  void NotifyScriptExecuting(const std::string& extension_id, int page_id);
+  // Returns true if the extension requesting script injection requires
+  // user consent. If this is true, the caller should then register a request
+  // via RequestScriptInjection().
+  bool RequiresUserConsentForScriptInjection(const Extension* extension);
+
+  // Register a request for a script injection, to be executed by running
+  // |callback|. The only assumption that can be made about when (or if)
+  // |callback| is run is that, if it is run, it will run on the current page.
+  void RequestScriptInjection(const Extension* extension,
+                              int page_id,
+                              const base::Closure& callback);
 
   // Notifies the ActiveScriptController of detected ad injection.
   void OnAdInjectionDetected(const std::vector<std::string> ad_injectors);
@@ -58,13 +67,26 @@ class ActiveScriptController : public LocationBarController::ActionProvider,
   virtual void OnNavigated() OVERRIDE;
 
  private:
-  // content::WebContentsObserver implementation.
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-
-  // Handle the NotifyExtensionScriptExecution message.
+  // A single pending request. This could be a pair, but we'd have way too many
+  // stl typedefs, and "request.closure" is nicer than "request.first".
+  struct PendingRequest {
+    PendingRequest();  // For STL.
+    PendingRequest(const base::Closure& closure, int page_id);
+    ~PendingRequest();
+
+    base::Closure closure;
+    int page_id;
+  };
+  typedef std::vector<PendingRequest> PendingRequestList;
+  typedef std::map<std::string, PendingRequestList> PendingRequestMap;
+
+  // Handles the NotifyExtensionScriptExecution message.
   void OnNotifyExtensionScriptExecution(const std::string& extension_id,
                                         int page_id);
 
+  // content::WebContentsObserver implementation.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
   // Log metrics.
   void LogUMA() const;
 
@@ -73,11 +95,14 @@ class ActiveScriptController : public LocationBarController::ActionProvider,
   // always allowing scripts to run and never displaying actions.
   bool enabled_;
 
-  // The extensions that have called ExecuteScript on the current frame.
-  std::set<std::string> extensions_executing_scripts_;
+  // The map of extension_id:pending_request of all pending requests.
+  PendingRequestMap pending_requests_;
 
-  // The extensions which have injected ads.
-  std::set<std::string> ad_injectors_;
+  // The extensions which have been granted permission to run on the given page.
+  // TODO(rdevlin.cronin): Right now, this just keeps track of extensions that
+  // have been permitted to run on the page via this interface. Instead, it
+  // should incorporate more fully with ActiveTab.
+  std::set<std::string> permitted_extensions_;
 
   // Script badges that have been generated for extensions. This is both those
   // with actions already declared that are copied and normalised, and actions

chrome/browser/extensions/active_script_controller_browsertest.cc

@@ -2,13 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/active_script_controller.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/extensions/location_bar_controller.h"
 #include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -18,113 +21,344 @@
 
 namespace extensions {
 
+namespace {
+
+const char kAllHostsScheme[] = "*://*/*";
+const char kExplicitHostsScheme[] = "http://127.0.0.1/*";
+const char kBackgroundScript[] =
+    "\"background\": {\"scripts\": [\"script.js\"]}";
+const char kBackgroundScriptSource[] =
+    "var listener = function(tabId) {\n"
+    "  chrome.tabs.onUpdated.removeListener(listener);\n"
+    "  chrome.tabs.executeScript(tabId, {\n"
+    "    code: \"chrome.test.sendMessage('inject succeeded');\"\n"
+    "  });"
+    "  chrome.test.sendMessage('inject attempted');\n"
+    "};\n"
+    "chrome.tabs.onUpdated.addListener(listener);";
+const char kContentScriptSource[] =
+    "chrome.test.sendMessage('inject succeeded');";
+
+const char kInjectAttempted[] = "inject attempted";
+const char kInjectSucceeded[] = "inject succeeded";
+
+enum InjectionType {
+  CONTENT_SCRIPT,
+  EXECUTE_SCRIPT
+};
+
+enum HostType {
+  ALL_HOSTS,
+  EXPLICIT_HOSTS
+};
+
+enum RequiresConsent {
+  REQUIRES_CONSENT,
+  DOES_NOT_REQUIRE_CONSENT
+};
+
+}  // namespace
+
 class ActiveScriptControllerBrowserTest : public ExtensionBrowserTest {
  public:
   ActiveScriptControllerBrowserTest()
       : feature_override_(FeatureSwitch::scripts_require_action(),
                           FeatureSwitch::OVERRIDE_ENABLED) {}
+
+  virtual void CleanUpOnMainThread() OVERRIDE;
+
+  // Returns an extension with the given |host_type| and |injection_type|. If
+  // one already exists, the existing extension will be returned. Othewrwise,
+  // one will be created.
+  // This could potentially return NULL if LoadExtension() fails.
+  const Extension* GetOrCreateExtension(HostType host_type,
+                                        InjectionType injection_type);
+
  private:
   FeatureSwitch::ScopedOverride feature_override_;
+  ScopedVector<TestExtensionDir> test_extension_dirs_;
+  std::vector<const Extension*> extensions_;
 };
 
+void ActiveScriptControllerBrowserTest::CleanUpOnMainThread() {
+  test_extension_dirs_.clear();
+}
+
+const Extension* ActiveScriptControllerBrowserTest::GetOrCreateExtension(
+    HostType host_type, InjectionType injection_type) {
+  std::string name =
+      base::StringPrintf(
+          "%s %s",
+          injection_type == CONTENT_SCRIPT ?
+              "content_script" : "execute_script",
+          host_type == ALL_HOSTS ? "all_hosts" : "explicit_hosts");
+
+  for (std::vector<const Extension*>::const_iterator iter = extensions_.begin();
+       iter != extensions_.end();
+       ++iter) {
+    if ((*iter)->name() == name)
+      return *iter;
+  }
+
+  const char* permission_scheme =
+      host_type == ALL_HOSTS ? kAllHostsScheme : kExplicitHostsScheme;
+
+  std::string permissions = base::StringPrintf(
+      "\"permissions\": [\"tabs\", \"%s\"]", permission_scheme);
+
+  std::string scripts;
+  std::string script_source;
+  if (injection_type == CONTENT_SCRIPT) {
+    scripts = base::StringPrintf(
+        "\"content_scripts\": ["
+        " {"
+        "  \"matches\": [\"%s\"],"
+        "  \"js\": [\"script.js\"],"
+        "  \"run_at\": \"document_start\""
+        " }"
+        "]",
+        permission_scheme);
+  } else {
+    scripts = kBackgroundScript;
+  }
+
+  std::string manifest = base::StringPrintf(
+      "{"
+      " \"name\": \"%s\","
+      " \"version\": \"1.0\","
+      " \"manifest_version\": 2,"
+      " %s,"
+      " %s"
+      "}",
+      name.c_str(),
+      permissions.c_str(),
+      scripts.c_str());
+
+  scoped_ptr<TestExtensionDir> dir(new TestExtensionDir);
+  dir->WriteManifest(manifest);
+  dir->WriteFile(FILE_PATH_LITERAL("script.js"),
+                 injection_type == CONTENT_SCRIPT ? kContentScriptSource :
+                                                    kBackgroundScriptSource);
+
+  const Extension* extension = LoadExtension(dir->unpacked_path());
+  if (extension) {
+    test_extension_dirs_.push_back(dir.release());
+    extensions_.push_back(extension);
+  }
+
+  // If extension is NULL here, it will be caught later in the test.
+  return extension;
+}
+
 class ActiveScriptTester {
  public:
   ActiveScriptTester(const std::string& name,
                      const Extension* extension,
-                     bool expect_has_action);
+                     Browser* browser,
+                     RequiresConsent requires_consent,
+                     InjectionType type);
   ~ActiveScriptTester();
 
-  testing::AssertionResult Verify(Browser* browser);
+  testing::AssertionResult Verify();
 
  private:
+  // Returns the location bar controller, or NULL if one does not exist.
+  LocationBarController* GetLocationBarController();
+
+  // Returns the active script controller, or NULL if one does not exist.
+  ActiveScriptController* GetActiveScriptController();
+
+  // Get the ExtensionAction for this extension, or NULL if one does not exist.
+  ExtensionAction* GetAction();
+
   // The name of the extension, and also the message it sends.
   std::string name_;
 
   // The extension associated with this tester.
   const Extension* extension_;
 
-  // Whether or not we expect the extension to have an action for script
-  // injection.
-  bool expect_has_action_;
+  // The browser the tester is running in.
+  Browser* browser_;
+
+  // Whether or not the extension has permission to run the script without
+  // asking the user.
+  RequiresConsent requires_consent_;
+
+  // The type of injection this tester uses.
+  InjectionType type_;
 
   // All of these extensions should inject a script (either through content
   // scripts or through chrome.tabs.executeScript()) that sends a message with
-  // their names (for verification).
-  // Be prepared to wait for each extension to inject the script.
-  linked_ptr<ExtensionTestMessageListener> listener_;
+  // the |kInjectSucceeded| message.
+  linked_ptr<ExtensionTestMessageListener> inject_attempt_listener_;
+
+  // After trying to inject the script, extensions sending the script via
+  // chrome.tabs.executeScript() send a |kInjectAttempted| message.
+  linked_ptr<ExtensionTestMessageListener> inject_success_listener_;
 };
 
 ActiveScriptTester::ActiveScriptTester(const std::string& name,
                                        const Extension* extension,
-                                       bool expect_has_action)
+                                       Browser* browser,
+                                       RequiresConsent requires_consent,
+                                       InjectionType type)
     : name_(name),
       extension_(extension),
-      expect_has_action_(expect_has_action),
-      listener_(
-          new ExtensionTestMessageListener(name, false /* won't reply */)) {
+      browser_(browser),
+      requires_consent_(requires_consent),
+      type_(type),
+      inject_attempt_listener_(
+          new ExtensionTestMessageListener(kInjectAttempted,
+                                           false /* won't reply */)),
+      inject_success_listener_(
+          new ExtensionTestMessageListener(kInjectSucceeded,
+                                           false /* won't reply */)) {
+  inject_attempt_listener_->set_extension_id(extension->id());
+  inject_success_listener_->set_extension_id(extension->id());
 }
 
 ActiveScriptTester::~ActiveScriptTester() {
 }
 
-testing::AssertionResult ActiveScriptTester::Verify(Browser* browser) {
+testing::AssertionResult ActiveScriptTester::Verify() {
   if (!extension_)
     return testing::AssertionFailure() << "Could not load extension: " << name_;
 
-  listener_->WaitUntilSatisfied();
-  content::WebContents* web_contents =
-      browser ? browser->tab_strip_model()->GetActiveWebContents() : NULL;
+  // If it's not a content script, the Extension lets us know when it has
+  // attempted to inject the script.
+  // This means there is a potential for a race condition with content scripts;
+  // however, since they are all injected at document_start, this shouldn't be
+  // a problem. If these tests start failing, though, that might be it.
+  if (type_ == EXECUTE_SCRIPT)
+    inject_attempt_listener_->WaitUntilSatisfied();
 
-  if (!web_contents)
-    return testing::AssertionFailure() << "Could not find web contents.";
+  // Make sure all running tasks are complete.
+  content::RunAllPendingInMessageLoop();
 
-  TabHelper* tab_helper = TabHelper::FromWebContents(web_contents);
-  if (!tab_helper)
-    return testing::AssertionFailure() << "Could not find tab helper.";
+  LocationBarController* location_bar_controller = GetLocationBarController();
+  if (!location_bar_controller) {
+    return testing::AssertionFailure()
+        << "Could not find location bar controller";
+  }
 
   ActiveScriptController* controller =
-      tab_helper->location_bar_controller()->active_script_controller();
+      location_bar_controller->active_script_controller();
   if (!controller)
     return testing::AssertionFailure() << "Could not find controller.";
 
-  bool has_action = controller->GetActionForExtension(extension_) != NULL;
-  if (expect_has_action_ != has_action) {
+  ExtensionAction* action = GetAction();
+  bool has_action = action != NULL;
+
+  // An extension should have an action displayed iff it requires user consent.
+  if ((requires_consent_ == REQUIRES_CONSENT && !has_action) ||
+      (requires_consent_ == DOES_NOT_REQUIRE_CONSENT && has_action)) {
+    return testing::AssertionFailure()
+        << "Improper action status for " << name_ << ": expected "
+        << (requires_consent_ == REQUIRES_CONSENT) << ", found " << has_action;
+  }
+
+  // If the extension has permission, we should be able to simply wait for it
+  // to execute.
+  if (requires_consent_ == DOES_NOT_REQUIRE_CONSENT) {
+    inject_success_listener_->WaitUntilSatisfied();
+    return testing::AssertionSuccess();
+  }
+
+  // Otherwise, we don't have permission, and have to grant it. Ensure the
+  // script has *not* already executed.
+  // Currently, it's okay for content scripts to execute, because we don't
+  // block them.
+  // TODO(rdevlin.cronin): Fix this.
+  if (inject_success_listener_->was_satisfied() && type_ != CONTENT_SCRIPT) {
+    return testing::AssertionFailure() <<
+        name_ << "'s script ran without permission.";
+  }
+
+  // If we reach this point, we should always have an action.
+  DCHECK(action);
+
+  // Grant permission by clicking on the extension action.
+  location_bar_controller->OnClicked(action);
+
+  // Now, the extension should be able to inject the script.
+  inject_success_listener_->WaitUntilSatisfied();
+
+  // The Action should have disappeared.
+  has_action = GetAction() != NULL;
+  if (has_action) {
     return testing::AssertionFailure()
-        << "Improper action status: expected " << expect_has_action_
-        << ", found " << has_action;
+        << "Extension " << name_ << " has lingering action.";
   }
 
   return testing::AssertionSuccess();
 }
 
+LocationBarController* ActiveScriptTester::GetLocationBarController() {
+  content::WebContents* web_contents =
+      browser_ ? browser_->tab_strip_model()->GetActiveWebContents() : NULL;
+
+  if (!web_contents)
+    return NULL;
+
+  TabHelper* tab_helper = TabHelper::FromWebContents(web_contents);
+  return tab_helper ? tab_helper->location_bar_controller() : NULL;
+}
+
+ActiveScriptController* ActiveScriptTester::GetActiveScriptController() {
+  LocationBarController* location_bar_controller = GetLocationBarController();
+  return location_bar_controller ?
+      location_bar_controller->active_script_controller() : NULL;
+}
+
+ExtensionAction* ActiveScriptTester::GetAction() {
+  ActiveScriptController* controller = GetActiveScriptController();
+  return controller ? controller->GetActionForExtension(extension_) : NULL;
+}
+
 IN_PROC_BROWSER_TEST_F(ActiveScriptControllerBrowserTest,
                        ActiveScriptsAreDisplayed) {
   base::FilePath active_script_path =
       test_data_dir_.AppendASCII("active_script");
 
   const char* kExtensionNames[] = {
-      "content_scripts_all_hosts",
       "inject_scripts_all_hosts",
+      "inject_scripts_explicit_hosts",
+      "content_scripts_all_hosts",
       "content_scripts_explicit_hosts"
   };
 
   // First, we load up three extensions:
-  // - An extension with a content script that runs on all hosts,
   // - An extension that injects scripts into all hosts,
+  // - An extension that injects scripts into explicit hosts,
+  // - An extension with a content script that runs on all hosts,
   // - An extension with a content script that runs on explicit hosts.
+  // The extensions that operate on explicit hosts have permission; the ones
+  // that request all hosts require user consent.
   ActiveScriptTester testers[] = {
       ActiveScriptTester(
           kExtensionNames[0],
-          LoadExtension(active_script_path.AppendASCII(kExtensionNames[0])),
-          true /* expect action */),
+          GetOrCreateExtension(ALL_HOSTS, EXECUTE_SCRIPT),
+          browser(),
+          REQUIRES_CONSENT,
+          EXECUTE_SCRIPT),
       ActiveScriptTester(
           kExtensionNames[1],
-          LoadExtension(active_script_path.AppendASCII(kExtensionNames[1])),
-          true /* expect action */),
+          GetOrCreateExtension(EXPLICIT_HOSTS, EXECUTE_SCRIPT),
+          browser(),
+          DOES_NOT_REQUIRE_CONSENT,
+          EXECUTE_SCRIPT),
       ActiveScriptTester(
           kExtensionNames[2],
-          LoadExtension(active_script_path.AppendASCII(kExtensionNames[2])),
-          false /* expect action */)
+          GetOrCreateExtension(ALL_HOSTS, CONTENT_SCRIPT),
+          browser(),
+          REQUIRES_CONSENT,
+          CONTENT_SCRIPT),
+      ActiveScriptTester(
+          kExtensionNames[3],
+          GetOrCreateExtension(EXPLICIT_HOSTS, CONTENT_SCRIPT),
+          browser(),
+          DOES_NOT_REQUIRE_CONSENT,
+          CONTENT_SCRIPT),
   };
 
   // Navigate to an URL (which matches the explicit host specified in the
@@ -135,7 +369,7 @@ IN_PROC_BROWSER_TEST_F(ActiveScriptControllerBrowserTest,
       browser(), embedded_test_server()->GetURL("/extensions/test_file.html"));
 
   for (size_t i = 0u; i < arraysize(testers); ++i)
-    ASSERT_TRUE(testers[i].Verify(browser())) << kExtensionNames[i];
+    EXPECT_TRUE(testers[i].Verify()) << kExtensionNames[i];
 }
 
 }  // namespace extensions

chrome/browser/extensions/script_executor.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/script_executor.h"
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/pickle.h"
@@ -14,6 +15,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension_messages.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
@@ -127,33 +129,64 @@ void ScriptExecutor::ExecuteScript(const std::string& extension_id,
                                    bool user_gesture,
                                    ScriptExecutor::ResultType result_type,
                                    const ExecuteScriptCallback& callback) {
-  ActiveScriptController* active_script_controller =
-      ActiveScriptController::GetForWebContents(web_contents_);
+  // Don't execute if the extension has been unloaded.
+  const Extension* extension =
+      ExtensionRegistry::Get(web_contents_->GetBrowserContext())
+          ->enabled_extensions().GetByID(extension_id);
+  if (!extension)
+    return;
+
+  // Don't execute if there's no visible entry. If this is the case, then our
+  // permissions checking is useless (because we can't evaluate the URL).
+  // TODO(rdevlin.cronin): This might be better somewhere higher up the
+  // callstack, but we know it's caught here.
   content::NavigationEntry* visible_entry =
       web_contents_->GetController().GetVisibleEntry();
-  if (active_script_controller && visible_entry) {
-    // TODO(rdevlin.cronin): Now, this is just a notification. Soon, it should
-    // block until the user gives the OK to execute.
-    active_script_controller->NotifyScriptExecuting(extension_id,
-                                                    visible_entry->GetPageID());
+  if (!visible_entry)
+    return;
+
+  scoped_ptr<ExtensionMsg_ExecuteCode_Params> params(
+      new ExtensionMsg_ExecuteCode_Params());
+  params->request_id = next_request_id_++;
+  params->extension_id = extension_id;
+  params->is_javascript = (script_type == JAVASCRIPT);
+  params->code = code;
+  params->all_frames = (frame_scope == ALL_FRAMES);
+  params->match_about_blank = (about_blank == MATCH_ABOUT_BLANK);
+  params->run_at = static_cast<int>(run_at);
+  params->in_main_world = (world_type == MAIN_WORLD);
+  params->is_web_view = (process_type == WEB_VIEW_PROCESS);
+  params->webview_src = webview_src;
+  params->file_url = file_url;
+  params->wants_result = (result_type == JSON_SERIALIZED_RESULT);
+  params->user_gesture = user_gesture;
+
+  ActiveScriptController* active_script_controller =
+      ActiveScriptController::GetForWebContents(web_contents_);
+  if (active_script_controller &&
+      active_script_controller->RequiresUserConsentForScriptInjection(
+          extension)) {
+    // The base::Unretained(this) is safe, because this and the
+    // ActiveScriptController are both attached to the TabHelper. Thus, if the
+    // ActiveScriptController is still alive to invoke the callback, this is
+    // alive, too.
+    active_script_controller->RequestScriptInjection(
+        extension,
+        visible_entry->GetPageID(),
+        base::Closure(base::Bind(&ScriptExecutor::ExecuteScriptHelper,
+                                 base::Unretained(this),
+                                 base::Passed(params.Pass()),
+                                 callback)));
+  } else {
+    ExecuteScriptHelper(params.Pass(), callback);
   }
-  ExtensionMsg_ExecuteCode_Params params;
-  params.request_id = next_request_id_++;
-  params.extension_id = extension_id;
-  params.is_javascript = (script_type == JAVASCRIPT);
-  params.code = code;
-  params.all_frames = (frame_scope == ALL_FRAMES);
-  params.match_about_blank = (about_blank == MATCH_ABOUT_BLANK);
-  params.run_at = static_cast<int>(run_at);
-  params.in_main_world = (world_type == MAIN_WORLD);
-  params.is_web_view = (process_type == WEB_VIEW_PROCESS);
-  params.webview_src = webview_src;
-  params.file_url = file_url;
-  params.wants_result = (result_type == JSON_SERIALIZED_RESULT);
-  params.user_gesture = user_gesture;
+}
 
+void ScriptExecutor::ExecuteScriptHelper(
+    scoped_ptr<ExtensionMsg_ExecuteCode_Params> params,
+    const ExecuteScriptCallback& callback) {
   // Handler handles IPCs and deletes itself on completion.
-  new Handler(script_observers_, web_contents_, params, callback);
+  new Handler(script_observers_, web_contents_, *params, callback);
 }
 
 }  // namespace extensions

chrome/browser/extensions/script_executor.h

@@ -13,6 +13,7 @@
 #include "extensions/common/user_script.h"
 
 class GURL;
+struct ExtensionMsg_ExecuteCode_Params;
 
 namespace base {
 class ListValue;
@@ -101,6 +102,10 @@ class ScriptExecutor {
                      const ExecuteScriptCallback& callback);
 
  private:
+  // Called upon a request being given to execute the script.
+  void ExecuteScriptHelper(scoped_ptr<ExtensionMsg_ExecuteCode_Params> params,
+                           const ExecuteScriptCallback& callback);
+
   // The next value to use for request_id in ExtensionMsg_ExecuteCode_Params.
   int next_request_id_;
 

chrome/test/data/extensions/active_script/content_scripts_all_hosts/content_script.js

-// Copyright 2014 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.
-
-chrome.test.sendMessage('content_scripts_all_hosts');

chrome/test/data/extensions/active_script/content_scripts_all_hosts/manifest.json

-{
-  "name": "Active Script Test: Content Scripts All Hosts",
-  "description": "An extension with all hosts that has a content script.",
-  "version": "2.0",
-  "content_scripts": [
-    {
-      "matches": ["*://*/*"],
-      "js": ["content_script.js"]
-    }
-  ],
-  "manifest_version": 2
-}

chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/content_script.js

-// Copyright 2014 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.
-
-chrome.test.sendMessage('content_scripts_explicit_hosts');

chrome/test/data/extensions/active_script/content_scripts_explicit_hosts/manifest.json

-{
-  "name": "Active Script Test: Content Scripts Explicit Hosts",
-  "description": "An extension with explicit hosts that has a content script.",
-  "version": "2.0",
-  "content_scripts": [
-    {
-      "matches": ["http://127.0.0.1/*"],
-      "js": ["content_script.js"]
-    }
-  ],
-  "manifest_version": 2
-}

chrome/test/data/extensions/active_script/inject_scripts_all_hosts/background.js

-// Copyright 2014 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.
-
-chrome.tabs.onUpdated.addListener(function(tabId) {
-  chrome.tabs.executeScript(
-      tabId,
-      { code: 'chrome.test.sendMessage("inject_scripts_all_hosts");' });
-});

chrome/test/data/extensions/active_script/inject_scripts_all_hosts/manifest.json

-{
-  "name": "Active Script Test: Inject Scripts All Hosts",
-  "description": "An extension with all hosts that injects scripts.",
-  "version": "1.5",
-  "background": {
-    "scripts": ["background.js"]
-  },
-  "permissions": [
-    "tabs", "*://*/*"
-  ],
-  "manifest_version": 2
-}

tools/metrics/histograms/histograms.xml

@@ -5962,6 +5962,26 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="Extensions.ActiveScriptController.DeniedExtensions"
+    units="Extension Count">
+  <owner>kalman@chromium.org</owner>
+  <owner>rdevlin.cronin@chromium.org</owner>
+  <summary>
+    The number of extensions on a page that wanted to execute a script, required
+    explicit user consent, and were denied permission.
+  </summary>
+</histogram>
+
+<histogram name="Extensions.ActiveScriptController.PermittedExtensions"
+    units="Extension Count">
+  <owner>kalman@chromium.org</owner>
+  <owner>rdevlin.cronin@chromium.org</owner>
+  <summary>
+    The number of extensions on a page that wanted to execute a script, required
+    explicit user consent, and were granted permission.
+  </summary>
+</histogram>
+
 <histogram name="Extensions.ActiveScriptController.PreventableAdInjectors"
     units="Extension Count">
   <owner>kalman@chromium.org</owner>