get(Entries|EntriesByType|EntriesByName) methods spec'ed to return a copy about performance-timeline HOT 17 CLOSED

I think Chrome's implementation is reasonable. Any objections to leave unspecified and at the UA's discression?

from performance-timeline.

In an environment with global buffer and many consumers returning copies makes sense. Otherwise we're relying on everyone being a good citizen and not modifying the underlying entries.. which is a dangerous assumption. In fact, separate but related question here is whether we should allow these entries to be modified or keep them readonly?

We should also test other browsers to see how they behave.

from performance-timeline.

Yeah, why are they even mutable? ^_^

from performance-timeline.

Testing this across various browsers:

• Safari:
  • performance.timing returns the same object.
  • getEntriesByType is not supported.
• Firefox:
  • performance.timing returns same object.
  • performance.getEntriesByType("resource") returns same objects each time.
• Chrome: same behavior as Firefox.
• IE (11):
  • performance.timing returns same object.
  • performance.getEntriesByType("resource") returns new copy each time.

As such, we have an inconsistency between FF, IE, and Chrome. The open question is: what behavior do we actually want? Is there a strong argument for one or the other?

/cc @esprehn @bzbarsky @toddreifsteck

from performance-timeline.

For Performance Observers, it would make sense to return the same objects each time since they are already local copies to that observer. For the global buffer, it's already a shared context, I don't see the harm in leaving it shared copies.
as far as getEntriesByType returning copies or the same object.. no strong opinion.

from performance-timeline.

@igrigorik There is no possible way Firefox returns the same object on each getEntriesByType call. And the way the spec is currently written, there is no way a conformant implementation could do so. Specifically, the spec at http://w3c.github.io/performance-timeline/ has:

PerformanceEntryList getEntriesByType (DOMString entryType);

and

typedef sequence<PerformanceEntry> PerformanceEntryList;

Since sequences are pass-by-value, each return value must be a new JS Array object. How did you determine that Firefox returns the same object each time, exactly?

from performance-timeline.

@bzbarsky same snippet as @esprehn's original example...

performance.getEntries()[0].foo = 1;
performance.getEntries()[0].foo == 1;

from performance-timeline.

Oh, you're looking at the entries inside the list, not the list itself! Sorry, I missed that context.

So yeah, Gecko definitely just returns a list of pointers to the same things for that.

I'm not sure I have a strong opinion on what the API here should look like for that, exactly.

from performance-timeline.

FWIW, I'm not aware of any developer bug reports that have been raised due to this behavior... against Chrome, at least. Scanning Mozilla's tracker, I don't see anything either. @toddreifsteck are you aware of any corner cases?

If we don't think this is a big deal either way, one route is to update the spec to "that contains a copy of all PerformanceEntry objects" -> "that contains a sequence of all the PerformanceEntry objects". I think that would be sufficient to reflect current behavior?

from performance-timeline.

This is history... It may take me a few days to follow up, but I'll try to find someone aware of the reason this was made as it is.

from performance-timeline.

Followed up with Jatinder. The intention of this requirement was that the call to getEntries() would snapshot the state of the various timers. This would allow a developer to know that a particular set of resource timings was loaded at one time and then to getEntries() later and know the changes.

Ensuring @nicjansma sees this as the RUM audience is well positioned to speak to whether this use-case is important enough to ask various browsers to make changes.

from performance-timeline.

@toddreifsteck ah, interesting, thanks. FWIW, I think we're addressing this use case with PerformanceObserver in a much better way: there is no need to poll or manually diff the returned set of records.

from performance-timeline.

The only reason I can think of for ensuring that copies of PerformanceEntry objects are returned is if there are two consumers of getEntries() on the same page, they could both be blindly adding attributes and having conflicts.

Having PerformanceEntry be readonly would ensure there are no conflicts and would allow for pointers to the same objects to be returned.

Though we're not specifically extending PerformanceEntry for our RUM, I could imagine others are. And for RUM, after calling getEntries() once, the delta between the next getEntries() call is what we'd care most about (and is better solved by PerformanceObserver).

So +++ to everything Ilya's said!

from performance-timeline.

@nicjansma @esprehn pointed out to me that read-only would add a perf cost, so not sure we should or need to require that in this context.

FWIW, my proposal is as stated earlier: "that contains a copy of all PerformanceEntry objects" -> "that contains a sequence of all the PerformanceEntry objects". Any objections?

from performance-timeline.

lgtm

from performance-timeline.

LGTM as well

from performance-timeline.

@nicjansma @toddreifsteck will resolve this as part of #11, ptal at that pull.

from performance-timeline.