Skip to content

Commit

Permalink
[client hints] Match update behavior to the spec.
Browse files Browse the repository at this point in the history
Accept-CH: from headers, should override, not merge; including having ""
disable things.

In http-equiv, OTOH (where it doesn't persist), it should still merge.

Change-Id: I5c3c94023b10b265e63a8abe29a431bd1da8dadf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093032
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766003}
  • Loading branch information
Maks Orlovich authored and Hexcles committed May 14, 2020
1 parent ea0378b commit 90015af
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 12 deletions.
11 changes: 11 additions & 0 deletions client-hints/accept-ch-stickiness/resources/accept-ch-blank.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<html>
<body>

<!-- Page with an empty accept-ch header, which disables client hints -->
<script>
window.top.opener.postMessage('Loaded', '*');
</script>


</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Accept-CH:
Access-Control-Allow-Origin: *
31 changes: 19 additions & 12 deletions client-hints/accept-ch-stickiness/resources/accept-ch-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const echo = "/client-hints/accept-ch-stickiness/resources/echo-client-hints-received.py";
const accept = "/client-hints/accept-ch-stickiness/resources/accept-ch.html";
const accept_blank = "/client-hints/accept-ch-stickiness/resources/accept-ch-blank.html";
const no_accept = "/client-hints/accept-ch-stickiness/resources/no-accept-ch.html";
const httpequiv_accept = "/client-hints/accept-ch-stickiness/resources/http-equiv-accept-ch.html";
const expect = "/client-hints/accept-ch-stickiness/resources/expect-client-hints-headers.html"
const do_not_expect = "/client-hints/accept-ch-stickiness/resources/do-not-expect-client-hints-headers.html"
Expand Down Expand Up @@ -49,30 +51,26 @@ function verify_subresource_state(expect_url, test_name) {
});
}, test_name + " got client hints according to expectations.");
}
const run_test = test => {
// First, verify the initial state to make sure that the browser does not have
// client hints preferences cached from a previous run of the test.
verify_initial_state(test.initial_url, test.name);

// Then, attempt to set Accept-CH
function attempt_set(test_type, accept_url, test_name, test_name_suffix) {
promise_test(t => {
return new Promise(resolve => {
if (test.type == "navigation") {
const win = window.open(test.accept_url);
if (test_type == "navigation") {
const win = window.open(accept_url);
assert_not_equals(win, null, "Popup windows not allowed?");
addEventListener('message', t.step_func(() => {
win.close();
resolve();
}), false);
} else if (test.type == "iframe") {
} else if (test_type == "iframe") {
const iframe = document.createElement("iframe");
iframe.addEventListener('load', t.step_func(() => {
resolve();
}), false);
iframe.src = test.accept_url;
iframe.src = accept_url;
document.body.appendChild(iframe);
} else if (test.type == "subresource") {
fetch(test.accept_url).then(r => {
} else if (test_type == "subresource") {
fetch(accept_url).then(r => {
assert_equals(r.status, 200, "subresource response status")
// Verify that the browser did not include client hints in the request
// headers, just because we can..
Expand All @@ -85,7 +83,16 @@ const run_test = test => {
assert_unreached("unknown test type");
}
});
}, test.name + " set Accept-CH");
}, test_name + " set Accept-CH" + test_name_suffix);
}

const run_test = test => {
// First, verify the initial state to make sure that the browser does not have
// client hints preferences cached from a previous run of the test.
verify_initial_state(test.initial_url, test.name);

// Then, attempt to set Accept-CH
attempt_set(test.type, test.accept_url, test.name, "");

// Finally, verify that CH are actually sent (or not) on requests
verify_navigation_state(test.expect_url, test.name);
Expand Down
11 changes: 11 additions & 0 deletions client-hints/accept-ch-stickiness/resources/no-accept-ch.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<html>
<body>

<!-- Page with out an accept-ch header; client hints are unaffected -->
<script>
window.top.opener.postMessage('Loaded', '*');
</script>


</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Access-Control-Allow-Origin: *
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!doctype html>
<meta name="timeout" content="long">
<title>Accept-CH Persistence test</title>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/get-host-info.sub.js"></script>
<script src="resources/accept-ch-test.js"></script>

<script>
// Tests that an empty accept-ch header disables client hints.
const test_name = "empty-ch on navigation";
verify_initial_state(echo, test_name);
attempt_set("navigation", accept, test_name, " to non-empty first");
attempt_set("navigation", accept_blank, test_name, " to empty second");
verify_navigation_state(do_not_expect, test_name);
</script>
</body>
</html>

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!doctype html>
<meta name="timeout" content="long">
<title>Accept-CH Persistence test</title>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/get-host-info.sub.js"></script>
<script src="resources/accept-ch-test.js"></script>

<script>
// Tests that a non-existing accept-ch header doesn't affect client hints.
const test_name = "empty-ch on navigation";
verify_initial_state(echo, test_name);
attempt_set("navigation", accept, test_name, " to non-empty first");
attempt_set("navigation", no_accept, test_name, " w/o header second");
verify_navigation_state(expect, test_name);
</script>
</body>
</html>

44 changes: 44 additions & 0 deletions client-hints/http-equiv-accept-ch-merge.https.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<html>
<head>
<meta http-equiv="Accept-CH" content="viewport-width, rtt">
<meta http-equiv="Accept-CH" content="downlink, ect, lang">
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/get-host-info.sub.js"></script>
<script>

// Test of merge of http-equiv headers on top of accept-ch provided ones.
//
// resources/echo-client-hints-received.py sets the response headers depending on the set
// of client hints it receives in the request headers.

promise_test(t => {
return fetch(get_host_info()["HTTPS_ORIGIN"] + "/client-hints/resources/echo-client-hints-received.py").then(r => {
assert_equals(r.status, 200)
// Verify that the browser includes client hints in the headers.
assert_true(r.headers.has("device-memory-received"), "device-memory-received");
assert_true(r.headers.has("dpr-received"), "dpr-received");
assert_true(r.headers.has("lang-received"), "lang-received");
assert_true(r.headers.has("viewport-width-received"), "viewport-width-received");

assert_true(r.headers.has("rtt-received"), "rtt-received");
var rtt = parseInt(r.headers.get("rtt-received"));
assert_greater_than_equal(rtt, 0);
assert_less_than_equal(rtt, 3000);
assert_equals(rtt % 50, 0, 'rtt must be a multiple of 50 msec');

assert_true(r.headers.has("downlink-received"), "downlink-received");
var downlinkKbps = r.headers.get("downlink-received") * 1000;
assert_greater_than_equal(downlinkKbps, 0);
assert_less_than_equal(downlinkKbps, 10000);

assert_in_array(r.headers.get("ect-received"), ["slow-2g", "2g",
"3g", "4g"], 'ect-received is unexpected');
});
}, "Accept-CH header test");

</script>

</body>
</html>
2 changes: 2 additions & 0 deletions client-hints/http-equiv-accept-ch-merge.https.html.headers
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Accept-CH: device-memory, dpr

0 comments on commit 90015af

Please sign in to comment.