Is it well defined when the "add a PerformanceResourceTiming entry" algorithm should run?

[yoavweiss]

I couldn't find when the "add a PerformanceResourceTiming entry" algorithm is defined to run, and I have ran into trouble in WebKit with an example similar to the one in Example 2 (only with XHR), where at the onload event, there is expectation that the entry should already be there.

Is the algorithm's running point well defined? Do we have tests for that?

[igrigorik]

Following the processing model:

• For each resource fetched by the current browsing context, ..., perform the following steps:
  • Create a new PerformanceResourceTiming object and set entryType to the DOMString resource.
  • ... initialize all the timestamps ...
  • After recording duration (once we know responseEnd)
    • Queue the PerformanceResourceTiming object.
    • Add the PerformanceResourceTiming object.

So, conceptually it's supposed to be added when the response becomes available - e.g. ~same time as XHR success callback. We don't have good hooks for this though in L2.. At least, not until we integrate with fetch. That said...

I have ran into trouble in WebKit with an example similar to the one in Example 2 (only with XHR), where at the onload event, there is expectation that the entry should already be there.

I think that's a reasonable read based on current definition of "add a perf entry", but perhaps something we should discourage: synchronous API's are an anti-pattern and developers shouldn't rely on Perf Timeline exposing new entries in synchronous fashion (e.g. XHR success != perf entry is immediately available). In fact, we already do this for PerformanceObserver: https://w3c.github.io/performance-timeline/#dfn-queue-a-performanceentry

[yoavweiss]

Based on my reading of this, adding the entry either before or after the onload is permitted and creates an observable implementation difference.

Your "synchronous APIs are an anti-pattern" argument makes sense to me, and I agree we should discourage it. As such, can we define that the entry is added after the onload event?

In any case, it'd probably be a good idea to modify the example so that it doesn't rely on that currently-implementation-specific behavior.

[igrigorik]

Your "synchronous APIs are an anti-pattern" argument makes sense to me, and I agree we should discourage it. As such, can we define that the entry is added after the onload event?

No, I don't think we want to postpone all buffer entries until after onload. There is no reason to: it would be incompatible with today's implementations, it would probably break and/or complicate a bunch of use cases. Rather, we just want to spec it in such a way that allows the UA to asynchronously append entries to the buffer.. just like we do with PerformanceObserver.

I guess a simple way to do that would be to move "Add the PerformanceResourceTiming object" into the "Queue the PerformanceResourceTiming object" algorithm.. Functionally, it's as if we have a "system observer" that's subscribed to bunch of events that have associated buffers.

@plehegar @toddreifsteck wdyt?

[plehegar]

I like the idea. One caveat: we don't add Frame Timing entries, so if you move add into the queuing part, you'll need to add a parameter to the algorithm to indicate whether the queued entry should also be added or not. We can make the default to be "don't add" of course.

[yoavweiss]

No, I don't think we want to postpone all buffer entries until after onload. There is no reason to: it would be incompatible with today's implementations, it would probably break and/or complicate a bunch of use cases.

To clarify, I meant the resource's onload event, not the window's onload event. (and looking back at the spec examples, I don't think they are as racy as I thought they are, and understand why you may have thought I'm talking about window.onload).

I'm not sure what are current implementations doing in that respect, and would be happy with aligning the spec accordingly (either before or after the resource's onload). AFAICT, moving "Add" into "Queue" doesn't solve that, and resolving it may require hooking into Fetch: #39 (as already mentioned)

[igrigorik]

@yoavweiss ah, ok that makes more sense.

I think we want to queue the event at resource onload, and then run through the same async steps as proposed earlier. That is, we make no guarantees that getEntriesByX() will return the entry if you run it within the resource onload handler -- we've already fielded bug reports about this and clarified that this is not something we want to encourage or support.

[igrigorik]

@plehegar @toddreifsteck this seems like a good L2 candidate. Any objections? Would one of you guys be willing to pick this one up?

[CTJyeh]

Is there a way for user to see requests that have been initiated fetch but has not been queued/added into buffer?

[igrigorik]

Is there a way for user to see requests that have been initiated fetch but has not been queued/added into buffer?

No. If Fetch registry ever becomes a thing, then you'll be able to.. whatwg/fetch#65.

[igrigorik]

Quick recap of what we have today:

Step 18: Record the difference between responseEnd and startTime in duration.
...
Step 21: Add the PerformanceResourceTiming object.

Based on above, the entry object should be added after responseEnd is recorded (i.e. we received all the bytes), but we don't indicate if and when step 21 should be executed with respect to load event for said resource. A simple test shows that Chrome/FF may run this step before the load event on the resource fires, but that's also not guaranteed.. I can't find the bug URL, but I recall fielding XHR-related bugs where the entry was only exposed later, and we (Chrome) closed it as a wontfix because we didn't want to promise that RT entries must be added before load event fires.

1. I think it's a bad idea to suggest in the spec that step 21 MUST run before load event.
2. There are existing implementations where step 21 runs before load event for some types of resources, and after load event for others.
   • A brief tangent... There is desire to expose loadEvent{Start,End} on ResourceTiming in L3 (see For all resources, add timestamp of when it was available to the browser #60), which further hints that we should stay away from (1) and want to allow the entry to be added sometime after the load event. However, the one gotcha here is that doing this will require modifying definition for duration which is currently defined as responseEnd-startTime; hence L3.

So, given the mix of existing implementations in the wild.. I don't think we're in a good position to force a "MUST do" clause for when step 21 should run. That said, I think we could add a "SHOULD" clause indicating that step 21 ought to run sometime after the user agent completes the load event for the resource (if applicable). Doing so doesn't break existing implementations, but still offers a good nudge for any new implementations + signals the direction we'll take in L3; in L3 we can codify this behavior with a MUST. In other words, I'm suggesting that we live with some implementation inconsistency on this particular issue in L2.

@toddreifsteck @plehegar @yoavweiss WDYT, does that sound reasonable to you?

---

p.s. ignore my suggestion in #55 (comment).. I agree with Yoav, shuffling where we run the "add" step doesn't really address the issue at hand.

[yoavweiss]

SGTM. We lived with these inconsistencies for a while now, and adding a SHOULD in the right direction would allow interested implementations to move towards more consistent behavior.

[toddreifsteck]

Off the top of my head, I'm not a fan of this.

Adding a SHOULD doesn't allow us to add tests. What about MUST with "At Risk"? Generally, I believe it is possible to guarantee that the resource timing entry is available for a resource's onload event and am unclear in what scenario that would not be good.

[igrigorik]

Adding a SHOULD doesn't allow us to add tests. What about MUST with "At Risk"?

My understanding is that we can't publish a REC with an 'at risk', so while we can insert it for a CR, we'd have to take it out anyway a few months later, which makes the whole exercise a noop.

Generally, I believe it is possible to guarantee that the resource timing entry is available for a resource's onload event and am unclear in what scenario that would not be good.

This forces synchronous delivery right before onload event fires, which is not something we want to promise; perf observer already indicates that records may be delayed until an idle period. Further, if we want to capture the onload time inside of the timing record it, by definition, needs to be queued afterwards.

[igrigorik]

Landed #79. Let's continue discussion in #82.

[JosephPecoraro]

This note was added, however I don't see the note in the latest Editor's Draft. I just spent quite a bit of time eventually hunting down this discussion. When should we expect the spec to reflect modifications?