Skip to content

Commit a1229a0

Browse files
fred-wangchromium-wpt-export-bot
authored andcommitted
Make registerAltDictionaryAndWait more robust and review usages.
Currently, `registerAltDictionaryAndWait()` calls `waitUntilAvailableDictionaryHeader()` which waits until a request to `echo-headers2.py` will be sent with an `available-dictionary` header matching the dictionary hash. This does not properly works if two calls of `registerAltDictionaryAndWait()` are made in the same `compression_dictionary_promise_test`, or can lead to flaky behavior if they are made in the same test file because it can race with test cleanup's cache clearing. To work around these issues, we instead force callers to pass distinct dictionary id and watch for the `Dictionary-ID` header instead. Also improve the documentation are verify call sites adhere to it. Bug: 522338661, 40255884 Change-Id: Ib0945969a4093b2e3abd16b222d2f7ebc2450a23
1 parent 1985b47 commit a1229a0

6 files changed

Lines changed: 53 additions & 23 deletions

fetch/compression-dictionary/compressed-large-resources-001.tentative.https.html

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,15 @@
2020
compression_dictionary_promise_test(async t => {
2121
// Load the non-compressed image to register it as a dictionary for itself.
2222
await fetchStaticResourceWithUseAsDictionaryHeader(image_dictionary, image_compressed, "image");
23-
await registerAltDictionaryAndWait(t);
23+
24+
// Wait for another dictionary to register to give a chance for the
25+
// previous registration to complete. That registration was not done via
26+
// register-dictionary.py, so we can't use waitUntilPreviousRequestHeaders.
27+
// It also uses a specific match and match-dest, so we cannot use
28+
// checkHeaders/checkHeader because echo-headers.py won't use the registered
29+
// dictionary (specifically we can't add echo-headers.py as an alternative
30+
// to image_compressed, because match does not accept Regexp groups).
31+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
2432

2533
// Append the compressed image.
2634
const img = document.createElement("img");
@@ -43,7 +51,11 @@
4351
compression_dictionary_promise_test(async t => {
4452
// Load the non-compressed script to register it as a dictionary for itself.
4553
await fetchStaticResourceWithUseAsDictionaryHeader(script_dictionary, script_compressed, "script");
46-
await registerAltDictionaryAndWait(t);
54+
55+
// Wait for another dictionary to register to give a chance for the
56+
// previous registration to complete. See comment above about why we can't
57+
// use waitUntilPreviousRequestHeaders, checkHeaders or checkHeader.
58+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
4759

4860
// Append the compressed script.
4961
const script = document.createElement("script");
@@ -68,7 +80,11 @@
6880
compression_dictionary_promise_test(async t => {
6981
// Load the non-compressed style to register it as a dictionary for itself.
7082
await fetchStaticResourceWithUseAsDictionaryHeader(style_dictionary, style_compressed, "style");
71-
await registerAltDictionaryAndWait(t);
83+
84+
// Wait for another dictionary to register to give a chance for the
85+
// previous registration to complete. See comment above about why we can't
86+
// use waitUntilPreviousRequestHeaders, checkHeaders or checkHeader.
87+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
7288

7389
// Append the compressed style and test div.
7490
const link = document.createElement("link");

fetch/compression-dictionary/compressed-large-resources-002.tentative.https.html

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323
compression_dictionary_promise_test(async t => {
2424
// Load the non-compressed style to register it as a dictionary for itself.
2525
await fetchStaticResourceWithUseAsDictionaryHeader(config.subframe_dictionary, config.subframe_compressed, "iframe");
26-
await registerAltDictionaryAndWait(t);
26+
27+
// Wait for another dictionary to register to give a chance for the
28+
// previous registration to complete. See comment in the 001 file about
29+
// why we can't use waitUntilPreviousRequestHeaders, checkHeaders or checkHeader.
30+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
2731

2832
// Append the compressed subframe.
2933
const iframe = document.createElement("iframe");

fetch/compression-dictionary/dictionary-fetch-no-cors.tentative.https.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
try {
4545
await fetch(redirect_url, {mode: 'no-cors'});
4646
} catch (e) {}
47-
await registerAltDictionaryAndWait(t);
47+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
4848
const headers = await (await fetch(`${CROSS_ORIGIN_RESOURCES_URL}/echo-headers.py`)).json();
4949
assert_false("available-dictionary" in headers);
5050
}, 'Opaque responses resulting from cross-origin redirects in no-cors mode do not register dictionary');

fetch/compression-dictionary/dictionary-match.tentative.https.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
'/fetch/compression-dictionary/resources/echo-headers.py');
1818
await fetch(
1919
`${kRegisterDictionaryPath}?id=id1&match-dest=${match_dest}&match=${pattern}`);
20-
await registerAltDictionaryAndWait(t);
20+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
2121
const headers = await (await fetch('./resources/echo-headers.py')).json();
2222
assert_false("available-dictionary" in headers);
2323
}, 'Dictionary with unknown match-dest is not used for fetch() API');
@@ -46,7 +46,7 @@
4646
'/fetch/compression-dictionary/resources/echo-headers.py');
4747
await fetch(
4848
`${kRegisterDictionaryPath}?id=id1&match-dest=${match_dest}&match=${pattern}`);
49-
await registerAltDictionaryAndWait(t);
49+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
5050
const headers = await (await fetch('./resources/echo-headers.py')).json();
5151
assert_false("available-dictionary" in headers);
5252
}, 'Dictionary with non-matching match-dest is not used');

