Update HTTPFetch algorithm to support new SW Handle Fetch returns #1832
Conversation
|
This PR is for the resource timing API changes of ServiceWorker static routing API. Since some of the criterias are not met yet, let me make this as draft. |
|
Let me make this a PR and ask for opinions about this. |
|
@yoshisatoyanagisawa Could you check if this make sense? Thanks! |
fetch.bs
Outdated
| <li><p>Set <var>fetchParams</var>'s <a for="fetch params">timing info</a>'s | ||
| <a for="fetch timing info">final service worker start time</a> to | ||
| <var>serviceWorkerStartTime</var>. | ||
| <li><p>Set <var>fetchParams</var>'s <a for="fetch params">timing info</a>'s |
There was a problem hiding this comment.
It is not a blocker but resource-timing may not not have a trivial way to know response, right? I just wondered why we need to copy the timing info.
There was a problem hiding this comment.
I have to double check, but I think you are right. We already set the timing info directly to fetch timing info, so maybe we do not need to set them to the response.
There was a problem hiding this comment.
Let me double check and update the draft
There was a problem hiding this comment.
Just to clarify, I think this is copying the response's timing_info's service worker timing info into the fetchParams timing info's service worker timing info, which does not expose response, so I feel like this is right.
There was a problem hiding this comment.
Will it be used for the redirect?
I revisited this and see how fetchParams are used, and it looks used "If internalResponse’s status is a redirect status:".
If so, lgtm.
|
@annevk Thanks for taking a look offline! Could you publish the comments you made during our offline chat at TPAC last week? Thanks! |
annevk
left a comment
annevk
left a comment
There was a problem hiding this comment.
Oh, I thought I had. Fortunately GitHub did preserve them. Sorry about that!
This will update the HttpFetch algorithm, particularly the handling of the response for Service Worker fetch (`handle fetch` step). Currently, the response of handle fetch step is assumed to return the `response` type (or null, if the ServiceWorker couldn't handle the fetch and need to fallback to the network request). However, we have changed the `handle fetch` step to also return `service worker timing info` when the ServiceWorker static routing API used, so that the corresponding timing information are correctly exposed when the ServiceWorker could not handle the fetch. To support this new return type, we need to update the handling of the response of `handle fetch`. To expose the Service Worker Timing Info to the resource timing API, we also associate them to the Fetch Timing Info so that it could be later referenced.
annevk
force-pushed
the
resource-timing/01/expose-service-worker-timing
branch
from
4b9cee0 to
025ab33
Compare
|
@quasi-mod I decided to take a pass at the formatting myself since it's your first contribution. While doing that I ended up fixing a couple of minor errors and also realized that the non-null check is redundant with the type checks. The result is a much cleaner diff. Could you please verify that all my changes were indeed editorial? (Note that I forced pushed to your branch to also rebase it so if you want to make further changes please be aware of that.) I'd also like to hear back from @yoshisatoyanagisawa about the unresolved thread above. |
3 participants
This will update the HttpFetch algorithm, particularly the handling of the response for Service Worker fetch (
handle fetchstep). Currently, the response of handle fetch step is assumed to return theresponsetype (or null, if the ServiceWorker couldn't handle the fetch and need to fallback to the network request). However, we have changed thehandle fetchstep to also returnservice worker timing infowhen the ServiceWorker static routing API used, so that the corresponding timing information are correctly exposed when the ServiceWorker could not handle the fetch. To support this new return type, we need to update the handling of the response ofhandle fetch.To expose the Service Worker Timing Info to the resource timing API, we also associate them to the Fetch Timing Info so that it could be later referenced.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff