New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

`fetch()` resources sometimes don't get added to the ResourceTiming timeline

Project Member Reported by y...@yoav.ws, Jan 23 2018

Issue description

Chrome Version: M66
OS: OSX 10.12.6

What steps will reproduce the problem?
(1) Run the attached file as a WPT

What is the expected result?
Test should pass

What happens instead?
dummy.xml is not added to the performance timeline, causing the test to timeout

This doesn't happen on simpler cases of `fetch()` (I do see simpler fetch calls get their resources added to the timeline).

This happens on Canary, but not in stable (M63), making me think this is a regression. 
 
single-download-preload.html
3.1 KB View Download

Comment 1 by npm@chromium.org, Jan 24 2018

Labels: Needs-Feedback
I assume you want this file to be located in the folder https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/preload/ correct?

I've tested with ToT and the test passes, so I wonder if this was just a short temporary regression that you happened to catch. Do you mind trying again on the next canary and reporting back?

If it still fails for you, then also let us know which operating system you're using to test.

Comment 2 by npm@chromium.org, Jan 24 2018

Oh actually this test is already in external/wpt/preload. I see that we should pass it since it should be running along with the other tests. There's only failure when running under the flag site-per-process: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process?q=site-per-process&dr=C&l=244

Comment 3 by y...@yoav.ws, Jan 25 2018

The test file attached here is slightly different from the one in the WPT repo (the one in the repo is based on a timeout, while this one is based on PerfObserver)

Comment 4 by y...@yoav.ws, Jan 25 2018

I see the element in the timeline in latest canary, so it is possible it was a temporary regression.

Comment 5 by ricea@chromium.org, Jan 30 2018

Cc: ricea@chromium.org
I'm confused: the attached file appears identical to the wpt version:

b600864fe95b5055b82f26e52243ed77  single-download-preload-805069.html
b600864fe95b5055b82f26e52243ed77  single-download-preload.html

Also, it doesn't use fetch(), it uses XMLHttpRequest.

Comment 6 by y...@yoav.ws, Jan 30 2018

Apologies, I'm the confused one. I uploaded (and re-tested) with the wrong version of the file. I now uploaded the right one, on which I still see the issue in Canary (and which passes the test fine in stable)
single-download-preload.html
3.7 KB View Download

Comment 7 by ricea@chromium.org, Jan 30 2018

Labels: -Needs-Feedback
Oh, I like this new version of the test a lot more than the old one. And I think I see the problem.

Since https://chromium-review.googlesource.com/c/chromium/src/+/861695 it's necessary to explicitly read the body from a fetch() in order for it to be considered complete. This is only visible via side-channels: I wasn't sure if ResourceTiming would be one of them.

This was always the case for fetches with a "Cache-Control: no-store" header, but it's more much noticeable now. AFAIK this behaviour is correct (and the old behaviour is wrong).

I probably have to revert https://chromium-review.googlesource.com/c/chromium/src/+/861695 because of the performance regression in  issue 804320 , but I hope to reinstate it once we have the performance issue.

The way to fix the test to explicitly fetch the body, for example by using the wonderful construct:

  await (await fetch()).arrayBuffer()

Comment 8 by y...@yoav.ws, Jan 30 2018

Thanks for looking into this! :)

That behavior (at least when it comes to ResourceTiming) seems confusing. Could you point me to the parts of the Fetch spec where this is defined?

Comment 9 by ricea@chromium.org, Jan 30 2018

Owner: yhirano@chromium.org
Status: Assigned (was: Untriaged)
Actually the standard is more complicated than I realised. I will leave the in-depth analysis to yhirano@, but based on this:

https://fetch.spec.whatwg.org/#concept-fetch-suspend

the old behaviour may have been acceptable. The new behaviour possibly violates "The user agent should ignore the suspension request if the ongoing fetch is updating the response in the HTTP cache for the request" but that's not a very good match for how the disk cache works in Chrome.

I think based on https://www.w3.org/TR/resource-timing-2/#step-response-end our behaviour is probably not standards compliant. My interpretation is that the PerformanceResourceTiming object is supposed to be queued as soon as the network stack receives the last byte, whereas we are queuing it when Blink accepts the last byte. This distinction didn't make much difference as long as Blink didn't apply backpressure.

Assigning to yhirano@ for a quick assessment of

1) What the Fetch Standard actually says about this, and
2) Whether we can straightforwardly change resource timing to be reported when the browser sends the last byte, rather than when the render process acknowledges it.

yhirano@, feel free to send it back if it looks like it will take a long time.

There may be sites depending on the order we currently fire events which would be broken if we changed this.
ping for getting stale.
Owner: domenic@chromium.org
This is basically WAI. Fetch is not guaranteed to happen if the result will not be used. See https://fetch.spec.whatwg.org/#garbage-collection.

Temporarily assigning to domenic@ to get his opinion on the interaction of that section with resource timing APIs.
Cc: annevank...@gmail.com
Owner: ----
Status: Available (was: Assigned)
I agree this is WAI, although there may be an interesting standards discussion for someone to have.

Basically, I think it's in the best interest of the browser to be able to terminate fetches that nobody will consume. The resource timing APIs should not change this; IIUC, they are meant to observe fetches, not to modify them or act as some sort of signal that the user is interested.

I think we may want to update Fetch to be clearer that resource timing specifically does not count as "observable through script". But I don't think we should change how this works in the specs or implementation.
Doesn't that mean that you can use resource timing to observe GC?
Filed https://github.com/w3c/resource-timing/issues/161 to ask for spec clarification.

Sign in to add a comment