fetch/compression-dictionary/dictionary-registration.tentative.https.html

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,9 @@
101101
'/fetch/compression-dictionary/resources/echo-headers.py');
102102
await fetch(
103103
`${kRegisterDictionaryPath}?id=id1&age=7200&max-age=3600&match=${pattern}`);
104-
await registerAltDictionaryAndWait(t);
105-
assert_equals((await checkHeaders({use_alt_path: true}))['dictionary-id'], '"id2"');
104+
const dictionary_id = `alt_${token()}`;
105+
await registerAltDictionaryAndWait(t, dictionary_id);
106+
assert_equals((await checkHeaders({use_alt_path: true}))['dictionary-id'], `"${dictionary_id}"`);
106107
// Make sure the expired dictionary isn't announced as being available.
107108
const headers = await (await fetch('./resources/echo-headers.py')).json();
108109
assert_false("available-dictionary" in headers);
@@ -111,15 +112,15 @@
111112
compression_dictionary_promise_test(async (t) => {
112113
const pattern = "%7B";
113114
await fetch(`${kRegisterDictionaryPath}?id=id1&match=${pattern}`);
114-
await registerAltDictionaryAndWait(t);
115+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
115116
const headers = await (await fetch('./resources/echo-headers.py')).json();
116117
assert_false("available-dictionary" in headers);
117118
}, 'Dictionary with invalid match string is not registered');
118119

119120
compression_dictionary_promise_test(async (t) => {
120121
const pattern = encodeURIComponent(`${CROSS_ORIGIN_RESOURCES_URL}/echo-headers.py`);
121122
await fetch(`${kRegisterDictionaryPath}?id=id1&match=${pattern}`);
122-
await registerAltDictionaryAndWait(t);
123+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
123124
const headers = await (await fetch('./resources/echo-headers.py')).json();
124125
assert_false("available-dictionary" in headers);
125126
}, 'Dictionary with match string for a different origin is not registered');
@@ -129,7 +130,7 @@
129130
'/fetch/compression-dictionary/resources/echo-headers.py');
130131
await fetch(
131132
`${kRegisterDictionaryPath}?id=id1&type=bogus&match=${pattern}`);
132-
await registerAltDictionaryAndWait(t);
133+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
133134
const headers = await (await fetch('./resources/echo-headers.py')).json();
134135
assert_false("available-dictionary" in headers);
135136
}, 'Dictionary with unsupported type is not registered');
@@ -149,7 +150,7 @@
149150
'/fetch/compression-dictionary/resources/echo-headers.py');
150151
await fetch(
151152
`${kRegisterDictionaryPath}?id=${dictionary_id}&match=${pattern}`);
152-
await registerAltDictionaryAndWait(t);
153+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
153154
const headers = await (await fetch('./resources/echo-headers.py')).json();
154155
assert_false("available-dictionary" in headers);
155156
}, 'Dictionary with 1025 character dictionary ID is not registered');
@@ -165,14 +166,14 @@
165166

