Skip to content
This repository was archived by the owner on Mar 19, 2021. It is now read-only.

Commit c0b0a0d

Browse files
anforowiczchromium-wpt-export-bot
anforowicz
authored and
chromium-wpt-export-bot
committed
Use correct URLLoaderFactory in an unload handler.
Context and behavior implemented + expected before and after the CL =================================================================== A content::RenderFrameImpl stores |subresource_loader_factories| that are used for all requests/XHRs/etc initiated by the frame. We don't currently swap the RenderFrame when committing another same-process document (this future work is tracked by https://crbug.com/936696). The URLLoaderFactory for handling http/https requests contains a |request_initiator_site_lock| in network::URLLoaderFactoryParams. This lock limits what network::ResourceRequest::request_initiator may be used. The lock is currently used in various, security-sensitive features like CORB, CORP, Sec-Fetch-Site. In the future the lock may be consulted for all requests handled by the NetworkService (see https://crbug.com/920634 and also https://crbug.com/961614). When a cross-origin, same-process navigation commits, then: - The commit IPC carries a bundle of |subresource_loader_factories|. The new |subresource_loader_factories| should be used by the newly committed document. - Before committing the new document, the old document's unload handler needs to run. The unload handler needs to use the old |subresource_loader_factories| (e.g. because otherwise |URLLoaderFactoryParams::request_initiator_site_lock| might not match the document origin). The problematic behavior before this CL ======================================= Before the CL, content::RenderFrameImpl::CommitNavigationWithParams would start using the new |subresource_loader_factories| before running the unload handler of the old document (i.e. before calling blink::WebNavigationControl::CommitNavigation on |frame_|). The CL adds WPT tests that fail when the old, incorrect behavior happens. The new test initiates a beacon request from a frame's unload handler and verifies the Sec-Fetch-Site request header associated with this request. The header depends on the correct request_initiator_site_lock / on using the correct URLLoaderFactory. The fixes in this CL ==================== This CL introduces the |call_before_attaching_new_document| callback that is taken by blink::WebNavigationControl::CommitNavigation and called *after* finishing running the old document's unload handler and *before* the new document can initiate any requests of its own. This fix is a bit icky. In the long-term the callback can be avoided if the |subresource_loader_factories| are stored in a per-document object (i.e. if we swap RenderFrame on every document change, or if we replace RenderFrame with a RenderDocument - see https://crbug.com/936696). The CL adds a TODO to call this out. The fix had to refactor BuildServiceWorkerNetworkProviderForNavigation to make sure that it uses the |new_loader_factories| (even though they have not yet been passes to SetLoaderFactoryBundle). The fix also refactored GetLoaderFactoryBundleFromCreator to a separate method, so that setting the creator-based factories can also be postponed to |call_before_attaching_new_document|. Bug: 986577 Change-Id: If0209df61b0305ec43b5179bfdc1b9f8668a88b5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1716084 Reviewed-by: Wei Li <[email protected]> Reviewed-by: Tommy Li <[email protected]> Reviewed-by: Yutaka Hirano <[email protected]> Reviewed-by: Kinuko Yasuda <[email protected]> Commit-Queue: Łukasz Anforowicz <[email protected]> Cr-Commit-Position: refs/heads/master@{#685290}
1 parent 2a0b3d6 commit c0b0a0d

File tree

2 files changed

+76
-0
lines changed

2 files changed

+76
-0
lines changed

‎fetch/sec-metadata/resources/unload-with-beacon.html

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!DOCTYPE html>
2+
<script>
3+
// When told, register an unload handler that will trigger a beacon to the
4+
// URL given by the sender of the message.
5+
window.addEventListener('message', e => {
6+
var url = e.data;
7+
window.addEventListener('unload', () => {
8+
navigator.sendBeacon(url, 'blah');
9+
});
10+
window.parent.postMessage('navigate-away', '*');
11+
});
12+
</script>

‎fetch/sec-metadata/unload.tentative.https.sub.html

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<!DOCTYPE html>
2+
<script src=/resources/testharness.js></script>
3+
<script src=/resources/testharnessreport.js></script>
4+
<script src=/resources/testdriver.js></script>
5+
<script src=/resources/testdriver-vendor.js></script>
6+
<script src=/fetch/sec-metadata/resources/helper.js></script>
7+
<script src=/common/utils.js></script>
8+
<body>
9+
<script>
10+
// The test
11+
// 1. Creates a same-origin iframe
12+
// 2. Adds to the iframe an unload handler that will
13+
// trigger a request to <unload_request_url>/.../record-header.py...
14+
// 3. Navigate the iframe to a cross-origin url (to data: url)
15+
// 4. Waits until the request goes through
16+
// 5. Verifies Sec-Fetch-Site request header of the request.
17+
//
18+
// This is a regression test for https://crbug.com/986577.
19+
function create_test(unload_request_origin, expectations) {
20+
async_test(t => {
21+
// STEP 1: Create an iframe.
22+
let nonce = token();
23+
let key = "unload-test-" + nonce;
24+
let url = unload_request_origin +
25+
"/fetch/sec-metadata/resources/record-header.py?file=" + key;
26+
let i = document.createElement('iframe');
27+
i.src = 'resources/unload-with-beacon.html';
28+
i.onload = () => {
29+
// STEP 2: Ask the iframe to add an unload handler.
30+
i.contentWindow.postMessage(url, '*');
31+
};
32+
window.addEventListener('message', e => {
33+
// STEP 3: Navigate the iframe away
34+
i.contentWindow.location = 'data:text/html,DONE';
35+
});
36+
document.body.appendChild(i);
37+
38+
// STEPS 4 and 5: Wait for the beacon to go through and verify
39+
// the request headers.
40+
function wait_and_verify() {
41+
t.step_timeout(() => {
42+
fetch("resources/record-header.py?retrieve=true&file=" + key)
43+
.then(response => response.text())
44+
.then(text => t.step(() => {
45+
if (text == 'No header has been recorded') {
46+
wait_and_verify();
47+
return;
48+
}
49+
assert_header_equals(text, expectations);
50+
t.done();
51+
}))
52+
}, 200);
53+
}
54+
wait_and_verify();
55+
}, "Fetch from an unload handler");
56+
}
57+
58+
create_test("https://{{host}}:{{ports[https][0]}}", {
59+
"dest": "empty",
60+
"site": "same-origin",
61+
"user": "",
62+
"mode": "no-cors"
63+
});
64+
</script>

0 commit comments

Comments
 (0)