The "convert artwork algorithm" is somewhere between underdefined and nonsensical

[bzbarsky]

Step 2 says:

For each entry in input’s artwork, perform the following steps:

OK, but what's "input"? When called from the MediaMetadata constructor, it's a sequence<MediaImage>. When called from the artwork attribute setter, it's an ES array object. In neither case does it possess an "artwork". If the intent is to actually iterate over "input", then in the ES array object case you need to define very precisely how the iteration happens, because the different ways of doing it are observably different in terms of their side-effects.

I suspect it would just be best to have the "convert artwork algorithm" always take a sequence as input, and instead of having a writable FrozenArray attribute have a setter function that takes sequence<MediaImage>.

[bzbarsky]

Oh, and in the ES array object case, of course, what it contains are ES objects, not MediaImage dictionaries, so the rest of this algorithm is underdefined as well (e.g. what does "entry's src" mean? is that an ES Get or something else?).

[mounirlamouri]

Oh, and in the ES array object case, of course, what it contains are ES objects, not MediaImage dictionaries, so the rest of this algorithm is underdefined as well (e.g. what does "entry's src" mean? is that an ES Get or something else?).

That is a leftover from the time where MediaImage was an interface. Will file a bug fro this.

[mounirlamouri]

@bzbarsky actually, looking more into this, regarding your second comment, how is it different from how notifications actions are handled in https://notifications.spec.whatwg.org/#dom-notification-actions ? It would help to understand how the prose from the notification is correct but not ours.

[mounirlamouri]

@bzbarsky would this solution work for you:

  attribute sequence<MediaImage> artwork;

When getting, it would run steps similar to:

1. Create array of `MediaImage`
2. For each element of the internal representation:
  1. create a copy
  2. freeze it
3. freeze the array
4. return it

When setting, it would do something similar to what we have today but this time, it would use a sequence as an entry which would solve your problem.

Does that work is it completely wrong?

[bzbarsky]

how is it different from how notifications actions are handled in https://notifications.spec.whatwg.org/#dom-notification-actions ?

That's a getter. We're talking about a setter here. The inputs are different...

@bzbarsky would this solution work for you:

Unfortunately, having a sequence-typed attribute is not valid IDL at the moment. The idea of FrozenArray was to address the use cases people had for sequence-typed attributes...

When getting, it would run steps similar to:

You don't have control over that if your type is "sequence". You could do this if your type were "object", or something....

[foolip]

@domenic @annevk, @bzbarsky says that you were involved with defining FrozenArray<T> in Web IDL like this, so maybe you can help figure this out?

The core of the problem as I understand it is that https://heycam.github.io/webidl/#es-frozen-array defines "An ECMAScript value V is converted to an IDL FrozenArray value" in a way that converts the input ES array to an IDL sequence and back to another ES array which is frozen. This means that any algorithm that takes a FrozenArray<T> has to operate on the ES array, which would amount to writing custom bindings.

To quote myself from a private email:

Would it work to say (in Web IDL) that converting an ES value to an IDL FrozenArray is synonymous with converting to a sequence and requiring prose to use "create a frozen array" to convert that to a frozen ES array? The typical pattern would then be creating to a frozen array at some well defined point and then just returning that in a getter.

I'm not able to imagine a situation where it's critical that generated bindings is what creates a frozen ES array, and in fact can't see when that would be desirable given that the C++/Rust/whatever within presumably wants to do something with the value, and all specs using this would essentially need custom bindings...

[foolip]

Are there any other cases where ES→IDL conversion actually results in an ES value like this?

[mounirlamouri]

Thank you @foolip for explaining this in layman's terms. I finally understand what the problem is 😃

As a temporary measure, would it make sense to say that the ES array should be converted to a sequence<T> in the setter algorithm?

[annevk]

This seems basically like another ask for distinct getters and setters in IDL.

[foolip]

@annevk, is it necessary to separate getters and setters for this? Instead saying that the IDL representation of a FrozenArray<T> is the same as sequence<T> would work for this case.

@mounirlamouri, yes, as a workaround it should work to have some prose that invokes the conversion to sequence<T>.

