Skip to content

Fix request clients for prefetch + service workers #378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 5, 2025
Merged

Fix request clients for prefetch + service workers #378

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
41f43a9
Fix request clients for prefetch + service workers
domenic May 14, 2025
f0b4cdb
Add an explicit field
domenic May 20, 2025
d3f9152
Do not count prefetch clients as controlling
domenic May 20, 2025
0205624
Use the correct active service worker
domenic Jun 4, 2025
b8c0c86
Revert "Do not count prefetch clients as controlling"
domenic Jun 4, 2025
98c946b
Revamp and centralize client creation per offline discussion
domenic Jun 4, 2025
016febd
Fix urlList request vs response
domenic Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions prefetch.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,28 +358,33 @@ The user agent may [=prefetch record/cancel and discard=] records from the [=Doc
1. Let |coopEnforcementResult| be the result of [=creating a cross-origin opener policy enforcement result for navigation=] given |navigable|'s [=navigable/active document=] and |documentState|'s [=document state/initiator origin=].
1. Let |finalSandboxFlags| be an empty [=sandboxing flag set=].
1. Let |responsePolicyContainer| be null.
1. Let |activeServiceWorker| be null.
1. Let |urlList| be an empty [=list=].
1. [=list/For each=] |exchangeRecord| in |record|'s [=prefetch record/redirect chain=]:
1. Let |response| be |exchangeRecord|'s [=exchange record/response=].
1. [=list/Append=] |response|'s [=response/URL=] to |urlList|.
1. Set |responsePolicyContainer| to the result of [=creating a policy container from a fetch response=] given |response| and |request|'s [=request/reserved client=].
1. Let |redirectChainRequest| be |exchangeRecord|'s [=exchange record/request=].
1. Let |redirectChainResponse| be |exchangeRecord|'s [=exchange record/response=].
1. [=list/Append=] |redirectChainRequest|'s [=request/URL=] to |urlList|.
1. Set |responsePolicyContainer| to the result of [=creating a policy container from a fetch response=] given |redirectChainResponse| and |redirectChainRequest|'s [=request/reserved client=].
1. Set |finalSandboxFlags| to the [=set/union=] of |targetSnapshotParams|'s [=target snapshot params/sandboxing flags=] and |responsePolicyContainer|'s [=policy container/CSP list=]'s [=CSP-derived sandboxing flags=].
1. Set |responseOrigin| to the result of [=determining the origin=] given |response|'s [=response/URL=], |finalSandboxFlags|, |documentState|'s [=document state/initiator origin=], and null.
1. Set |responseOrigin| to the result of [=determining the origin=] given |redirectChainResponse|'s [=response/URL=], |finalSandboxFlags|, |documentState|'s [=document state/initiator origin=], and null.
1. Set |activeServiceWorker| to |redirectChainRequest|'s [=request/reserved client=]'s [=environment/active service worker=].
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously, I was thinking of copying the reserved client creation process in
https://wicg.github.io/nav-speculation/prefetch.html#create-navigation-params-by-fetching
i.e. copying the Step 17.1 etc:

If request’s reserved client is not null and currentURL’s origin is not the same as request’s reserved client’s creation URL’s origin, then: ...

to make the reserved creation process consistent between navigation and prefetch.

But on the second thought, I'm starting feeling that we can do like (at Line 381):

Set |request|'s [=request/reserved client=]'s [=environment/active service worker=] to the last element of exchangeRecord's redirect chain's last element's request's reserved client's active service worker.

Because the result of the reserved creation process at prefetch (create-navigation-params-by-fetching) is already stored as the last request's reserved client and we don't re-run/emulate it.

Also

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like the current text of setting it each time through the loop as it treats activeServiceWorker the same as responsePolicyContainer, coopEnforcementResult, and finalSandboxFlags.

I don't understand how this avoids #385 since it's behaviorally equivalent...

So, let me keep this as-is for now.

1. If |navigable| is a [=top-level traversable=], then:
1. Set |responseCOOP| to the result of [=obtaining a cross-origin opener policy=] given |response| and |request|'s [=request/reserved client=].
1. Set |responseCOOP| to the result of [=obtaining a cross-origin opener policy=] given |redirectChainResponse| and |redirectChainRequest|'s [=request/reserved client=].
1. [=Assert=]: If |finalSandboxFlags| is not empty, then |responseCOOP|'s [=cross-origin opener policy/value=] is "`unsafe-none`".

<p class="note">This is guaranteed since the sandboxing flags of the document cannot change to become non-empty since this was prefetched, and the check was not done for a different window. If this changes, this will need to be able to handle failure of this check.</p>
1. Set |coopEnforcementResult| to the result of [=enforcing a response's cross-origin opener policy=] given |navigable|'s [=active browsing context=], |response|'s [=response/URL=], |responseOrigin|, |responseCOOP|, |coopEnforcementResult|, and |exchangeRecord|'s [=exchange record/request=]'s [=request/referrer=].
1. Set |coopEnforcementResult| to the result of [=enforcing a response's cross-origin opener policy=] given |navigable|'s [=active browsing context=], |redirectChainResponse|'s [=response/URL=], |responseOrigin|, |responseCOOP|, |coopEnforcementResult|, and |exchangeRecord|'s [=exchange record/request=]'s [=request/referrer=].
1. Set |request|'s [=request/URL list=] to |urlList|.
1. <span id="prefetch-activation-client-creation"></span>Set |request|'s [=request/reserved client=] to the result of [=creating a reserved client=] given |navigable| and |request|'s [=request/URL=].
Copy link
Contributor

Choose a reason for hiding this comment

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

|request|'s [=request/URL=] here after Set |request|'s [=request/URL list=] to |urlList|. just above points to the last redirect leg's URL (in this PR last redirect leg's response URL per #386), and thus doesn't fix #350 -- it's just because the |request|'s [=request/URL=] reference timing is moved here in the recent commit.

But, if we reversing the order (i.e. doing this line BEFORE URL list update), we'll use the initial request URL, even it was redirected.

Maybe it's better to fix #350 separately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ugh, you're right. That's frustrating. OK, let's fix it separately.


Copy link
Contributor

@hiroshige-g hiroshige-g Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Memo: also, the creation URL of the reserved client and the last request URL are:

  • Not always the same in the spec (the creation URL is not updated on same-origin redirects?)
  • Same in Chromium impl

And thus the creation URL can be mismatched between navigation and prefetch, even without #350 #386.

Anyway I'll investigate a little more and file spec issues, and let's fix that later.

<p class="note">This will be the [=environment=] used for constructing the resulting {{Document}} and [=environment settings object=]. We need a separate [=environment=] for each time a [=prefetch record=] is consumed, instead of using |record|'s [=prefetch record/redirect chain=]'s last item's [=exchange record/request=]'s [=request/reserved client=], because otherwise reusing the same prefetch record for multiple [=navigation params=] (i.e., multiple navigations) would cause the created [=environment settings objects=] to share an [=environment/id=]. This would then be visible, e.g., through the {{Clients/get()|clients.get()}} API, in a very confusing way.
1. Set |request|'s [=request/reserved client=]'s [=environment/active service worker=] to |activeServiceWorker|.
1. Let |resultPolicyContainer| be the result of [=determining navigation params policy container=] given |record|'s [=prefetch record/response=]'s [=response/URL=], |documentState|'s [=document state/history policy container=], |sourceSnapshotParams|'s [=source snapshot params/source policy container=], null, and |responsePolicyContainer|.
1. Let |response| be |record|'s [=prefetch record/response=].
1. Optionally, set |response| to a [=response/clone=] of |response|.

<p class="note">An implementation might wish to do this if it believes that the prefetch will be consumed more than once. For example, if in the future the response is consumed by a prerender, that [=prerendering traversable=] might be [=destroy a top-level traversable|destroyed=] through various means. Normally that would mean the response is discarded, but if the implementation performs this step, then the prefetched response will still be available to serve a future navigation. [[PRERENDERING-REVAMPED]]
1. If the user agent did not perform the previous optional step, then it must [=list/remove=] |record| from |navigable|'s [=navigable/active document=]'s [=Document/prefetch records=].


1. Return a new [=navigation params=], with:
: [=navigation params/id=]
:: |navigationId|
Expand Down Expand Up @@ -431,6 +436,11 @@ A <dfn export>cross-origin prefetch IP anonymization policy</dfn> has an <dfn ex

This section contains patches to [[HTML]].

Add an additional item to [=environment=] as follows:

: <dfn for="environment">is navigational prefetch client</dfn>
:: a [=boolean=] (default false)

Add an additional item to [=navigation params=] as follows:
: <dfn for="navigation params">delivery type</dfn>
:: a [=string=] (corresponding to {{PerformanceResourceTiming}} [=PerformanceResourceTiming/delivery type=])
Expand Down Expand Up @@ -585,7 +595,9 @@ Modify the [=snapshot source snapshot params=] algorithm to set the return value
1. Run the [=environment discarding steps=] for |request|'s [=request/reserved client=].
1. Set |request|'s [=request/reserved client=] to null.
1. Set |commitEarlyHints| to null.
1. If |request|'s [=request/reserved client=] is null, then set |request|'s [=request/reserved client=] to the result of [=creating a reserved client=] given |navigable|, |currentURL| and |isolationOrigin|.
1. If |request|'s [=request/reserved client=] is null, then:
1. Set |request|'s [=request/reserved client=] to the result of [=creating a reserved client=] given |navigable|, |currentURL| and |isolationOrigin|.
1. If |prefetchRecord| was given, then set |request|'s [=request/reserved client=]'s [=environment/is navigational prefetch client=] to true.
1. If the result of [=should navigation request of type be blocked by Content Security Policy?=] given |request| and |cspNavigationType| is "`Blocked`", then set |response| to a [=network error=] and [=iteration/break=]. [[CSP]]
1. If |prefetchRecord| was given, then:
1. Let |purpose| be a [=structured header/List=] containing the [=structured header/Token=] `prefetch`.
Expand Down Expand Up @@ -719,6 +731,20 @@ This section contains patches to [[NAVIGATION-TIMING]].
Add an additional parameter to [=create the navigation timing entry=], which is a [=string=] <var ignore>deliveryType</var>, and pass it as an additional argument to [=setup the resource timing entry=].
</div>

<h2 id="service-workers-patches">Service Workers Patches</h2>

This section contains patches to [[SERVICE-WORKERS]].

<div algorithm="Create Fetch Event and Dispatch">
Modify <a spec="SERVICE-WORKERS">Create Fetch Event and Dispatch</a>'s step which sets {{FetchEvent/resultingClientId}} as follows:

<blockquote>
If |request| is a [=non-subresource request=] and |request|'s [=request/destination=] is not "{{RequestDestination/report}}"<ins> and |reservedClient|'s [=environment/is navigational prefetch client=] is false</ins>, initialize <var ignore>e</var>'s {{FetchEvent/resultingClientId}} attribute to |reservedClient|'s [=environment/id=], and to the empty string otherwise.
</blockquote>

<p class="note">In the prefetch case, although |request| has a [=request/reserved client=] at this time, it is just a specification fiction to help the request go through. It does not represent an actual client, of the sort that can be seen in e.g. {{Clients/matchAll()|clients.matchAll()}}. Actual client creation happens <a href="#prefetch-activation-client-creation">at activation time</a>, which will be after the {{FetchEvent}} is done firing.
</div>

<h2 id="clear-site-data-patches">Clear Site Data patches</h2>

Add an additional header value description in [[CLEAR-SITE-DATA#header]]:
Expand Down Expand Up @@ -752,8 +778,6 @@ Modify <a abstract-op spec="CLEAR-SITE-DATA">clear site data for response</a>'s

<h2 id="prefetch-algorithms">Prefetch algorithms</h2>

<p class="issue">Check Service Worker integration</p>

The <dfn>list of sufficiently strict speculative navigation referrer policies</dfn> is a list containing the following: "", "`strict-origin-when-cross-origin`", "`strict-origin`", "`same-origin`", "`no-referrer`".

<div algorithm>
Expand Down