WICG / service-worker-static-routing-api

A repository for the ServiceWorker static routing API.
Other
24 stars 6 forks source link

Consider timingInfo handling in static routing API #19

Open sisidovski opened 7 months ago

sisidovski commented 7 months ago

In https://www.w3.org/TR/service-workers/#handle-fetch, timingInfo is set with start time, fetch event dispatch time appropriately. As the static routing API change the flow of the handle fetch algorithm, we have to define when/what value should be set to timingInfo.

Spec update to the ServiceWorker and WPTs are needed.

yoshisatoyanagisawa commented 7 months ago

In current Chromium implementation, if "cache" source is used, the timing is set like: https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_cache_storage_matcher.cc;l=118;drc=c686e8f4fd379312469fe018f5c390e9c8f20d0d I just put random timing info to adjust to the DidDispatchFetchEvent interface.

However, we should revisit what information is set there and given as the timing Info. There might be three options: Option A: giving new meanings to existing timing Info fields. Option B: new timing Info field is defined for the static routing API. Option C: no timing Info are provided for the static routing API.

My recommendation is Option B. No implementation is needed for Option C, but developers lose the timing Information when the API is used, which sounds not good when the API is widely used. Option A can be easy to implement, but developers may want to know more details on the route. Option A cannot fulfill that. Option B may bring dedicated information for the static routing API, and fix concerns for Option A and C. Drawback should be design and implementation cost.

domenic commented 7 months ago

It looks like this impacts the two properties workerStart and fetchStart of the navigation timing entries.

Option B involves adding new properties, right? Do you have a proposal for those?

But even if you do option B, we still need to decide what values the existing properties get. Have you thought about what is preferred there? Answering this seems more important, as you can always add new properties later, after gathering more input from partners and the standards community.

To me, setting them both to 0 seems reasonable. (Or whatever default value falls out of the processing, if it is not 0.)

yoshisatoyanagisawa commented 7 months ago

Yes, I have drafted the proposal for Option B. I suggested to add 4 fields there. Since timing is got inside the Handle Fetch algorithm, I suggested to refer the Service Worker timing in the Resource timing.

Existing properties are left as-is. Values may set upon the source. For the cache source, since we do not call the Run Service Worker algorithm, workerStart is left 0. The new cache lookup related field will be filled.

As you know, I am working on drafting the Service Worker static routing specification (PR that gathers all changes, working branch). The proposal will be adjusted upon its progress.

sisidovski commented 7 months ago

To me, setting them both to 0 seems reasonable. (Or whatever default value falls out of the processing, if it is not 0.)

Agree. If the router source is network or cache, setting 0 is reasonable. If the source is race-network-and-fetch-handler, wondering what values are appropriate, especially when network wins the race. Should we set the actual values to workerStart and fetchStart, or we can just set 0 in that case?

yoshisatoyanagisawa commented 3 months ago

There is a github issue to discuss about this in the resource-timing repo. https://github.com/w3c/resource-timing/issues/389

yoshisatoyanagisawa commented 3 months ago

I suggest to continue the discussion in https://github.com/w3c/resource-timing/issues/389 instead.