Skip to content
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

Allow the addRoutes() to be called during installing #1744

Merged
merged 14 commits into from
Feb 4, 2025
Merged

Allow the addRoutes() to be called during installing #1744

Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
494d7c6
Ensure the addRoutes() to be called within the install event
yoshisatoyanagisawa Jan 22, 2025
51242ff
Merge branch 'w3c:main' into lifetime
yoshisatoyanagisawa Jan 23, 2025
1f867a7
Update the step based on the current Chromium behavior
yoshisatoyanagisawa Jan 23, 2025
4556ac0
Minor fix on |promise| lifetime
yoshisatoyanagisawa Jan 23, 2025
7cd9483
Merge branch 'w3c:main' into lifetime
yoshisatoyanagisawa Jan 24, 2025
d3082ca
Update docs/index.bs
yoshisatoyanagisawa Jan 24, 2025
050f195
Clarify the update of rules is executed sequentially
yoshisatoyanagisawa Jan 24, 2025
f5fafc7
Avoid ServiceWorker failure on addRoutes() failure
yoshisatoyanagisawa Jan 24, 2025
60ea69b
Remove redundant "in parallel"
yoshisatoyanagisawa Jan 27, 2025
2c61b5b
Remove check on event's dispatch flag
yoshisatoyanagisawa Jan 27, 2025
7d434ca
Merge branch 'main' into lifetime
yoshisatoyanagisawa Jan 28, 2025
7ebfac4
Rewrite the step with a parallel queue
yoshisatoyanagisawa Jan 28, 2025
487292c
Resolve the promise in the service worker event loop
yoshisatoyanagisawa Jan 28, 2025
b8ab89e
Merge branch 'w3c:main' into lifetime
yoshisatoyanagisawa Jan 30, 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
30 changes: 24 additions & 6 deletions docs/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1605,15 +1605,33 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/

The <dfn method for="InstallEvent"><code>addRoutes(|rules|)</code></dfn> method steps are:

1. Let |serviceWorker| be the [=current global object=]'s associated [=ServiceWorkerGlobalScope/service worker=].
1. Let |routerRules| be a copy of |serviceWorker|'s [=list of router rules=].
1. Let |event| be [=this=].
1. If |event|'s [=dispatch flag=] is unset, [=throw=] an "{{InvalidStateError}}" {{DOMException}}.
1. If |rules| is a {{RouterRule}} dictionary, set |rules| to &#x00AB; |rules| &#x00BB;.
1. Let |serviceWorker| be the [=current global object=]'s associated [=ServiceWorkerGlobalScope/service worker=].
1. For each |rule| of |rules|:
1. If running the [=Verify Router Condition=] algorithm with |rule|["{{RouterRule/condition}}"] and |serviceWorker| returns false, return [=a promise rejected with=] a {{TypeError}}.
1. Append |rule| to |routerRules|.
1. If |routerRules| [=list/contains=] a {{RouterRule}} whose {{RouterRule/source}} is "{{RouterSourceEnum/fetch-event}}" and |serviceWorker|'s [=set of event types to handle=] does not [=set/contain=] {{ServiceWorkerGlobalScope/fetch!!event}}, return [=a promise rejected with=] a {{TypeError}}.
1. Set |serviceWorker|'s [=service worker/list of router rules=] to |routerRules|.
1. Return [=a promise resolved with=] undefined.
1. If |rule|["{{RouterRule/source}}"] is "{{RouterSourceEnum/fetch-event}}" and |serviceWorker|'s [=set of event types to handle=] does not [=set/contain=] {{ServiceWorkerGlobalScope/fetch!!event}}, return [=a promise rejected with=] a {{TypeError}}.
1. Let |lifetimePromise| be a new [=promise=].
1. [=ExtendableEvent/Add lifetime promise=] |lifetimePromise| to |event|.

Note: {{InstallEvent/addRoutes(rules)|event.addRoutes(rules)}} extends the lifetime of the event by default as if {{ExtendableEvent/waitUntil()|event.waitUntil(promise)}} is called.

1. Let |promise| be a new [=promise=].
1. [=In parallel=]:
yoshisatoyanagisawa marked this conversation as resolved.
Show resolved Hide resolved
1. Upon [=upon fulfillment|fulfillment=] or [=upon rejection|rejection=] of |promise|, resolve |lifetimePromise| with undefined.

Note: this step is for making |lifetimePromise| always fullfilled to avoid the install event failure.

1. Let |serviceWorkerEventLoop| be the [=current global object=]'s [=event loop=].
1. [=Queue a task=] to run the following steps on |serviceWorkerEventLoop| using the [=DOM manipulation task source=]:
Copy link

Choose a reason for hiding this comment

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

This is not exactly what we want here.
|serviceWorker|'s list of router rules are used when matching a fetch, which happens out of process.
The usual way of handling this is to use [=in parallel=] to do manipulation out of process.
And we queue a task to actually go back to the service worker execution context.

This step could be replaced with something like:

1. [=In parallel], run the following substeps:

And then, at the end of these substeps:

1. [=Queue a task=] to run the following steps on |serviceWorkerEventLoop| using the [=DOM manipulation task source=]
    1. Resolve |promise| with undefined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me ask on this more. I got confused for this.

To allow third-party libraries to set routes, addRoutes() can be called multiple times. Also, to avoid ambiguity on the evaluation order, we want to force the rules to be evaluated in the added order.
Then, what happens if addRoutes() is called multiple times with the suggested process? I am afraid its execution order is not guaranteed.

Since it says [=in parallel=], I suppose one procedure can be executed while the other is executed. If multiple addRoutes() are called at once, there can be race condition and rules added at the same time may not be lost? That is my intention to make a task to do followings:

  1. get the existing rule.
  2. add the new rules.
  3. write back to |serviceWorker| [=list of router rules=]

Or, can we expect the deterministic execution order for [=in parallel=]?

Copy link

Choose a reason for hiding this comment

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

Right, we definitely want to preserve determinism.
The solution is probably to use a parallel queue, like https://w3c.github.io/webcodecs/#codec-work-queue, and use it.

That said, this is an existing issue in the service worker spec, see cache algorithms for instance.
I would go with a comment stating that it is expected that the order of the rules is preserved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I tried to revised it to use the parallel queue.

Copy link

Choose a reason for hiding this comment

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

If using the parallel queue, you need to queue a task on the service worker event loop to resolve the promise. We are not expected to resolve promises from a parallel queue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure.

1. Let |allRules| be a copy of |serviceWorker|'s [=list of router rules=].
1. For each |rule| of |rules|:
1. Append |rule| to |allRules|.

1. Set |serviceWorker|'s [=service worker/list of router rules=] to |allRules|.
1. Resolve |promise| with undefined.
1. Return |promise|.

</section>
</section>
Expand Down