search

web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
5k stars 3.1k forks source link

[resource-timing] Some tests assume synchronous performance entries #25650

Open emilio opened 4 years ago

emilio commented 4 years ago

And I don't think that's guaranteed?

All the callers of xhrScript seem to rely on the entry being available synchronously, which doesn't happen in Firefox's implementation and I don't think is guaranteed by the spec unless I'm missing something.

@yoavweiss you seemed to introduce these, do you know what would guarantee that that entry is available synchronously?

yoavweiss commented 4 years ago

Thanks for catching that. I don't think there are such guarantees. Seems like we'd need to rewrite these using PerformanceObserver (under the assumption that entries delivered to the observer were similarly delivered to the Performance Timeline).

/cc @npm1

npm1 commented 4 years ago

The spec is not very clear about this, as the processing model is written as if everything happens synchronously but it does not because it depends on the fetch result. I think it's fine for entries to be dispatched async, and we should modify the tests. PR is welcome, otherwise we can take a stab at it as part of the WebPerf hackathon which is happening Tuesdays for a couple of weeks!

yoavweiss commented 4 years ago

I tried to fix this and realized that the tests rely on sync XHR for a reason. All the tests that still use syncXHR after https://github.com/web-platform-tests/wpt/pull/25673 are testing what happens when buffering entries that are added while the buffer is full and before the resourcetimingbufferfull event is done.

Adding asynchronicity to those tests results in the event running in points where we don't want it to run, so we're not testing what needs to be tested. I removed dependence on sync addition of entries from other places where it was possible without making the test useless.