166167
compression_dictionary_promise_test(async (t) => {
167168
await fetch(`${kRegisterDictionaryPath}?id=id1&use-as-dictionary=%3F0`);
168-
await registerAltDictionaryAndWait(t);
169+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
169170
const headers = await (await fetch('./resources/echo-headers.py')).json();
170171
assert_false("available-dictionary" in headers);
171172
}, 'Dictionary with malformed structured header is not registered');
172173

173174
compression_dictionary_promise_test(async (t) => {
174175
await fetch(`${kRegisterDictionaryPath}?id=id1&max-age=0`);
175-
await registerAltDictionaryAndWait(t);
176+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
176177
const headers = await (await fetch('./resources/echo-headers.py')).json();
177178
assert_false("available-dictionary" in headers);
178179
}, 'Dictionary with max-age=0 is not registered');
@@ -196,7 +197,7 @@
196197
const pattern = encodeURIComponent(
197198
'/fetch/compression-dictionary/resources/echo-headers.py');
198199
await fetch(`${kRegisterDictionaryPath}?id=${encodeURIComponent("€")}&match=${pattern}`);
199-
await registerAltDictionaryAndWait(t);
200+
await registerAltDictionaryAndWait(t, `alt_${token()}`);
200201
const headers = await (await fetch('./resources/echo-headers.py')).json();
201202
assert_false("available-dictionary" in headers);
202203
}, 'Dictionary with invalid character in dictionary ID is not registered');

fetch/compression-dictionary/resources/compression-dictionary-util.sub.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,23 @@ function compression_dictionary_promise_test(func, name, properties) {
148148
}
149149

150150
// Registers an alternative dictionary and waits for its registration to
151-
// complete. This is used in tests to confirm that another dictionary's
152-
// registration process has fully finished.
153-
async function registerAltDictionaryAndWait(t) {
151+
// complete. This is used to wait for another dictionary's registration process.
152+
// If that registration process is expected to succeed, please consider using
153+
// waitUntilPreviousRequestHeaders (if the dictionary is register-dictionary
154+
// with some save_header) or checkHeaders/checkHeader (if match includes
155+
// echo-headers) to obtain more reliable wait time.
156+
// To ensure registerAltDictionaryAndWait() does not resolve immediately due
157+
// to a previous call, pass a different dictionary_id for each call.
158+
async function registerAltDictionaryAndWait(t, dictionary_id) {
159+
assert_true(typeof dictionary_id === 'string' && dictionary_id.length > 0,
160+
'registerAltDictionaryAndWait requires a valid dictionary_id');
154161
const pattern = encodeURIComponent("/fetch/compression-dictionary/resources/echo-headers2.py");
155-
await fetch(`${kRegisterDictionaryPath}?id=id2&match=${pattern}`);
156-
assert_equals(
157-
await waitUntilAvailableDictionaryHeader(t, {use_alt_path: true}),
158-
kDefaultDictionaryHashBase64);
162+
await fetch(
163+
`${kRegisterDictionaryPath}?id=${dictionary_id}&match=${pattern}`);
164+
const result = await waitUntilHeader(
165+
t, 'dictionary-id',
166+
{expected_header: `"${dictionary_id}"`, use_alt_path: true});
167+
assert_equals(result, `"${dictionary_id}"`);
159168
}
160169

161170
function navigateToTestWithCompressionDictionaryEarlyHints(test_url, dictionary_url) {

0 commit comments

Comments
 (0)