Skip to content

Add test to verify that link rel=compression-dictionary fetches can be blocked via connect-src#61309

Merged
fred-wang merged 1 commit into
masterfrom
fetch-compression-dictionary-link-connect-src
Jul 18, 2026
Merged

Add test to verify that link rel=compression-dictionary fetches can be blocked via connect-src#61309
fred-wang merged 1 commit into
masterfrom
fetch-compression-dictionary-link-connect-src

Conversation

@fred-wang

Copy link
Copy Markdown
Contributor

No description provided.

@fred-wang
fred-wang force-pushed the fetch-compression-dictionary-link-connect-src branch from 1a5b856 to 6c8b19f Compare July 16, 2026 10:51
@fred-wang
fred-wang requested a review from pmeenan July 16, 2026 11:46
@fred-wang fred-wang assigned fred-wang and unassigned annevk Jul 16, 2026

@pmeenan pmeenan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two last tweaks.


// Link from HTTP headers are only loaded after page, so don't ever try to
// run the test before.
promise_setup(_ => new Promise(resolve => addEventListener("load", resolve)));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could run async after the document itself has loaded (which could hang). Maybe change to:

promise_setup(() => new Promise(resolve => {
  if (document.readyState === 'complete') {
    resolve();
  } else {
    addEventListener("load", resolve);
  }
}));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I commented above, the doc says the function of the promise_setup is run synchronously, so the event listener would be registered before the load completes i.e. the document is still being parsed and we always have document.readyState === 'loading' here.

I just checked the testharness code to be sure and it's actually only called synchronously if there were no prior promise_setup or promise_tests promises pending to resolve (which is the case in that test).

I'm happy to change to a more verbose promise, but I'd like to understand why this is necessary, as this is not obvious for me from the doc and code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all that guarantees (and the documents assert) is that it runs synchronously before the tests start, not relative to the inline code and the document itself. I don't know the .then async microtask scheduling well enough to know if Gemini is wrong in it's code tracing but this is what it claims (pretty adamantly):

Why promise_setup does not run synchronously (Tracing testharness.js)

In testharness.js#L1151-L1161:

if (!tests.promise_tests) {
    tests.promise_tests = Promise.resolve();
}

tests.promise_tests = tests.promise_tests
    .then(function() {
        tests.setup(null, properties);
        result = func(); // <-- Your function executes inside this .then() callback
        ...
  1. Promise.prototype.then() is always asynchronous (Microtask):
    When there is no prior pending promise, tests.promise_tests is initialized to Promise.resolve(). Calling .then(callback) on a fulfilled Promise does not invoke callback synchronously on the spot. By ECMAScript specification (PerformPromiseThen), .then() queues a PromiseReactionJob onto the microtask queue. Therefore, promise_setup(func) always executes func() asynchronously as a microtask after the current synchronous script execution block finishes.

  2. Why you don't see the hang when loading the page directly in a browser:
    When you load the .html file directly in a standalone browser tab, the HTML parser evaluates the inline <script> tag synchronously during document parsing. When the <script> block finishes, the JavaScript engine drains the microtask queue immediately before the parser resumes and before window.onload fires. In that pristine top-level scenario, func() runs right after the script tag while document.readyState === 'loading'.

  3. Why it can hang across WPT Test Runners (testharnessreport.js & automated harnesses):
    In automated WPT execution (wptrunner, Chromium's run_web_tests, testdriver.js, or multi-window/iframe test wrappers), documents do not always execute as standalone top-level pages:

    • Test Runner & Iframe Injection: Test runners (testharnessreport.js) often run tests inside wrapper iframes, inject scripts, or delay tests.promise_tests execution while setting up cross-process automation hooks or parent-frame communication.
    • Async Setup Hooks: If any runner script or harness utility delays the tests.promise_tests chain across the page load boundary, func() inside promise_setup will execute after the document has already finished loading (document.readyState === 'complete').
  4. The load Event Race Condition:
    Because window.onload only dispatches once per document lifecycle and does not replay for late listeners, addEventListener("load", resolve) inside any promise callback creates a race condition against the harness. If document.readyState === 'complete' when func() runs, resolve is never called, tests.promise_tests stays pending forever, and all subsequent promise_tests hang until the harness times out.

I don't know if the non-blocking microtask is something we generally need to protect against in WPT's. If not, I'm happy to stamp it but it feels like an easy safety net to put in place just in case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pmeenan Thanks. OK, I was wrongly assuming Promise.prototype.then() was called immediately for a fulfilled promise but Gemini seems right here. At least it's document on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then#return_value ; will apply the suggestion then.

@fred-wang
fred-wang force-pushed the fetch-compression-dictionary-link-connect-src branch from 6c8b19f to b1603e9 Compare July 17, 2026 06:22
@fred-wang
fred-wang requested a review from pmeenan July 17, 2026 06:24
@fred-wang
fred-wang force-pushed the fetch-compression-dictionary-link-connect-src branch from b1603e9 to 493a454 Compare July 17, 2026 13:33

@pmeenan pmeenan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@fred-wang
fred-wang merged commit 49a6294 into master Jul 18, 2026
28 checks passed
@fred-wang
fred-wang deleted the fetch-compression-dictionary-link-connect-src branch July 18, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants