Skip to content

Add new fields and getter steps for static routing resource timing #1769

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.

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 13 commits into
base: main
Choose a base branch
from

Conversation

quasi-mod
Copy link
Contributor

@quasi-mod quasi-mod commented May 7, 2025

This PR adds the new fields for the static routing API and the getter steps. This will be later exposed to the resource timing API. It has been discussed in w3c/resource-timing#389.


Preview | Diff

@quasi-mod quasi-mod marked this pull request as ready for review May 7, 2025 07:45
@quasi-mod
Copy link
Contributor Author

This is still a draft PR for the resource timing changes. Let me make it open to get the previews

@quasi-mod
Copy link
Contributor Author

quasi-mod commented May 8, 2025

@domenic @yoshisatoyanagisawa @sisidovski Could you take a look and see if this change makes sense? Thanks!
(Especially around race, as I have changed to queue the source as well using tuple)

@yoshisatoyanagisawa
Copy link
Collaborator

The direction looks good to me. I left some comments on the step order and a data structure for queueing.

@quasi-mod
Copy link
Contributor Author

Let me update the doc a bit more

@quasi-mod
Copy link
Contributor Author

I think the PR is ready to be reviewed again. PTAL. Thanks!

@yoshisatoyanagisawa
Copy link
Collaborator

@asutherland @domenic @youennf FYI

1. Wait until |queue| is not empty.
1. Return the result of [=dequeue=] |queue|.
1. Let |result| be the result of [=dequeue=] |queue|.
1. Set |timingInfo|'s [=service worker timing info/worker final router source=] be set to |result|'s [=/used route=].
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit please use [=race result/used route=] instead.

@@ -2557,6 +2565,12 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/

Note: Keep this definition in sync with [=fetch a classic worker-imported script=].

A <dfn id="dfn-race-result">race result</dfn> is a [=tuple=] of a [=race result/race response=] and [=race result/used route=].
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please update existing [=race response=] to [=/race response=] to avoid confusion between [=/race response=] and [=race result/race response=]. Or, give a different name.

There is bikeshed warning like:

      LINE 1103:217: Multiple possible 'dfn' local refs for 'race response'.
      Randomly chose one of them; other instances might get a different random choice.
      [=race response=]
      LINE 3264:46: Multiple possible 'dfn' local refs for 'race response'.
      Randomly chose one of them; other instances might get a different random choice.
      [=race response=]
      LINE 3274:50: Multiple possible 'dfn' local refs for 'race response'.
      Randomly chose one of them; other instances might get a different random choice.
      [=race response=]
      LINE 3322:28: Multiple possible 'dfn' local refs for 'race response'.
      Randomly chose one of them; other instances might get a different random choice.
      [=race response=]

@@ -3278,6 +3300,7 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/

1. If |navigationPreloadResponse|'s [=response/type=] is "`error`", reject |preloadResponse| with a `TypeError` and terminate these substeps.
1. Associate |preloadResponseObject| with |navigationPreloadResponse|.
1. If |timingInfo|'s [=service worker timing info/worker final router source=]is set to {{RouterSourceEnum/"network"}}, Set |timingInfo|'s [=service worker timing info/worker final router source=] be set to {{RouterSourceEnum/"fetch-event"}}.
Copy link
Collaborator

@yoshisatoyanagisawa yoshisatoyanagisawa May 21, 2025

Choose a reason for hiding this comment

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

Can I ask why this line is written here? If this is for SW static routing API, I thought the final router source is set in L3284.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants