Resubmit: Block content scripts from executing until user grants permission

chrome/browser/extensions/active_script_controller_browsertest.cc

 // 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.
 
 #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"
+#include "content/public/test/browser_test_utils.h"
 #include "extensions/common/feature_switch.h"
+#include "extensions/common/switches.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace extensions {
 
 namespace {
 
 const char kAllHostsScheme[] = "*://*/*";
 const char kExplicitHostsScheme[] = "http://127.0.0.1/*";
 const char kBackgroundScript[] =
@@ skipping 25 matching lines @@
 
 enum RequiresConsent {
   REQUIRES_CONSENT,
   DOES_NOT_REQUIRE_CONSENT
 };
 
 }  // namespace
 
 class ActiveScriptControllerBrowserTest : public ExtensionBrowserTest {
  public:
-  ActiveScriptControllerBrowserTest()
-      : feature_override_(FeatureSwitch::scripts_require_action(),
-                          FeatureSwitch::OVERRIDE_ENABLED) {}
+  ActiveScriptControllerBrowserTest() {}
 
+  virtual void SetUpCommandLine(base::CommandLine* command_line) OVERRIDE;
   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);
+  const Extension* CreateExtension(HostType host_type,
+                                   InjectionType injection_type);
 
  private:
-  FeatureSwitch::ScopedOverride feature_override_;
   ScopedVector<TestExtensionDir> test_extension_dirs_;
   std::vector<const Extension*> extensions_;
 };
 
+void ActiveScriptControllerBrowserTest::SetUpCommandLine(
+    base::CommandLine* command_line) {
+  ExtensionBrowserTest::SetUpCommandLine(command_line);
+  // We append the actual switch to the commandline because it needs to be
+  // passed over to the renderer, which a FeatureSwitch::ScopedOverride will
+  // not do.
+  command_line->AppendSwitch(switches::kEnableScriptsRequireAction);
+}
+
 void ActiveScriptControllerBrowserTest::CleanUpOnMainThread() {
   test_extension_dirs_.clear();
 }
 
-const Extension* ActiveScriptControllerBrowserTest::GetOrCreateExtension(
+const Extension* ActiveScriptControllerBrowserTest::CreateExtension(
     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(
@@ skipping 146 matching lines @@
 
   // 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) {
+  if (inject_success_listener_->was_satisfied()) {
     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);
 
@@ skipping 26 matching lines @@
   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) {
+                       ActiveScriptsAreDisplayedAndDelayExecution) {
   base::FilePath active_script_path =
       test_data_dir_.AppendASCII("active_script");
 
   const char* kExtensionNames[] = {
       "inject_scripts_all_hosts",
       "inject_scripts_explicit_hosts",
       "content_scripts_all_hosts",
       "content_scripts_explicit_hosts"
   };
 
   // First, we load up three extensions:
   // - 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],
-          GetOrCreateExtension(ALL_HOSTS, EXECUTE_SCRIPT),
+          CreateExtension(ALL_HOSTS, EXECUTE_SCRIPT),
           browser(),
           REQUIRES_CONSENT,
           EXECUTE_SCRIPT),
       ActiveScriptTester(
           kExtensionNames[1],
-          GetOrCreateExtension(EXPLICIT_HOSTS, EXECUTE_SCRIPT),
+          CreateExtension(EXPLICIT_HOSTS, EXECUTE_SCRIPT),
           browser(),
           DOES_NOT_REQUIRE_CONSENT,
           EXECUTE_SCRIPT),
       ActiveScriptTester(
           kExtensionNames[2],
-          GetOrCreateExtension(ALL_HOSTS, CONTENT_SCRIPT),
+          CreateExtension(ALL_HOSTS, CONTENT_SCRIPT),
           browser(),
           REQUIRES_CONSENT,
           CONTENT_SCRIPT),
       ActiveScriptTester(
           kExtensionNames[3],
-          GetOrCreateExtension(EXPLICIT_HOSTS, CONTENT_SCRIPT),
+          CreateExtension(EXPLICIT_HOSTS, CONTENT_SCRIPT),
           browser(),
           DOES_NOT_REQUIRE_CONSENT,
           CONTENT_SCRIPT),
   };
 
   // Navigate to an URL (which matches the explicit host specified in the
-  // extension content_scripts_explicit_hosts). All three extensions should
+  // extension content_scripts_explicit_hosts). All four extensions should
   // inject the script.
   ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL("/extensions/test_file.html"));
 
   for (size_t i = 0u; i < arraysize(testers); ++i)
     EXPECT_TRUE(testers[i].Verify()) << kExtensionNames[i];
 }
 
+// Test that removing an extension with pending injections a) removes the
+// pending injections for that extension, and b) does not affect pending
+// injections for other extensions.
+IN_PROC_BROWSER_TEST_F(ActiveScriptControllerBrowserTest,
+                       RemoveExtensionWithPendingInjections) {
+  // Load up two extensions, each with content scripts.
+  const Extension* extension1 = CreateExtension(ALL_HOSTS, CONTENT_SCRIPT);
+  ASSERT_TRUE(extension1);
+  const Extension* extension2 = CreateExtension(ALL_HOSTS, CONTENT_SCRIPT);
+  ASSERT_TRUE(extension2);
+
+  ASSERT_NE(extension1->id(), extension2->id());
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents);
+  ActiveScriptController* active_script_controller =
+      ActiveScriptController::GetForWebContents(web_contents);
+  ASSERT_TRUE(active_script_controller);
+
+  ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL("/extensions/test_file.html"));
+
+  // Both extensions should have pending requests.
+  EXPECT_TRUE(active_script_controller->GetActionForExtension(extension1));
+  EXPECT_TRUE(active_script_controller->GetActionForExtension(extension2));
+
+  // Unload one of the extensions.
+  UnloadExtension(extension2->id());
+
+  // This is slight hack to achieve a RunPendingInRenderer() method. Since IPCs
+  // are sent synchronously, the renderer will be notified of the extension
+  // being unloaded before the script is executed, and, since ExecuteScript() is
+  // synchronous, the renderer is guaranteed to be done updating scripts.
+  EXPECT_TRUE(content::ExecuteScript(web_contents, "1 == 1;"));
+
+  // We should have pending requests for extension1, but not the removed
+  // extension2.
+  EXPECT_TRUE(active_script_controller->GetActionForExtension(extension1));
+  EXPECT_FALSE(active_script_controller->GetActionForExtension(extension2));
+
+  // We should still be able to run the request for extension1.
+  ExtensionTestMessageListener inject_success_listener(
+      new ExtensionTestMessageListener(kInjectSucceeded,
+                                       false /* won't reply */));
+  inject_success_listener.set_extension_id(extension1->id());
+  active_script_controller->OnClicked(extension1);
+  inject_success_listener.WaitUntilSatisfied();
+}
+
+// A version of the test with the flag off, in order to test that everything
+// still works as expected.
+class FlagOffActiveScriptControllerBrowserTest
+    : public ActiveScriptControllerBrowserTest {
+ private:
+  // Simply don't append the flag.
+  virtual void SetUpCommandLine(base::CommandLine* command_line) OVERRIDE {
+    ExtensionBrowserTest::SetUpCommandLine(command_line);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(FlagOffActiveScriptControllerBrowserTest,
+                       ScriptsExecuteWhenFlagAbsent) {
+  const char* kExtensionNames[] = {
+    "content_scripts_all_hosts",
+    "inject_scripts_all_hosts",
+  };
+  ActiveScriptTester testers[] = {
+    ActiveScriptTester(
+          kExtensionNames[0],
+          CreateExtension(ALL_HOSTS, CONTENT_SCRIPT),
+          browser(),
+          DOES_NOT_REQUIRE_CONSENT,
+          CONTENT_SCRIPT),
+      ActiveScriptTester(
+          kExtensionNames[1],
+          CreateExtension(ALL_HOSTS, EXECUTE_SCRIPT),
+          browser(),
+          DOES_NOT_REQUIRE_CONSENT,
+          EXECUTE_SCRIPT),
+  };
+
+  ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL("/extensions/test_file.html"));
+
+  for (size_t i = 0u; i < arraysize(testers); ++i)
+    EXPECT_TRUE(testers[i].Verify()) << kExtensionNames[i];
+}
+
 }  // namespace extensions