Support "always allow" for runtime script execution

chrome/browser/extensions/active_script_controller_unittest.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 <map>
 
 #include "base/values.h"
 #include "chrome/browser/extensions/active_script_controller.h"
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ skipping 10 matching lines @@
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/id_util.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/user_script.h"
 #include "extensions/common/value_builder.h"
 
 namespace extensions {
 
 namespace {
 
 const char kAllHostsPermission[] = "*://*/*";

[not at google - send to devlin, 2014/08/13 17:21:22]

hm I'd actually like to check both this pattern and <all_urls> since I don't
trust the <all_urls> logic. we can do that later, but it would involve
parameterising all these tests with the Extension to test against, then using it
twice, once with */* and again with <all_urls>.

but: later.

[gpdavis, 2014/08/13 22:18:36]

Acknowledged.

 
 }  // namespace
 
 // Unittests for the ActiveScriptController mostly test the internal logic
 // of the controller itself (when to allow/deny extension script injection).
 // Testing real injection is allowed/denied as expected (i.e., that the
 // ActiveScriptController correctly interfaces in the system) is done in the
 // ActiveScriptControllerBrowserTests.
 class ActiveScriptControllerUnitTest : public ChromeRenderViewHostTestHarness {
  protected:
@@ skipping 273 matching lines @@
 
   // Turning off the preference should have instant effect.
   util::SetAllowedScriptingOnAllUrls(extension->id(), profile(), false);
   EXPECT_TRUE(RequiresUserConsent(extension));
 
   // And should also persist across navigations and websites.
   NavigateAndCommit(GURL("http://www.bar.com"));
   EXPECT_TRUE(RequiresUserConsent(extension));
 }
 
+// Test that the "always run" extension action context menu item will create

[Devlin, 2014/08/13 16:55:03]

This is separate from the context menu, and shouldn't need to mention it.

[gpdavis, 2014/08/13 22:18:36]

Done.

+// a valid persisted permission even when active permissions are cleared.
+TEST_F(ActiveScriptControllerUnitTest, TestAlwaysRun) {
+  const Extension* extension = AddExtension();
+  ASSERT_TRUE(extension);
+
+  NavigateAndCommit(GURL("https://www.google.com"));
+
+  // Ensure that there aren't any executions pending.
+  ASSERT_EQ(0u, GetExecutionCountForExtension(extension->id()));
+  ASSERT_FALSE(controller()->GetActionForExtension(extension));
+
+  // Since the extension requests all_hosts, we should require user consent.
+  EXPECT_TRUE(RequiresUserConsent(extension));
+
+  // Request an injection. There should be an action visible, but no executions.
+  RequestInjection(extension);
+  EXPECT_TRUE(controller()->GetActionForExtension(extension));
+  EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
+
+  // Simulate clicking "always run" menu item.
+  controller()->AlwaysRunOnVisibleHost(extension);
+
+  // The extension should execute, and the action should go away.
+  EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
+  EXPECT_FALSE(controller()->GetActionForExtension(extension));
+
+  // Since we already executed on the given page, we shouldn't need permission
+  // for a second time.
+  EXPECT_FALSE(RequiresUserConsent(extension));
+
+  // Navigating to another site that hasn't been granted a persisted permission
+  // should necessitate user consent.
+  NavigateAndCommit(GURL("https://www.foo.com"));
+  EXPECT_TRUE(RequiresUserConsent(extension));
+
+  // We shouldn't need user permission upon returning to the original host.
+  NavigateAndCommit(GURL("https://www.google.com"));
+  EXPECT_FALSE(RequiresUserConsent(extension));
+
+  // Reinitializing permissions should clear active permissions, but not
+  // persisted permissions.
+  PermissionsUpdater(profile()).InitializePermissions(extension);

[Devlin, 2014/08/13 16:55:04]

Better: unload and reload the extension.

[gpdavis, 2014/08/13 22:18:36]

We don't seem to have an extension service for this unittest.  Do you want me to
integrate it so I can unload and reload the extension?

[not at google - send to devlin, 2014/08/13 22:59:17]

Yeah, testing this would require introducing a dependency on an
ExtensionService. In theory you can get that from
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/base/c...
with ExtensionSystem::Get(profile())->extension_service()

but this test would need to be more dramatically restructured to do real
installation rather than just adding to the extension registry. Exercising
PermissionsUpdater::InitializePermissions is a little fragile since it looks
like PermissionsUpdater isn't really meant to be used in isolation like that,
but that *is* the interface that we're presented with.

We should also be testing extension re-install/re-enable/reload logic for the
whole of ActiveScriptController, not just this feature.

[gpdavis, 2014/08/13 23:23:06]

So... perhaps a later CL?  Or what would you like to do for this one?

[not at google - send to devlin, 2014/08/13 23:32:47]

Here's a good approximation we can implement without the full ExtensionService
stack:

const Extension* ReloadExtension(const std::string& id) {
  registry->RemoveEnabled(id);
  // need an alternative version of AddExtension which takes an ID
  return AddExtension(id);
}

the advantage of that is it does re-create the Extension object and clear away
its permissions.

[gpdavis, 2014/08/14 00:55:28]

Alright, that works.

+  Reload();
+  EXPECT_FALSE(RequiresUserConsent(extension));
+}
+
 }  // namespace extensions