Turn Critical-CH restarts into internal redirects
This fixes a number of issues, including crashes. Unfortunately, this
will not fix issues with ACCEPT_CH, which is having a very similar
crasher problem.
The behaviour is now as follows: if a Critical-CH restart is detected,
the Accept-CH preferences are stored as they would be and the
ThrottlingURLLoader changes the response to a synthetic redirect. This
is registered and takes the response through the normal navigation
redirect codepath (which adds the client hints that were stored) and
allows extensions, service workers, and other interceptors the chance to
handle the new request. The one-redirect limit was also removed and now
counts towards the network/navigation redirect limits. Coincidentally,
this also runs through the devtools, which makes both the initial and
new requests both visible in the Network tab as an internal redirect
and regular request.
The crashing code is very much NOT fixed by this (see crbug.com/1217589)
but rather is side-stepped (instead of restarting the request outright,
there's now an internal redirect, which also makes more sense
semantically)
Also (somewhat coincidentally) fixed an issue with the
client hint/service worker WPTs, in that the tests were not waiting for
the load event to finish
(see the changes in
//third_party/blink/web_tests/external/wpt/client-hints/service-workers/resources/util.js)
Bug: 1217589, 1228536, 1176879
Change-Id: I8401c67818cdd6cf16a75422b2ba48fe429dc961
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2983659
Commit-Queue: Aaron Tagliaboschi <aarontag@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904079}
diff --git a/client-hints/service-workers/critical-ch/echo-hint-in-html.py b/client-hints/service-workers/critical-ch/echo-hint-in-html.py
new file mode 100644
index 0000000..4eaca29
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/echo-hint-in-html.py
@@ -0,0 +1,24 @@
+import sys
+
+def main(request, response):
+ """
+ Simple handler that sets a response header based on which client hint
+ request headers were received.
+ """
+
+ response.headers.append(b"Content-Type", b"text/html; charset=UTF-8")
+ response.headers.append(b"Access-Control-Allow-Origin", b"*")
+ response.headers.append(b"Access-Control-Allow-Headers", b"*")
+ response.headers.append(b"Access-Control-Expose-Headers", b"*")
+
+ response.headers.append(b"Cache-Control", b"no-store")
+
+ response.headers.append(b"Accept-CH", b"device-memory");
+ response.headers.append(b"Critical-CH", b"device-memory");
+
+ result = "FAIL"
+
+ if b"device-memory" in request.headers:
+ result = "PASS"
+
+ response.content = result
diff --git a/client-hints/service-workers/critical-ch/foo.html b/client-hints/service-workers/critical-ch/foo.html
new file mode 100644
index 0000000..ba578e4
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/foo.html
@@ -0,0 +1 @@
+BAR
diff --git a/client-hints/service-workers/critical-ch/foo.html.headers b/client-hints/service-workers/critical-ch/foo.html.headers
new file mode 100644
index 0000000..4fc42a2
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/foo.html.headers
@@ -0,0 +1,2 @@
+Accept-CH: device-memory
+Critical-CH: device-memory
diff --git a/client-hints/service-workers/critical-ch/intercept-request.js b/client-hints/service-workers/critical-ch/intercept-request.js
new file mode 100644
index 0000000..b85e6e9
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/intercept-request.js
@@ -0,0 +1,6 @@
+self.addEventListener('fetch', (event) => {
+ result="FAIL";
+ if(event.request.headers.has("device-memory"))
+ result="PASS";
+ event.respondWith(new Response(result));
+});
diff --git a/client-hints/service-workers/critical-ch/navigation-preload.js b/client-hints/service-workers/critical-ch/navigation-preload.js
new file mode 100644
index 0000000..d8a38ee
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/navigation-preload.js
@@ -0,0 +1,2 @@
+self.addEventListener('activate', () => self.registration.navigationPreload.enable());
+self.addEventListener('fetch', (event) => event.respondWith(event.preloadResponse));
diff --git a/client-hints/service-workers/critical-ch/new-request.js b/client-hints/service-workers/critical-ch/new-request.js
new file mode 100644
index 0000000..395c771
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/new-request.js
@@ -0,0 +1,3 @@
+self.addEventListener('fetch', (event) => {
+ event.respondWith(fetch("/client-hints/service-workers/resources/echo-hint-in-html.py"))
+});
diff --git a/client-hints/service-workers/critical-ch/passthrough-request.js b/client-hints/service-workers/critical-ch/passthrough-request.js
new file mode 100644
index 0000000..5541c5e
--- /dev/null
+++ b/client-hints/service-workers/critical-ch/passthrough-request.js
@@ -0,0 +1 @@
+self.addEventListener('fetch', (event) => fetch(event.request));
diff --git a/client-hints/service-workers/intercept-request-critical.https.window.js b/client-hints/service-workers/intercept-request-critical.https.window.js
new file mode 100644
index 0000000..079b9fd
--- /dev/null
+++ b/client-hints/service-workers/intercept-request-critical.https.window.js
@@ -0,0 +1,5 @@
+// META: script=/service-workers/service-worker/resources/test-helpers.sub.js
+// META: script=resources/util.js
+promise_test((t) =>
+ ch_sw_test(t, 'critical-ch/intercept-request.js', 'critical-ch/foo.html', 'FAIL'),
+ "Service workers succsefully receives hints from request");
diff --git a/client-hints/service-workers/navigation-preload-critical.https.window.js b/client-hints/service-workers/navigation-preload-critical.https.window.js
new file mode 100644
index 0000000..5d05ab3
--- /dev/null
+++ b/client-hints/service-workers/navigation-preload-critical.https.window.js
@@ -0,0 +1,5 @@
+//META: script=/service-workers/service-worker/resources/test-helpers.sub.js
+//META: script=resources/util.js
+promise_test((t) =>
+ ch_sw_test(t, 'critical-ch/navigation-preload.js', 'critical-ch/echo-hint-in-html.py', 'PASS'),
+ "Service worker successfully passes hints through to new fetch");
diff --git a/client-hints/service-workers/navigation-preload.https.html b/client-hints/service-workers/navigation-preload.https.html
new file mode 100644
index 0000000..a952e1e
--- /dev/null
+++ b/client-hints/service-workers/navigation-preload.https.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<title>Client Hint/Service Worker interaction</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/service-workers/service-worker/resources/test-helpers.sub.js"></script>
+<script src="resources/util.js"></script>
+<script>
+promise_test((t) =>
+ ch_sw_test(t, 'resources/navigation-preload.js', 'resources/echo-hint-in-html.py', 'PASS'),
+ "Service worker successfully passes hints through to new fetch");
+</script>
diff --git a/client-hints/service-workers/navigation-preload.https.html.headers b/client-hints/service-workers/navigation-preload.https.html.headers
new file mode 100644
index 0000000..91ee406
--- /dev/null
+++ b/client-hints/service-workers/navigation-preload.https.html.headers
@@ -0,0 +1 @@
+Accept-CH: device-memory
diff --git a/client-hints/service-workers/new-request-critical.https.window.js b/client-hints/service-workers/new-request-critical.https.window.js
new file mode 100644
index 0000000..9dee11a
--- /dev/null
+++ b/client-hints/service-workers/new-request-critical.https.window.js
@@ -0,0 +1,5 @@
+//META: script=/service-workers/service-worker/resources/test-helpers.sub.js
+//META: script=resources/util.js
+promise_test((t) =>
+ ch_sw_test(t, 'critical-ch/new-request.js', 'critical-ch/foo.html', 'FAIL'),
+ "Service worker does NOT generate client hints in a new request");
diff --git a/client-hints/service-workers/passthrough-request-critical.https.window.js b/client-hints/service-workers/passthrough-request-critical.https.window.js
new file mode 100644
index 0000000..4d59f5f
--- /dev/null
+++ b/client-hints/service-workers/passthrough-request-critical.https.window.js
@@ -0,0 +1,5 @@
+//META: script=/service-workers/service-worker/resources/test-helpers.sub.js
+//META: script=resources/util.js
+promise_test((t) =>
+ ch_sw_test(t, 'critical-ch/passthrough-request.js', 'critical-ch/echo-hint-in-html.py', 'PASS'),
+ "Service worker successfully passes hints through to new fetch");
diff --git a/client-hints/service-workers/resources/navigation-preload.js b/client-hints/service-workers/resources/navigation-preload.js
new file mode 100644
index 0000000..d8a38ee
--- /dev/null
+++ b/client-hints/service-workers/resources/navigation-preload.js
@@ -0,0 +1,2 @@
+self.addEventListener('activate', () => self.registration.navigationPreload.enable());
+self.addEventListener('fetch', (event) => event.respondWith(event.preloadResponse));
diff --git a/client-hints/service-workers/resources/util.js b/client-hints/service-workers/resources/util.js
index 5692cd8..d9b8f3f 100644
--- a/client-hints/service-workers/resources/util.js
+++ b/client-hints/service-workers/resources/util.js
@@ -1,7 +1,7 @@
async function ch_sw_test(t, worker, url, response) {
- r = await service_worker_unregister_and_register(t, worker, worker)
+ r = await service_worker_unregister_and_register(t, worker, url);
await wait_for_state(t, r.installing, 'activated')
- var popup_window = window.open('about:blank');
+ var popup_window = window.open("/common/blank.html");
assert_not_equals(popup_window, null, "Popup windows not allowed?");
t.add_cleanup(async _=>{
@@ -9,10 +9,15 @@
await r.unregister();
});
- popup_window.addEventListener('load', (e) => {
- t.step(()=>{assert_equals(popup_window.document.body.textContent, response)});
- t.done();
+ popup_load = new Promise((resolve, reject) => {
+ popup_window.addEventListener('load', t.step_func((e) => {
+ if(popup_window.location.pathname != "/blank.html") {
+ assert_equals(popup_window.document.body.textContent, response);
+ resolve();
+ }
+ }))
});
popup_window.location = url;
+ await popup_load;
}
