Skip to content

Add race-network-and-cache source to static routing api. #1764

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add race-network-and-cache source to static routing api. #1764

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
778bf3b
add race-network-and-cache
monica-ch Apr 16, 2025
50af98b
minor format fixes
monica-ch Apr 16, 2025
5b14fcd
update failure cases
monica-ch Apr 17, 2025
b69c7da
Follow spec format
monica-ch Apr 20, 2025
8b8c331
use helper algorithm
monica-ch Apr 20, 2025
3305a7f
use |raceResponse| when empty
monica-ch Apr 22, 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
49 changes: 46 additions & 3 deletions docs/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/

A {{ServiceWorkerGlobalScope}} object has an associated <dfn for="ServiceWorkerGlobalScope">race response map</dfn> which is an [=ordered map=] where the [=map/keys=] are [=/requests=] and the [=map/values=] are [=race response=].

A <dfn id="dfn-race-response">race response</dfn> is a [=struct=] used to contain the network response when {{RouterSourceEnum/"race-network-and-fetch-handler"}} performs. It has a <dfn for="race response">value</dfn>, which is a [=/response=], "<code>pending</code>", or null.
A <dfn id="dfn-race-response">race response</dfn> is a [=struct=] used to contain the network response when {{RouterSourceEnum/"race-network-and-fetch-handler"}} or {{RouterSourceEnum/"race-network-and-cache"}} performs. It has a <dfn for="race response">value</dfn>, which is a [=/response=], "<code>pending</code>", or null.

Note: {{ServiceWorkerGlobalScope}} object provides generic, event-driven, time-limited script execution contexts that run at an origin. Once successfully <a>registered</a>, a [=/service worker=] is started, kept alive and killed by their relationship to events, not [=/service worker clients=]. Any type of synchronous requests must not be initiated inside of a [=/service worker=].

Expand Down Expand Up @@ -1591,14 +1591,16 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/

dictionary RouterSourceDict {
DOMString cacheName;
DOMString raceNetworkAndCacheCacheName;
};

enum RunningStatus { "running", "not-running" };
enum RouterSourceEnum {
"cache",
"fetch-event",
"network",
"race-network-and-fetch-handler"
"race-network-and-fetch-handler",
"race-network-and-cache"
};
</pre>

Expand Down Expand Up @@ -3242,12 +3244,15 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
1. Let |queue| be an empty [=queue=] of [=/response=].
1. Let |raceFetchController| be null.
1. Let |raceResponse| be a [=race response=] whose [=race response/value=] is "<code>pending</code>".
1. Let |networkFetchCompleted| be false.
1. Let |fetchHandlerCompleted| be false.
1. Run the following substeps [=in parallel=], but [=abort when=] |fetchController|'s [=fetch controller/state=] is "<code>terminated</code>" or "<code>aborted</code>":
1. Set |raceFetchController| to the result of calling [=fetch=] given |request|, with [=fetch/processResponse=] set to the following steps given a [=/response=] |raceNetworkRequestResponse|:
1. If |raceNetworkRequestResponse|'s [=response/status=] is [=ok status=], then:
1. Set |raceResponse|'s [=race response/value=] to |raceNetworkRequestResponse|.
1. [=queue/Enqueue=] |raceNetworkRequestResponse| to |queue|.
1. Otherwise, set |raceResponse|'s [=race response/value=] to a [=network error=].
1. Set |networkFetchCompleted| to true.
1. [=If aborted=] and |raceFetchController| is not null, then:
1. [=fetch controller/Abort=] |raceFetchController|.
1. Set |raceResponse| to a [=race response=] whose [=race response/value=] is null.
Expand All @@ -3256,7 +3261,45 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
1. Let |fetchHandlerResponse| be the result of [=Create Fetch Event and Dispatch=] with |request|, |registration|, |useHighResPerformanceTimers|, |timingInfo|, |workerRealm|, |reservedClient|, |preloadResponse|, and |raceResponse|.
1. If |fetchHandlerResponse| is not null and not a [=network error=], and |raceFetchController| is not null, [=fetch controller/abort=] |raceFetchController|.
1. [=queue/Enqueue=] |fetchHandlerResponse| to |queue|.
1. Wait until |queue| is not empty.
1. Set |fetchHandlerCompleted| to true.
1. Wait until |queue| is not empty or (|networkFetchCompleted| is true and |fetchHandlerCompleted| is true).
Copy link
Collaborator

Choose a reason for hiding this comment

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

@domenic Can I ask your advice on how this kinds of thing is usually written in the spec?
The explanation sounds reasonable as code, but I am afraid it is not usual way of writing spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you concerned mainly about the parentheses? I think they are OK since they are easy to understand, but yes, it is unusual. The pattern I would use would be:

Wait until any of the following are true:

  • |queue| is not empty; or
  • |networkFetchCompleted| is true and |fetchHandlerCompleted| is true.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I wanted to know how people usually write this because I have not seen specs using parentheses.
And, thank you for the advice.

