Editorial: queue a task to reject promise in InstallEvent.addRoutes - #1839
Merged
Conversation
The rejection path in the [[service worker queue]] block was resolving/rejecting a JS promise directly from a parallel queue, which violates the same pattern that w3c#1740 flags. Wrap it in a Queue a task on the service worker event loop, and abort further steps so we don't also set the rules and resolve the same promise afterwards. Refs: w3c#1740
monica-ch
marked this pull request as draft
July 31, 2026 22:23
monica-ch
marked this pull request as ready for review
August 4, 2026 00:10
Collaborator
Author
|
@yoshisatoyanagisawa Can you also review this when you get a chance! |
yoshisatoyanagisawa
approved these changes
Aug 4, 2026
yoshisatoyanagisawa
left a comment
Collaborator
There was a problem hiding this comment.
good catch!
github-actions Bot
added a commit
to asleekgeek/ServiceWorker
that referenced
this pull request
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a small follow-up related to #1740.
The rejection path inside the
[[service worker queue]]block ofInstallEvent.addRoutes()was rejecting |promise| directly from inside the [[service worker queue]] (a parallel queue)". Rejecting a JS promise from a parallel queue crosses the event-loop/parallel boundary and is the same pattern that #1740 flags across the spec.This PR:
Queue a taskon the service worker's event loop, using the DOM manipulation task source, matching the existing resolve path a few lines below.Abort these steps.after the rejection queue so we don't fall through toSet |serviceWorker|'s list of router rulesand the follow-up resolve on the same promise.Let |serviceWorkerEventLoop| be the current global object's event loop.above the[=queue/Enqueue=]block so both the reject and resolve tasks can reference it (and so the event loop is captured on the event-loop side, not inside the parallel queue).Editorial only, no normative behavior change beyond making the failure path stop after rejecting (which it should already do — the previous text would set the rules and resolve after rejecting, which was a bug in the failure path).
This is a follow-up spun out of #1755, InstallEvent.addRoutes had the same reject-in-parallel bug, but sits outside the sections covered by any of those.
Refs: #1740
Preview | Diff