navigation-timing icon indicating copy to clipboard operation
navigation-timing copied to clipboard

What should transferSize, encodedBodySize and decodedBodySize in Navigation + Resource Timing represent for responses where a Service Worker is involved?

Open abhishekcghosh opened this issue 4 years ago • 10 comments

The specifications for Navigation Timing API 2 or Resource Timing API 2 does not make it explicitly clear what is the definition or the expected value to be observed for transferSize, encodedBodySize and decodedBodySize parameters in both Navigation + Resource Timing APIs for responses that have been synthesised using a Service Worker.

The expected values are not clear either, considering how request interception through Service Workers opens up possibilities of zero-to-many fanout of actual network requests and subsequent response synthesis by combining resources from network and/or cache in any number of ways.

Should a page document (the service worker client) be aware of the actual network transfer costs for a request originating from it which can cause zero, one or more actual network request(s) based on Service Worker logic, or should it simply be opaque to all the Service Worker magic and consider whatever it received to be coming as if from the network? Sadly, none of these behaviour definitions has made clear sense to me so far, and there must be a better way of defining the behaviour here.

Current implementation on latest builds of a number of popular browsers (Chrome, Chromium, Firefox, Edge, Samsung Internet) seem to allude to this fact by simply reporting a "0" for all these parameters when a Service Worker is involved in between. A few examples of use-cases include:

  1. encodedBodySize or decodedBodySize for a resource that passes the timing-allow-check algorithm should not logically return 0, because there is a payload for those resources.
  2. Cache hit/miss checks based on the value of transferSize parameter is also thrown off in all these cases because it always returns 0 even if the timing-allow-check algorithm passes.

In effect, this means that today any website which uses a Service Worker (a large number of website do) will not be able to use certain important capabilities of the Navigation Timing or Resource Timing APIs correctly, which is a sad trade-off considering practical use-cases of website latency/performance improvements and metrics collection (in the process of doing so) can often tie website owners to a need of having to use these things together!

abhishekcghosh avatar Mar 10 '20 07:03 abhishekcghosh

I think "0" makes clear sense for transferSize for a synthetic Response or cache_storage Response. It seems we could populate a real transfer size for a Response returned from fetch(url).

I could see the case for making decodedBodySize match the body size of the response.  All response data coming from the service worker is always decoded, so it would basically be the length of the complete body.

The encodedBodySize could again come from the Response. If the Response is from fetch() it would get the wire encoded size. Otherwise the size would match decodedBodySize.

Anyway, that's all just my opinion.

@yoavweiss do you know if there is a reason to keep these as 0? For example, do measurement frameworks assume they are not populated for service workers in order to infer service worker existence?

wanderview avatar Jun 26 '20 15:06 wanderview

I'm not aware of a concrete reason. What you outlined above makes sense to me. For semi-synthetic responses, or responses that are comprised of multiple Response objects (is that a thing?), can we trace that to fill in the right values?

yoavweiss avatar Jul 03 '20 09:07 yoavweiss

For semi-synthetic responses, or responses that are comprised of multiple Response objects (is that a thing?), can we trace that to fill in the right values?

Not sure what this means. If its the output of a Response.clone() we could trace it through. If its a new Response(response.body, response) then we can't really trace it since the body is just a ReadableStream without any extra metadata on it. If you want multiple sources stitched together you have to do that in js and we definitely cannot automatically trace.

wanderview avatar Jul 13 '20 21:07 wanderview

Hi all,

I'm interested in fixing this isssue in Chromium, and want to cofirm what other implementors think before starting implementation.

So far, I think what @wanderview mentioned in https://github.com/w3c/navigation-timing/issues/124#issuecomment-650227989 makes sense. Regarding semi-synthetic response case, as @wanderview mentioned, Response.clone() could be tracked, but I guess other type of responses can be considered as synthetic response because it's manufactured by new Response(...).

In terms of spec change, some tweaks in the spec for encodedBodySize and decodedBodySize to cover the Handle Fetch case seem to be needed. Does @yoavweiss have thoughts of how to introduce the service worker's integration?

makotoshimazu avatar Oct 22 '20 04:10 makotoshimazu

I agree with @wanderview too.

In terms of encoded/decoded body size, it feels like we should use the transmitted and total bytes stored in the body - https://fetch.spec.whatwg.org/#concept-body-transmitted. Rather confusingly, body transmitted bytes seems to map to encodedBodySize rather than transferSize

Cases where encodedBodySize will take content encoding into account:

  • respondWith(fetch(url))
  • respondWith(fetch(url).then(r => r.clone())) - this may require some changes to https://fetch.spec.whatwg.org/#concept-body-clone to make this clear.
  • respondWith(fetch(url).then(r => new Reponse(r.body)))

If the body is constructed from a new ReadableStream, then encodedBodySize becomes the same as decodedBodySize. Calling response.body.tee()[0] uses normal stream teeing, so encodedBodySize becomes the same as decodedBodySize.

Edit: I got the rest wrong because I thought transfer size included the request.

Unfortunately, the fetch spec doesn't currently track total transfer size. This will need to be stored initially on the request (counting bytes for headers and such), then that number should be passed to the response, which adds on its own bytes from headers, and encodedBodySize.

Cases where transfer size should be retained:

  • respondWith(fetch(url))
  • respondWith(fetch(url).then(r => r.clone()))

Since this data should be stored on the response, it will be lost in methods that only copy the body, such as respondWith(fetch(url).then(r => new Reponse(r.body))).

jakearchibald avatar Nov 13 '20 13:11 jakearchibald

I also agree with @wanderview.

@jakearchibald, I don't understand what you're referring to in "This will need to be stored initially on the request (counting bytes for headers and such)", can you clarify in what situation request headers would be part of the response size?

asutherland avatar Nov 20 '20 02:11 asutherland

@asutherland ohh, I think I got confused by the stuff about redirects and HTTP overhead. If it's purely related to the response, that makes things much easier.

jakearchibald avatar Nov 20 '20 13:11 jakearchibald

(belated) Notes from Service Worker WG meeting on Nov 20:

  • @yoavweiss to update the issues with what was discussed.
  • It's unclear about whether these can be specified before or after full Fetch and Timing spec integration.

mfalken avatar Jan 22 '21 06:01 mfalken

Added a few failing tests for this: https://github.com/web-platform-tests/wpt/pull/33283 Will submit once the Fetch PR is also submitted.

noamr avatar Mar 21 '22 14:03 noamr

I updated the fetch spec in this PR to account for encoded/decoded body size for constructed responses. Thoughts welcome there.

noamr avatar Jun 19 '22 09:06 noamr

This is fixed now.

noamr avatar Jan 01 '23 13:01 noamr