supportedEntryTypes API has some issues

[bzbarsky]

The two (interrelated) problems are:

1) It's an attribute that is returning a new value each time. This is a bad enough antipattern, known for long enough, that Web IDL tries to prevent it; it looks like the definition of this API used FrozenArray instead of sequence to try to work around that prevention.

2) The workaround is broken: the steps for the attribute getter return an infra list, but per Web IDL if you are returning a value of type FrozenArray you need to return an actual frozen Array object. There is no implicit conversion from list defined at https://heycam.github.io/webidl/#idl-frozen-array or https://heycam.github.io/webidl/#es-frozen-array or https://infra.spec.whatwg.org/#lists (compare to sequence, which does define such a conversion at https://heycam.github.io/webidl/#idl-sequence

If we want to return a new object each time here, this should be a method, not an attribute. If we want to return the same object each time, then using a FrozenArray makes sense, but it's worth thinking about where that object is going to be cached, because it needs to be cached somewhere. Probably in a slot on either the PerformanceObserver interface object or the global.

My personal preference would be for this API to be a static method returning a sequence, fwiw.

@npm1 @domenic @foolip @yoavweiss @igrigorik @spanicker @bakulf @w3c/web-assembly

P.S. I am aware that Chrome has already shipped this, with the "return a new value each time" behavior. I am fairly opposed to Gecko shipping anything of the sort.

[npm1]

Hi, it should be an attribute that returns the same object each time. Will send a PR soon to fix, and will fix Chrome implementation. Feel free to implement in Gecko without returning a new value each time. Regarding where it should be cached, I think it's related to https://github.com/w3c/performance-timeline/issues/113. The list of supported entry types may depend on the execution context, so it should be cached per execution context.

[bzbarsky]

The caching matters because it affects which object is returned, observably.

So in particular, if you take the getter from one global and call it with a this value that is PerformanceObserver interface object from another global, which global does the value come from?

For that matter, what happens if you call it with a this value that is not a PerformanceObserver object at all? https://heycam.github.io/webidl/#dfn-attribute-getter step 1.1.2 does nothing for static attributes, so they don't enforce any constraints on their "this" value.

Presumably you can tie the value to the current global of the getter. It implies a bit of weirdness if someone moves the function around across globals, but that might be ok.

[bzbarsky]

(By the way, I am rather curious how FrozenArray is implemented in Chrome, if implementing based on the spec as written actually worked. And whether we can fix it to not allow mistakes like that...)

[npm1]

An item could be added to the list here https://w3c.github.io/performance-timeline/#performance-timeline. That is, Each ECMAScript global environment has a list / FrozenArray of supported entry types.

(By the way, I am rather curious how FrozenArray is implemented in Chrome, if implementing based on the spec as written actually worked. And whether we can fix it to not allow mistakes like that...)

A FrozenArray is unmodifiable, that does not mean that the attribute getter for an object can change the FrozenArray it returns, if it needs to.

[bzbarsky]

That is, Each ECMAScript global environment has a list / FrozenArray of supported entry types.

Sure. The spec needs to say which global environment is used.

A FrozenArray is unmodifiable, that does not mean that the attribute getter for an object can change the FrozenArray it returns, if it needs to.

That wasn't my question. My question was how you go from spec prose returning an infralist to having a FrozenArray object. There are no implicit conversions between the two defined in the IDL spec. Does Chrome's implementation have such implicit conversions, or was it not actually doing what the spec said in terms of returning a list?

[npm1]

There is this: https://heycam.github.io/webidl/#dfn-create-frozen-array

[bzbarsky]

Yes, but that's not an implicit thing that "just happens". You have to explicitly invoke it when you want to create the object, then you typically cache the object.

[npm1]

Yep, I'll include that in the PR. And yes, the Chrome bindings layer seems to do that automatically.

[bzbarsky]

And yes, the Chrome bindings layer seems to do that automatically.

And I'm arguing that's a bug that we should get filed on the Chrome bindings. ;)

[npm1]

Well, maybe, I don't work on Chrome bindings. Since you're the best person to explain the problem, could you file a bug in https://crbug.com/new adding Blink>Bindings in the component? Then you can actually have a discussion with the people who can implement your desired behavior ;)

[bzbarsky]

@npm1 I reported https://bugs.chromium.org/p/chromium/issues/detail?id=946659 but I don't have permissions to set components or any other interesting metadata in the chromium bug system, so if you can set the component, that would be much appreciated.

[npm1]

Done. BTW https://heycam.github.io/webidl/#SameObject might be what you want (we can add it to the IDL).

[bzbarsky]

[SameObject] says the value in fact never changes, right. Adding it to the IDL in this case is a good idea, sure. I don't know whether it does any enforcement that the value never changes; in Gecko it does not, in fact.

[annevk]

Making [SameObject] useful is tracked by https://github.com/heycam/webidl/issues/212, FWIW.

[npm1]

Copying from the PR, there's two options here. Option 1: having a complete registration list in PerformanceTimeline. The benefits of the registration list approach are that it's easier to spec and more in line with what an implementation would actually do. The drawbacks are that there would not be a clear way to signal implementors of new specifications that they should update the list, seems incompatible with W3C workmode, and testing would be trickier (we want tests for specific strings to be in the folder of the corresponding specification, for example 'longtask' in the Long Tasks folder).

Option 2: having a hook for registration sp that specifications which want to add new entry types have to call. This approach is the one we're following right now in the spec and in the PR. The benefits are that it might work better with the W3C versioning system and allows separating tests into the appropriate folders. The drawbacks are that it involves more spec complexity and it differs more with what implementors would probably do (keep a list of the types).

Surfacing this in case people here have opinions!

[annevk]

Definitely 1. Reflecting implementations is much more important than any kind of process. This also goes for tests, so it's a little puzzling as why that would be harder. Sounds like it might also be an artifact of a particular process.

[bzbarsky]

I'm actually pretty torn here. Having the list in PerformanceTimeline could easily lead to a situation where a UA puts things in that list even though it doesn't implement the relevant specs, no? Unless the list is very clear about which specs need to be implemented to put which values in the list?

As far as how it would be implemented in practice, I'm not sure whether "just a list in PerformanceTimeline is the best match for a model where a UA may want to allow some of the specs involved to be disabled via a preference, so it's not 100% clear to me that the single list model is the best match for implementations either...

[npm1]

Discussed this at the call today. We'll move forward with option 1, and to address the problem of needing to modify the list whenever a new entry type is created we will have the registry in a WG Note. This list will similarly include the values for availableFromTimeline and maxBufferSize which were recently introduced to support the buffering behavior.

Regarding tests, we'll keep the tests for a specific entry type in the corresponding folder that introduces that type. This will help implementors notice the need to change the return value of supportedEntryTypes when implementing a new entryType. We may also have a single test with the full list of entries in PerformanceTimeline, though we'll ignore it when determining whether 'two implementations are green'.

We may optionally also add non-normative notes in other specifications to remind the implementor to update suppportedEntryTypes, but the normative text will live in PerformanceTimeline.

[rniwa]

I agree with @bzbarsky's concern here. Having a centralized list is likely going to result in UAs putting things that they don't yet support fully.

[plehegar]

I created https://w3c.github.io/timing-entrytypes-registry/index.html . If that works for folks, we'll publish and set up automatic updates.