When is resourcetimingbufferfull supposed to fire?

[smaug----]

Per spec resourcetimingbufferfull fires synchronously when a PerformanceResourceTiming is added and the buffer is full.
When does that synchronous dispatch actually happen in case of sync XHR for example?
Does the resourcetimingbufferfull fire before or after XHR's readystatechange/load/loadend events?

[smaug----]

#95 is also related

[smaug----]

In workers the issue applies to importScripts, and there there is the question whether event fires before or after the script has run. I guess it needs to fire before, so that if a script initiates other network connections, resourcetimingbufferfull can have been handled already.

[igrigorik]

/cc @npm1 @tdresser

[npm1]

Since synchronous XHR is deprecated, I don't think it deserves a new special mention in the spec. That said, for other types the question is still valid. This sounds like a duplicate of #82 and the comments suggest that currently browsers do it before the load event of the resource. We will need to test it and spec it.

[smaug----]

synchronous XHR isn't deprecated in workers and resourcetimingbufferfull should fire also in workers.

[npm1]

I plan to add some comment saying that entries (and therefore the firing of resourcetimingbufferfull, when applicable) should be added before the load event of a resource. But I'm still confused regarding what needs to be added specifically for sync XHR requests. Notice that things like onreadystatechange should not be used with sync XHR requests https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/onreadystatechange

[igrigorik]

Does the resourcetimingbufferfull fire before or after XHR's readystatechange/load/loadend events?

Per discussion in #82 (comment), we don't make any guarantees on when RT entries are delivered — nor do we want to, I think. As such, resourcetimingbufferfull will fire.. sometime.

@smaug---- close with no action?

[smaug----]

The spec really must be more clear.
If entries are delivered async, then it must be defined to happen so, and if buffer then becomes full asynchronously too, it must be defined when the event fires. We can't leave the spec as it is, since it will lead to implementations do different things. Per spec firing resourcetimingbufferfull synchronously during XHR is ok.

@annevk may have some feedback on spec'ing this kind of stuff.

[annevk]

I think @smaug---- is correct. You need to say something about the timing. You can maybe say that a UA may wait and then queue a task at some point (although even that is questionable, imo, as it can easily lead to subtle bugs across differing implementations), but you cannot leave it so ambiguous that some can fire it synchronously and others not.

Part of the problem here is that this is monkey patching other specifications. Is there an issue already on actually grounding these objects/values in terms of primitives defined elsewhere?

[npm1]

web-platform-tests/wpt#9459 passing all tests (including allowed-failure ones). So entries are added by onload on all browsers.

[annevk]

That also tests they are added by the time you received all headers, no? So those tests are at odds with the statement by @igrigorik above about timing. And definitely none of that is integrated properly with Fetch / XHR.

[smaug----]

Those tests are for async XHR only, yet sync XHR is the more problematic case, I'd think.

[igrigorik]

FYI, we landed #146. Within that PR there was a related discussion to timing, see: #146 (comment).

You need to say something about the timing. You can maybe say that a UA may wait and then queue a task at some point (although even that is questionable, imo, as it can easily lead to subtle bugs across differing implementations), but you cannot leave it so ambiguous that some can fire it synchronously and others not.

@rniwa @npm1 anything we can do to help clarify the above?

[rniwa]

