Skip to content

Commit f81c4ab

Browse files
sisidovskichromium-wpt-export-bot
authored andcommitted
Delay SW loader deletion until fetch clone is complete
When using `race-network-and-fetch-handler`, the ServiceWorkerMainResourceLoader could be destroyed after the network response is received, but before the fetch handler has finished cloning the response body. This could lead to a race condition where the loader is deleted prematurely. This change ensures that the loader's lifetime is extended until the response body has been fully cloned for the fetch handler or the mojo disconnection to the ServiceWorker. A callback is introduced to signal the completion of the cloning process or disconnection, and the deletion logic is updated to wait for this signal before invalidating the loader. This behavior is gated behind the `kServiceWorkerStaticRouterRaceRequestFix2` feature flag. Change-Id: If7ec62b18c99c8fc13faf657a44969a42bf7f82a Bug: 340949948 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7079632 Reviewed-by: Rakina Zata Amni <[email protected]> Reviewed-by: Yoshisato Yanagisawa <[email protected]> Commit-Queue: Shunya Shishido <[email protected]> Reviewed-by: Keita Suzuki <[email protected]> Cr-Commit-Position: refs/heads/main@{#1538932}
1 parent d1ec182 commit f81c4ab

File tree

2 files changed

+46
-16
lines changed

2 files changed

+46
-16
lines changed

service-workers/service-worker/resources/static-router-race-network-and-fetch-handler-sw.js

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,28 @@ self.addEventListener('activate', e => {
2727
e.waitUntil(clients.claim());
2828
});
2929

30-
self.addEventListener('fetch', function(event) {
31-
recordRequest(event.request);
32-
const url = new URL(event.request.url);
33-
34-
35-
// Force slow response
36-
if (url.searchParams.has('sw_slow')) {
37-
const start = Date.now();
38-
while (true) {
39-
if (Date.now() - start > 200) {
40-
break;
30+
self.addEventListener('fetch', async function(event) {
31+
try {
32+
recordRequest(event.request);
33+
const url = new URL(event.request.url);
34+
// Force slow response
35+
if (url.searchParams.has('sw_slow')) {
36+
const start = Date.now();
37+
while (true) {
38+
if (Date.now() - start > 200) {
39+
break;
40+
}
4141
}
4242
}
43+
const nonce = url.searchParams.get('nonce');
44+
event.respondWith(new Response(nonce));
45+
// Call `fetch(e.request)` but it's not consumed.
46+
if (url.searchParams.has('call_fetch')) {
47+
await fetch(event.request);
48+
}
49+
} catch (e) {
50+
recordError(e)
4351
}
44-
45-
const nonce = url.searchParams.get('nonce');
46-
event.respondWith(new Response(nonce));
4752
});
4853

4954
self.addEventListener('message', function(event) {

service-workers/service-worker/static-router-race-network-and-fetch-handler.https.html

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
const iframe = await createIframe(t, `${FRAME_SRC}?nonce=${rnd}&server_slow`);
2323
// Expect the response from the fetch handler.
2424
assert_equals(iframe.contentWindow.document.body.innerText, rnd);
25-
const {requests} = await get_info_from_worker(worker);
25+
const {requests, errors} = await get_info_from_worker(worker);
2626
assert_equals(requests.length, 1);
27+
assert_equals(errors.length, 0);
2728
}, 'Main resource load matched the rule with race-network-and-fetch-handler source, and the fetch handler response is faster than the server response');
2829

2930
promise_test(async t => {
@@ -33,10 +34,34 @@
3334
// Expect the response from the netowrk request.
3435
assert_equals(iframe.contentWindow.document.body.innerText, "Network with GET request");
3536
// Ensure the fetch handler is also executed.
36-
const {requests} = await get_info_from_worker(worker);
37+
const {requests, errors} = await get_info_from_worker(worker);
3738
assert_equals(requests.length, 1);
39+
assert_equals(errors.length, 0);
3840
}, 'Main resource load matched the rule with race-network-and-fetch-handler source, and the server reseponse is faster than the fetch handler');
3941

42+
promise_test(async t => {
43+
const rnd = randomString();
44+
const worker = await registerAndActivate(t, ROUTER_KEY, SW_SRC);
45+
const iframe = await createIframe(t, `${FRAME_SRC}?nonce=${rnd}&server_slow&call_fetch`);
46+
// Expect the response from the fetch handler.
47+
assert_equals(iframe.contentWindow.document.body.innerText, rnd);
48+
const {requests, errors} = await get_info_from_worker(worker);
49+
assert_equals(requests.length, 1);
50+
assert_equals(errors.length, 0);
51+
}, 'Main resource load matched the rule with race-network-and-fetch-handler source, and the fetch handler response is faster than the server response while fetch() is triggered inside the fetch handler');
52+
53+
promise_test(async t => {
54+
const rnd = randomString();
55+
const worker = await registerAndActivate(t, ROUTER_KEY, SW_SRC);
56+
const iframe = await createIframe(t, `${FRAME_SRC}?nonce=${rnd}&sw_slow&call_fetch`);
57+
// Expect the response from the netowrk request.
58+
assert_equals(iframe.contentWindow.document.body.innerText, "Network with GET request");
59+
// Ensure the fetch handler is also executed.
60+
const {requests, errors} = await get_info_from_worker(worker);
61+
assert_equals(requests.length, 1);
62+
assert_equals(errors.length, 0);
63+
}, 'Main resource load matched the rule with race-network-and-fetch-handler source, and the server reseponse is faster than the fetch handler while fetch() is triggered inside the fetch handler');
64+
4065
promise_test(async t => {
4166
const rnd = randomString();
4267
const worker = await registerAndActivate(t, ROUTER_KEY, SW_SRC);

0 commit comments

Comments
 (0)