[annevk]

@foolip oh yes, we could probably do that for the setter case, although it'll get a little confusing. For methods we should probably not allow FrozenArray<> as argument though.

[bzbarsky]

Some of this discussion should maybe move into an IDL spec issue, but....

Are there any other cases where ES→IDL conversion actually results in an ES value like this?

No. Even object and any create IDL values that represent the same thing as some ES value, as opposed to just "being" an ES value. Of course in practice they could just be an ES value.

Also in practice, the typed array types could be ES values. But they don't have to be. In Gecko's bindings, for example, the IDL representation of typed arrays is not just as the corresponding ES values.

So technically, the current spec for FrozenArray should really convert it to an IDL object when all is said and done.

As a temporary measure, would it make sense to say that the ES array should be converted to a sequence in the setter algorithm?

It would at least make the spec well-defined. It would produce behavior that is black-box different from the one Chrome implements and Gecko would like to implement if it were implementing this spec, of course. ;)

This seems basically like another ask for distinct getters and setters in IDL.

You mean distinct types, right? It sort of does, yes.

Instead saying that the IDL representation of a FrozenArray is the same as sequence would work for this case.

Not on its own, no. It would mess up the getter situation: if on the IDL side you have a sequence, then when does the ES Array get created (presumably during the IDL-to-ES conversion?) and where is it cached (this is the part that the current spec is intending to solve).

That said, the current IDL spec is pretty annoying for any case where the setter or method wants to do any processing at all on its argument. Passing it in as a sequence would make that a lot saner. The current IDL spec really only has good support for a setter that takes the object and then returns that exact object...

One thing that would work for a lot of cases is to define something like this:

1. The IDL representation of FrozenArray is identical to sequence.
2. FrozenArray is only allowed as the type of an attribute, not in any other situation. This is not breaking compat so far; the only uses I'm aware of are readonly attributes anyway.
3. Define that an attribute of type FrozenArray induces an internal slot on the corresponding object, initially storing undefined.
4. Define that a getter returning FrozenArray first checks the corresponding slot. If the value is undefined, execute https://heycam.github.io/webidl/#dfn-create-frozen-array on the attribute's IDL representation and store the resulting object in the slot. Finally return the value in the slot (which is now guaranteed to be an object).
5. When the value is supposed to change, the relevant spec would need to clear the autogenerated slot. This is no worse than what the IDL spec has right now; right now a spec that has a FrozenArray attribute has to replace the stored ES Array object every so often.

That's all lovely, but it doesn't solve the use case of the mediasession spec, because that spec wants an algorithm different from https://heycam.github.io/webidl/#dfn-create-frozen-array for creating its return value: it wants to freeze all the array entries too.

So I think my best shot at a useful proposal is this:

1. We disallow FrozenArray as the type of anything that's not a readonly attribute, until we have more use cases to inform how to design other uses of it.
2. Mediasession uses object as the type, with the following behavior:
   a. The thing stored is an actual ES object.
   b. The setter manually invokes https://heycam.github.io/webidl/#es-to-sequence on its input to end up with a sequence, then passes that sequence to the already-existing https://wicg.github.io/mediasession/#convert-artwork-algorithm and then takes the result and creates the exact ES object it wants from it and stores it. There's various talk of "lists" here where I think sequences are meant, but that's editorial detail that's easy to fix. The setter would probably need to store the actual sequence too, to make hhttps://wicg.github.io/mediasession/#fetch-image-algorithm work.

Or something like that. I guess the "official" stored value could be the sequence, with a separate slot for the cached object and the conversion done in the getter (basically manually implementing the thing I describe above as "that would work for a lot of cases", except with a different IDL-to-ES conversion algorithm). But one way or another, you need to store both the ES value and the sequence, as far as I can tell, to make this spec work right.

[foolip]

I've been meaning to come back to this issue, but haven't yet. In the meantime, those on this issue may be interested in #183 too.

[youennf]

@bzbarsky proposal to update the setter makes sense to me and fixes this issue.
Marking as "Ready for PR" for now.

[youennf]

PR available at #343

[youennf]

Fixed by #343