Skip to content

Commit

Permalink
If an extension does a content injection disable bf cache.
Browse files Browse the repository at this point in the history
This code tracks whether a content injection (insertCSS, contentScript,
executeScript) has occurred for a WebFrame. If so then turn off
BFCache for the frame.

BUG=1192785

Change-Id: I682a9efb247aae358023e3a591368c84d47001ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2787195
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#870582}
  • Loading branch information
dtapuska authored and Chromium LUCI CQ committed Apr 8, 2021
1 parent 9d3df97 commit c787d2a
Show file tree
Hide file tree
Showing 23 changed files with 306 additions and 48 deletions.
137 changes: 137 additions & 0 deletions chrome/browser/extensions/back_forward_cache_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// Copyright 2021 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 "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test.h"
#include "extensions/common/extension.h"
#include "net/dns/mock_host_resolver.h"

namespace extensions {

class ExtensionBackForwardCacheBrowserTest : public ExtensionBrowserTest {
public:
explicit ExtensionBackForwardCacheBrowserTest(
bool allow_content_scripts = true) {
feature_list_.InitWithFeaturesAndParameters(
{{features::kBackForwardCache,
{{"content_injection_supported",
allow_content_scripts ? "true" : "false"}}}},
{features::kBackForwardCacheMemoryControls});
}

void SetUpOnMainThread() override {
host_resolver()->AddRule("*", "127.0.0.1");
ExtensionBrowserTest::SetUpOnMainThread();
}

private:
base::test::ScopedFeatureList feature_list_;
};

class ExtensionBackForwardCacheContentScriptDisabledBrowserTest
: public ExtensionBackForwardCacheBrowserTest {
public:
ExtensionBackForwardCacheContentScriptDisabledBrowserTest()
: ExtensionBackForwardCacheBrowserTest(/*allow_content_scripts=*/false) {}
};

IN_PROC_BROWSER_TEST_F(
ExtensionBackForwardCacheContentScriptDisabledBrowserTest,
ScriptDisallowed) {
ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("back_forward_cache")
.AppendASCII("content_script")));

ASSERT_TRUE(embedded_test_server()->Start());
GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));

// 1) Navigate to A.
content::RenderFrameHost* rfh_a =
ui_test_utils::NavigateToURL(browser(), url_a);
content::RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);

// 2) Navigate to B.
content::RenderFrameHost* rfh_b =
ui_test_utils::NavigateToURL(browser(), url_b);
content::RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);

// Expect that |rfh_a| is destroyed as it wouldn't be placed in the cache.
EXPECT_TRUE(delete_observer_rfh_a.deleted());
}

IN_PROC_BROWSER_TEST_F(ExtensionBackForwardCacheBrowserTest, ScriptAllowed) {
ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("back_forward_cache")
.AppendASCII("content_script")));

ASSERT_TRUE(embedded_test_server()->Start());
GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));

// 1) Navigate to A.
content::RenderFrameHost* rfh_a =
ui_test_utils::NavigateToURL(browser(), url_a);
content::RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);

// 2) Navigate to B.
content::RenderFrameHost* rfh_b =
ui_test_utils::NavigateToURL(browser(), url_b);
content::RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);

// Ensure that |rfh_a| is in the cache.
EXPECT_FALSE(delete_observer_rfh_a.deleted());
EXPECT_NE(rfh_a, rfh_b);
EXPECT_TRUE(rfh_a->IsInBackForwardCache());
}

IN_PROC_BROWSER_TEST_F(
ExtensionBackForwardCacheContentScriptDisabledBrowserTest,
CSSDisallowed) {
ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("back_forward_cache")
.AppendASCII("content_css")));

ASSERT_TRUE(embedded_test_server()->Start());
GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));

// 1) Navigate to A.
content::RenderFrameHost* rfh_a =
ui_test_utils::NavigateToURL(browser(), url_a);
content::RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);

// 2) Navigate to B.
content::RenderFrameHost* rfh_b =
ui_test_utils::NavigateToURL(browser(), url_b);
content::RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);

// Expect that |rfh_a| is destroyed as it wouldn't be placed in the cache.
EXPECT_TRUE(delete_observer_rfh_a.deleted());
}

IN_PROC_BROWSER_TEST_F(ExtensionBackForwardCacheBrowserTest, CSSAllowed) {
ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("back_forward_cache")
.AppendASCII("content_css")));

ASSERT_TRUE(embedded_test_server()->Start());
GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));

// 1) Navigate to A.
content::RenderFrameHost* rfh_a =
ui_test_utils::NavigateToURL(browser(), url_a);
content::RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);

// 2) Navigate to B.
content::RenderFrameHost* rfh_b =
ui_test_utils::NavigateToURL(browser(), url_b);
content::RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);

// Ensure that |rfh_a| is in the cache.
EXPECT_FALSE(delete_observer_rfh_a.deleted());
EXPECT_NE(rfh_a, rfh_b);
EXPECT_TRUE(rfh_a->IsInBackForwardCache());
}

} // namespace extensions
3 changes: 2 additions & 1 deletion chrome/renderer/cart/commerce_hint_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,8 @@ void CommerceHintAgent::ExtractProducts() {
new JavaScriptRequest(weak_factory_.GetWeakPtr());
main_frame->RequestExecuteScriptInIsolatedWorld(
ISOLATED_WORLD_ID_CHROME_INTERNAL, &source, 1, false,
blink::WebLocalFrame::kAsynchronous, request);
blink::WebLocalFrame::kAsynchronous, request,
blink::BackForwardCacheAware::kAllow);
}

CommerceHintAgent::JavaScriptRequest::JavaScriptRequest(
Expand Down
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2042,6 +2042,7 @@ if (!is_android) {
"../browser/extensions/app_process_apitest.cc",
"../browser/extensions/app_window_overrides_browsertest.cc",
"../browser/extensions/autoplay_browsertest.cc",
"../browser/extensions/back_forward_cache_browsertest.cc",
"../browser/extensions/background_header_browsertest.cc",
"../browser/extensions/background_page_apitest.cc",
"../browser/extensions/background_scripts_apitest.cc",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Copyright 2021 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.

body {background-color: cyan;}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "no caching",
"version": "0.1",
"manifest_version": 2,
"description": "Checks that injected CSS do prevent back forward cache.",
"permissions": ["http://*/*", "https://*/*"],
"content_scripts": [
{
"matches": ["http://*/*", "https://*/*"],
"css": ["color.css"],
"run_at": "document_start"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Copyright 2021 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.

document.title = "modified";
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "no caching",
"version": "0.1",
"manifest_version": 2,
"description": "Checks that content scripts do prevent back forward cache.",
"permissions": ["http://*/*", "https://*/*"],
"content_scripts": [
{
"matches": ["http://*/*", "https://*/*"],
"js": ["change_page_title.js"],
"run_at": "document_end"
}
]
}
19 changes: 10 additions & 9 deletions components/translate/content/renderer/per_frame_translate_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ void PerFrameTranslateAgent::ExecuteScript(const std::string& script) {
return;

WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
local_frame->ExecuteScriptInIsolatedWorld(world_id_, source);
local_frame->ExecuteScriptInIsolatedWorld(
world_id_, source, blink::BackForwardCacheAware::kAllow);
}

bool PerFrameTranslateAgent::ExecuteScriptAndGetBoolResult(
Expand All @@ -214,8 +215,8 @@ bool PerFrameTranslateAgent::ExecuteScriptAndGetBoolResult(
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_,
source);
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
DCHECK(result->IsBoolean());

return result.As<v8::Boolean>()->Value();
Expand All @@ -232,8 +233,8 @@ std::string PerFrameTranslateAgent::ExecuteScriptAndGetStringResult(
v8::HandleScope handle_scope(isolate);
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_,
source);
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
DCHECK(result->IsString());

v8::Local<v8::String> v8_str = result.As<v8::String>();
Expand All @@ -253,8 +254,8 @@ double PerFrameTranslateAgent::ExecuteScriptAndGetDoubleResult(
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_,
source);
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
DCHECK(result->IsNumber());

return result.As<v8::Number>()->Value();
Expand All @@ -270,8 +271,8 @@ int64_t PerFrameTranslateAgent::ExecuteScriptAndGetIntegerResult(
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_,
source);
local_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
DCHECK(result->IsNumber());

return result.As<v8::Integer>()->Value();
Expand Down
15 changes: 10 additions & 5 deletions components/translate/content/renderer/translate_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ void TranslateAgent::ExecuteScript(const std::string& script) {
return;

WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
main_frame->ExecuteScriptInIsolatedWorld(world_id_, source);
main_frame->ExecuteScriptInIsolatedWorld(
world_id_, source, blink::BackForwardCacheAware::kAllow);
}

bool TranslateAgent::ExecuteScriptAndGetBoolResult(const std::string& script,
Expand All @@ -262,7 +263,8 @@ bool TranslateAgent::ExecuteScriptAndGetBoolResult(const std::string& script,
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_, source);
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
if (result.IsEmpty() || !result->IsBoolean()) {
NOTREACHED();
return fallback;
Expand All @@ -281,7 +283,8 @@ std::string TranslateAgent::ExecuteScriptAndGetStringResult(
v8::HandleScope handle_scope(isolate);
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_, source);
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
if (result.IsEmpty() || !result->IsString()) {
NOTREACHED();
return std::string();
Expand All @@ -306,7 +309,8 @@ double TranslateAgent::ExecuteScriptAndGetDoubleResult(
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_, source);
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
if (result.IsEmpty() || !result->IsNumber()) {
NOTREACHED();
return 0.0;
Expand All @@ -324,7 +328,8 @@ int64_t TranslateAgent::ExecuteScriptAndGetIntegerResult(
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource source = WebScriptSource(WebString::FromASCII(script));
v8::Local<v8::Value> result =
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(world_id_, source);
main_frame->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id_, source, blink::BackForwardCacheAware::kAllow);
if (result.IsEmpty() || !result->IsNumber()) {
NOTREACHED();
return 0;
Expand Down
13 changes: 13 additions & 0 deletions content/browser/renderer_host/back_forward_cache_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ bool IsGeolocationSupported() {
return geolocation_supported.Get();
}

bool IsContentInjectionSupported() {
if (!IsBackForwardCacheEnabled())
return false;
static constexpr base::FeatureParam<bool> content_injection_supported(
&features::kBackForwardCache, "content_injection_supported", false);
return content_injection_supported.Get();
}

bool IsFileSystemSupported() {
if (!IsBackForwardCacheEnabled())
return false;
Expand Down Expand Up @@ -204,6 +212,11 @@ uint64_t GetDisallowedFeatures(RenderFrameHostImpl* rfh,
WebSchedulerTrackedFeature::kRequestedGeolocationPermission);
}

if (!IsContentInjectionSupported()) {
result |= FeatureToBit(WebSchedulerTrackedFeature::kIsolatedWorldScript) |
FeatureToBit(WebSchedulerTrackedFeature::kInjectedStyleSheet);
}

if (!IgnoresOutstandingNetworkRequestForTesting()) {
result |=
FeatureToBit(
Expand Down
6 changes: 4 additions & 2 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,8 @@ void RenderFrameImpl::JavaScriptExecuteRequestForTests(
WebScriptSource(WebString::FromUTF16(javascript)));
} else {
result = frame_->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id, WebScriptSource(WebString::FromUTF16(javascript)));
world_id, WebScriptSource(WebString::FromUTF16(javascript)),
blink::BackForwardCacheAware::kAllow);
}

if (!weak_this)
Expand Down Expand Up @@ -2481,7 +2482,8 @@ void RenderFrameImpl::JavaScriptExecuteRequestInIsolatedWorld(
JavaScriptIsolatedWorldRequest* request = new JavaScriptIsolatedWorldRequest(
weak_factory_.GetWeakPtr(), wants_result, std::move(callback));
frame_->RequestExecuteScriptInIsolatedWorld(
world_id, &script, 1, false, WebLocalFrame::kSynchronous, request);
world_id, &script, 1, false, WebLocalFrame::kSynchronous, request,
blink::BackForwardCacheAware::kAllow);
}

RenderFrameImpl::JavaScriptIsolatedWorldRequest::JavaScriptIsolatedWorldRequest(
Expand Down
7 changes: 4 additions & 3 deletions content/web_test/renderer/test_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1075,8 +1075,8 @@ TestRunnerBindings::EvaluateScriptInIsolatedWorldAndReturnValue(
return {};

blink::WebScriptSource source = blink::WebString::FromUTF8(script);
return GetWebFrame()->ExecuteScriptInIsolatedWorldAndReturnValue(world_id,
source);
return GetWebFrame()->ExecuteScriptInIsolatedWorldAndReturnValue(
world_id, source, blink::BackForwardCacheAware::kAllow);
}

void TestRunnerBindings::EvaluateScriptInIsolatedWorld(
Expand All @@ -1086,7 +1086,8 @@ void TestRunnerBindings::EvaluateScriptInIsolatedWorld(
return;

blink::WebScriptSource source = blink::WebString::FromUTF8(script);
GetWebFrame()->ExecuteScriptInIsolatedWorld(world_id, source);
GetWebFrame()->ExecuteScriptInIsolatedWorld(
world_id, source, blink::BackForwardCacheAware::kAllow);
}

void TestRunnerBindings::SetIsolatedWorldInfo(
Expand Down
8 changes: 5 additions & 3 deletions extensions/renderer/script_injection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,8 @@ void ScriptInjection::InjectJs(std::set<std::string>* executing_scripts,

render_frame_->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
world_id, &sources.front(), sources.size(), is_user_gesture,
execution_option, callback.release());
execution_option, callback.release(),
blink::BackForwardCacheAware::kPossiblyDisallow);
}

void ScriptInjection::OnJsInjectionCompleted(
Expand Down Expand Up @@ -436,8 +437,9 @@ void ScriptInjection::InjectOrRemoveCss(
} else {
DCHECK(adding_css);
for (const blink::WebString& css : css_sources)
web_frame->GetDocument().InsertStyleSheet(css, &style_sheet_key,
blink_css_origin);
web_frame->GetDocument().InsertStyleSheet(
css, &style_sheet_key, blink_css_origin,
blink::BackForwardCacheAware::kPossiblyDisallow);
}
}

Expand Down
Loading

0 comments on commit c787d2a

Please sign in to comment.