@monica-ch Will you revise the sentences as @domenic's way?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we really really need to introduce networkFetchCompleted and fetchHandlerCompleted. For race-network-and-fetch-handler, the race is expressed by queue. Why we need these two flags?

It looks networkFetchCompleted doesn't wait for the promise resolve. So assuming queue can be still empty even when networkFetchCompleted is true. This will return null, which means the fetch event fallback in the handle fetch algorithm.

Also not sure why we should wait for fetchHandlerCompleted as well. This means we have to wait for both the network and fetch handler completion.

IIUC race-network-and-fetch-handler won't run the race anymore after this change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This change should be due to #1764 (comment).
I concerned that if both network and cache failed, nothing is enqueued, and waiting for queue won't finish. I stepped back and saw the race-network-and-fetch-handler, and felt it may also have the same issue.
Having the way to know network and cache lookup completion should be needed to avoid the infinite queue waiting.

1. If |queue| is empty, return null.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to reuse raceResponse if possible?
The reason of making [=Create Fetch Event and Dispatch=] to take [=raceResponse=] is that we wanted to avoid sending a duplicated request to the server. If we follow that thought, it might be reasonable to reuse network error on both complete but queue empty case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated, does it looks better?

1. Return the result of [=dequeue=] |queue|.
1. If |source| is {{RouterSourceEnum/"race-network-and-cache"}}, and |request|'s [=request/method=] is \`<code>GET</code>\`, then:
1. If |shouldSoftUpdate| is true, then [=in parallel=] run the [=Soft Update=] algorithm with |registration|.
1. Let |queue| be an empty [=queue=] of [=/response=].
1. Let |raceFetchController| be null.
1. Let |raceResponse| be a [=race response=] whose [=race response/value=] is "<code>pending</code>".
1. Let |networkFetchCompleted| be false.
1. Let |cacheLookupCompleted| be false.
1. Run the following substeps [=in parallel=], but [=abort when=] |fetchController|'s [=fetch controller/state=] is "<code>terminated</code>" or "<code>aborted</code>":
1. Set |raceFetchController| to the result of calling [=fetch=] given |request|, with [=fetch/processResponse=] set to the following steps given a [=/response=] |raceNetworkRequestResponse|:
1. If |raceNetworkRequestResponse|'s [=response/status=] is [=ok status=], then:
1. Set |raceResponse|'s [=race response/value=] to |raceNetworkRequestResponse|.
1. [=queue/Enqueue=] |raceNetworkRequestResponse| to |queue|.
1. Otherwise, set |raceResponse|'s [=race response/value=] to a [=network error=].
1. Set |networkFetchCompleted| to true.
1. [=If aborted=] and |raceFetchController| is not null, then:
1. If |raceFetchController|'s [=fetch controller/state=] is not "<code>terminated</code>", [=fetch controller/abort=] |raceFetchController|.
1. Set |raceResponse| to a [=race response=] whose [=race response/value=] is null.
1. Resolve |preloadResponse| with undefined.
1. Run the following substeps [=in parallel=]:
1. [=map/For each=] |cacheName| → |cache| of the |registration|'s [=service worker registration/storage key=]'s [=name to cache map=]:
1. If |source|["{{RouterSourceDict/raceNetworkAndCacheCacheName}}"] [=map/exists=] and |source|["{{RouterSourceDict/raceNetworkAndCacheCacheName}}"] [=string/is=] not |cacheName|, [=continue=].
1. Let |requestResponses| be the result of running [=Query Cache=] with |request|, a new {{CacheQueryOptions}}, and |cache|.
1. Let |globalObject| be null.
1. If |requestResponses| is not an empty [=list=], then:
1. Let |requestResponse| be the first element of |requestResponses|.
1. Let |response| be |requestResponse|'s response.
1. If |response|'s [=response/type=] is "`opaque`", and |globalObject| is null:
1. Set |globalObject| to the result of running [=Setup ServiceWorkerGlobalScope=] with |activeWorker|.
1. If |globalObject| is null, [=continue=].
1. If |response|'s [=response/type=] is "`opaque`", and [=cross-origin resource policy check=] with |globalObject|'s [=environment settings object/origin=], |globalObject|, "", and |response|'s [=filtered response/internal response=] returns <b>blocked</b>, then [=continue=].
1. [=queue/Enqueue=] |response| to |queue|.
1. If |raceFetchController| is not null, [=fetch controller/abort=] |raceFetchController|.
1. Set |cacheLookupCompleted| to true.
1. Wait until |queue| is not empty or (|networkFetchCompleted| is true and |cacheLookupCompleted| is true).
1. If |queue| is empty, return null.
1. Return the result of [=dequeue=] |queue|.
1. Assert: |source| is "{{RouterSourceEnum/fetch-event}}"
1. If |request| is a [=navigation request=], |registration|'s [=navigation preload enabled flag=] is set, |request|'s [=request/method=] is \`<code>GET</code>\`, |registration|'s [=active worker=]'s [=set of event types to handle=] [=set/contains=] <code>fetch</code>, and |registration|'s [=active worker=]'s [=all fetch listeners are empty flag=] is not set then:
Expand Down