We always delay the event until the next task due to synchronous resource load that happens elsewhere (it's not safe to fire synchronous events in those cases). All new entires are queued in a secondary buffer until the event fires, and then inserted back to the buffer exposed to the scripts.

The fundamental problem here is that there is no clear definition as to when resources in images, fonts, etc... are loaded within a stylesheet, and a bunch of other places. As a result, our implementation is loading those resources at times that's not safe / correct to dispatch events in the document. In fact, there are some cases like the loading of module scripts which happens while resolving the module dependency, which are legitimately impossible to fire events in semantically sensible way.

As far as I can tell, firing this event asynchronously as we do in WebKit is the most expedient way to get to interoperability short of spec'ing the timing of each and every resource load that happens in the browser, and deciding which cases can fire this event synchronously.

[npm1]

Since it seems that implementations no longer do the same, we can't really hard-code something. I think resourcetimingbufferfull should mostly be used just to clear the buffer, or are there other important use cases? So maybe we can clarify that no entries should be lost as long as the buffer is cleared within resourcetimingbufferfull.

[rniwa]

That's an important invariant but I don't think that's clear enough definition. In particular, @smaug----'s original issue description was that we need to clarify whether the event fires before or after load event.

[npm1]

I don't see that where that is the question in this issue, but if we need to decide between before or after the load event then I'd say it definitely should be before: that has been the case most of the time for all implementations, and is still the case for most implementations.

This issue states that resourcetimingbufferfull fires synchronously and asks how that interacts with sync XHR events. And then similarly for importScripts. I'm just curious about how resourcetimingbufferfull is being used such that it's important to clarify the timing. Since we can't currently specify the precise timing, maybe we can at least give some guarantees about properties that must hold which are good enough to prevent problems in the use cases being considered.

[rniwa]

I don't see that where that is the question in this issue, but if we need to decide between before or after the load event then I'd say it definitely should be before: that has been the case most of the time for all implementations, and is still the case for most implementations.

We'd object to that definition since we literally can't implement such a behavior in WebKit.

[annevk]

Since we can't currently specify the precise timing

Why not? As @rniwa states there's a bunch of troublesome implications here and sites will come to rely on the differences. I really wish folks figured out the full architecture out first before adding new features to the platform.

[npm1]

I don't understand what the 'troublesome implications' are but I can agree on the problem of sites relying on certain behaviors. Like I said, from my experimentation with common objects, most browsers fire the event quite early, before onload. So spec'ing firing the event after the end of task would be incompatible with the current implementations, and this API is quite old. Sure, we can ask all browsers to change their behaviors, but that might be problematic for websites that rely on the current behavior. Spec'ing earlier firing is not something WebKit can implement, so we wouldn't want to spec that either.

[annevk]

Yeah, the answer to that is figuring out which browser(s) need to align and what the model should be. It's not to just not specify it.

[yoavweiss]

Ok, so my understanding of this issue so far:

• For synchronous fetches
  • Sync XHR - IIUC, the readystatechange, load and loadend events are fired synchronously after the response body was fully received. (steps 10-12 at Handle response end of body
  • importScript - IIUC, Main Fetch step 15 means that no events fire for such sync fetches.
• For async fetches
  • The current spec indicates that the event should fire synchronously, but that's something WebKit cannot follow due to their architecture.
  • At the same time, firing the event asynchronously will mean that requests could attempt to add themselves while the buffer is full and before the event fired, and get dropped to the floor.
  • To overcome this issue, WebKit added a secondary buffer which collects entries between the time the buffer was found to be full and the time the event fired, to make sure these requests don't get dropped.

Does that match everyone's understanding of the issue?

Seems to me that WebKit's model may be a good compromise that addresses the use-cases without firing the event synchronously. We could specify something similar (a secondary buffer, etc) to achieve something like that across implementations.

@annevk and @smaug---- - Can you expand on your concerns with the current definition? Would an async model be more acceptable from your perspective?

@tdresser and @npm1 - Would moving to an async model similar to WebKit's something that Chromium would be interested in?

@toddreifsteck - same question for Edge :)

[smaug----]

My concern with the current definition is that it doesn't define what the behavior should be in case of synchronous loads. See the initial comment in this bug.
Asynchronous firing sounds reasonable to me.

I may not like #141 (comment) too much, if that really means that webkit fires at the end of the current task. But perhaps it doesn't mean that, but just that events are queued to fire asynchronously.

[npm1]

I think we can change to do async buffering if all other browsers are committed to doing so as well. It would be nice to measure the impact on current users, for instance by looking at how often entries are queried during onload. @igrigorik did you have a proposal to only buffer entries before onload? How would this change affect that proposal?