[heycam/webidl] non-readonly [FrozenArray] attributes are footguns as currently specced (#810)

I've brought this up before, but I can't find any relevant issues, so trying to put it all down in one place.

Consider this IDL:

    attribute FrozenArray<long> arr;

What happens when the setter is called?  Per spec we land in https://heycam.github.io/webidl/#es-to-frozen-array which converts the value passed to the setter to a sequence, then converts that sequence to a JS `Array` object, freezes it, and passes that off to the underlying spec-defined setter.

OK, but now the spec-defined setter is getting a JS `Array`, and spec authors have no idea how to deal with those.  A typical example of a spec using this stuff (not linking, because I am not trying to call out anyone in particular) looks like this:

> For each entry in input
....
> Set image’s sizes to entry’s sizes

Both of those statements are problematic in this case.  "For each entry in input" is ambiguous when _input_ is a JS `Array`.  Does that mean calling `input.forEach`?  Does it mean calling the original value of `Array.prototype.forEach`?  Does it mean doing the equivalent of `for (let entry of input)`?  Does it mean doing `for (var i = 0; i < input.length; ++i) { let entry = input[i]; // etc`?  All of those produce observably different behaviors.  Spec authors probably (1) don't realize those options exist, (2) don't realize they do different things.  Implementors are left to make up something.

"Set image's sizes to entry's sizes" is similarly problematic: _input_ is a JS `Array`, so _entry_ is a JS value.  In this case, because the FrozenArray type parameter is a dictionary type we know that `Type(entry)` is Object, but that says nothing about how "_entry_'s sizes" is computed.  The principled thing would be to convert the object to the relevant dictionary type, but that is a little silly because we already had that dictionary in our sequence before we created the JS `Array`, and dictionaries don't actually roundtrip through to-JS-and-back conversions if someone has messed with `Object.prototype`...  

OK, so what do implementations do?

* Gecko doesn't implement `FrozenArray` at all, for some of the above reasons; the thing we do implement has the semantics of a sequence-typed attribute with automatic caching at the binding layer on get and automatic cache invalidation on set.

* Blink claims to implement `FrozenArray`, but doesn't follow the spec and passes a sequence instead of a JS `Array` to the underlying setter, as far as I can get.  Getters are expected to return a JS `Array`; the typical Blink implementation of a `FrozenArray` API returns a new JS `Array` on each get, which is also problematic.

* WebKit seems to pass a sequence-like thing, not a JS `Array` for the setter and likewise expect one for the getter; I didn't dig enough into whether they cache on get or not.  There are no non-test writable FrozenArray-typed attributes in WebKit, as far as I can see.

I would like to propose that we more or less reify the Gecko semantics into the IDL spec: an ES-to-FrozenArray conversion creates a sequence, while a FrozenArray-to-ES conversion takes a sequence as input, creates a frozen JS Array from it, and caches it.  At least for the attribute getter case.  I'm not sure about method return values or FrozenArray used in dictionary members, but I'm also not sure we have any use cases for that sort of thing, and perhaps we should just disallow it....

@domenic @annevk @ms2ger @heycam @tobie @saschanaz 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/issues/810

Received on Wednesday, 2 October 2019 17:20:44 UTC