Commit 4b8d1c64 authored by kalman@chromium.org's avatar kalman@chromium.org

Don't clear the activeTab permission for same-origin navigations when the

--scripts-require-action flag is on. The flag requirement is because we haven't
confirmed this is safe, but we need something like it for testing the flag.

BUG=404243
R=rdevlin.cronin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#290080}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290080 0039d316-1c4b-4281-b951-d872f2087c98
parent 1f8447df
......@@ -203,9 +203,17 @@ TEST_F(ActiveScriptControllerUnitTest, RequestPermissionAndExecute) {
// for a second time.
EXPECT_FALSE(RequiresUserConsent(extension));
// Reloading should clear those permissions, and we should again require user
// consent.
// Reloading and same-origin navigations shouldn't clear those permissions,
// and we shouldn't require user constent again.
Reload();
EXPECT_FALSE(RequiresUserConsent(extension));
NavigateAndCommit(GURL("https://www.google.com/foo"));
EXPECT_FALSE(RequiresUserConsent(extension));
NavigateAndCommit(GURL("https://www.google.com/bar"));
EXPECT_FALSE(RequiresUserConsent(extension));
// Cross-origin navigations should clear permissions.
NavigateAndCommit(GURL("https://otherdomain.google.com"));
EXPECT_TRUE(RequiresUserConsent(extension));
// Grant access.
......@@ -289,9 +297,21 @@ TEST_F(ActiveScriptControllerUnitTest, ActiveScriptsUseActiveTabPermissions) {
// anymore.
EXPECT_FALSE(RequiresUserConsent(extension));
// Also test that granting active tab runs any pending tasks.
// Reloading and other same-origin navigations maintain the permission to
// execute.
Reload();
// Navigating should mean we need permission again.
EXPECT_FALSE(RequiresUserConsent(extension));
NavigateAndCommit(GURL("https://www.google.com/foo"));
EXPECT_FALSE(RequiresUserConsent(extension));
NavigateAndCommit(GURL("https://www.google.com/bar"));
EXPECT_FALSE(RequiresUserConsent(extension));
// Navigating to a different origin will require user consent again.
NavigateAndCommit(GURL("https://yahoo.com"));
EXPECT_TRUE(RequiresUserConsent(extension));
// Back to the original origin should also re-require constent.
NavigateAndCommit(GURL("https://www.google.com"));
EXPECT_TRUE(RequiresUserConsent(extension));
RequestInjection(extension);
......
......@@ -11,6 +11,7 @@
#include "content/public/browser/web_contents.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/extension_messages.h"
#include "extensions/common/feature_switch.h"
#include "extensions/common/permissions/permission_set.h"
#include "extensions/common/permissions/permissions_data.h"
#include "extensions/common/user_script.h"
......@@ -86,7 +87,25 @@ void ActiveTabPermissionGranter::DidNavigateMainFrame(
if (details.is_in_page)
return;
DCHECK(details.is_main_frame); // important: sub-frames don't get granted!
ClearActiveExtensionsAndNotify();
// Only clear the granted permissions for cross-origin navigations.
//
// See http://crbug.com/404243 for why. Currently we only differentiate
// between same-origin and cross-origin navigations when the
// script-require-action flag is on. It's not clear it's good for general
// activeTab consumption (we likely need to build some UI around it first).
// However, the scripts-require-action feature is all-but unusable without
// this behaviour.
if (FeatureSwitch::scripts_require_action()->IsEnabled()) {
const content::NavigationEntry* navigation_entry =
web_contents()->GetController().GetVisibleEntry();
if (!navigation_entry || (navigation_entry->GetURL().GetOrigin() !=
details.previous_url.GetOrigin())) {
ClearActiveExtensionsAndNotify();
}
} else {
ClearActiveExtensionsAndNotify();
}
}
void ActiveTabPermissionGranter::WebContentsDestroyed() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment