Open Bug 1236777 Opened 8 years ago Updated 9 days ago

Implement FrozenArray in webidl

Categories

(Core :: DOM: Bindings (WebIDL), task, P3)

task

Tracking

()

Tracking Status
firefox46 --- affected

People

(Reporter: jib, Unassigned)

References

(Blocks 3 open bugs, )

Details

(Keywords: dev-doc-needed)

To implement RTCTrackEvent [1], we should ideally use FrozenArray:

  // TODO: Use FrozenArray once available.
  //  readonly        attribute FrozenArray<MediaStream> streams;

Workaround:

  [Frozen, Cached, Pure]
  readonly        attribute sequence<MediaStream> streams;

[1] http://w3c.github.io/webrtc-pc/#rtctrackevent
Is FrozenArray now considered stable enough?  It seems fine to me, but have we had any feedback from the other browser vendors?

Note that the workaround in comment 0 will in fact produce identical semantics to FrozenArray, and in fact the implementation plan for this should probably be to just desugar FrozenArray, at least for attributes, so consumers are not forced to deal with JS arrays.
Flags: needinfo?(cam)
The only feedback I've seen (which you have too) is in https://www.w3.org/Bugs/Public/show_bug.cgi?id=29004 where Travis still doesn't seem convinced.  Really the only feasible alternative is to use non-frozen arrays, don't create new instances when we need to change their contents, define carefully how updates to arrays work, and for the platform object that vends such such arrays not to use their contents as the source of truth.  The two competing concerns of "live lists are bad" and "object identity from getter should be preserved" are fundamentally at odds.

At this point I don't know what can be done beyond trying it out.
Flags: needinfo?(cam)
We can try it out in practice using said workaround....
When this is implemented, it should be documented here: https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings
Keywords: dev-doc-needed
This is needed for the vibrate and actions attributes of Notifications: https://notifications.spec.whatwg.org/#api
Blocks: 1273641, 1225110
Not really blocking anything; see comment 0.  We already support this, just with an uglier syntax.
Flags: needinfo?(overholt)
Oops, sorry.
No longer blocks: 1225110, 1273641
Flags: needinfo?(overholt)
See Also: → 1284357
Note, I have a spec that requires `[SameObject] FrozenArray<>`.  [Pure] seems incompatible with [SameObject].

See ancestorOrigins attribute here:

https://w3c.github.io/ServiceWorker/#client-interface
> Note, I have a spec that requires `[SameObject] FrozenArray<>`.  [Pure] seems incompatible with [SameObject].

Well, our IDL parser rejects, because [SameObject] is a stronger claim than [Pure].

Since it's a stronger claim, you can just replace "Pure" with "SameObject" in the workaround annotations.

Specifically:

1)  We allow a sequence<> type for an attribute if the attribute is [Cached].  See https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Cached

2)  [Cached] attributes must be [Pure] or [Constant] or [SameObject], as documented in the same place.
> because [SameObject] is a stronger claim than [Pure]

Which means claiming _both_ is nonsense and you're probably doing something wrong if you claim both...
Priority: -- → P3
Blocks: 1497430
Component: DOM → DOM: Core & HTML

Just wanted to throw my hat in the ring on this one. Unfortunately we need support for FrozenArrays because it's the only way to implement a static attribute that returns a sequence.

E.g., https://w3c.github.io/performance-timeline/#supportedentrytypes-attribute

The other workarounds are useful, but cannot be applied in the static case:

TypeError: Attribute PerformanceObserver.supportedEntryTypes is static, so we don't have a useful slot to cache it in, because we don't have support for that on interface objects.

Will, I just looked at the linked spec and it has two problems:

  1. If it's returning a new object each time, why is that object a FrozenArray? Why not just return a sequence?
  2. If it's returning a new object each time, why is it an attribute? That's a serious anti-pattern for attributes.

My guess is that "it's not a sequence, because IDL does not allow sequences, because it's an antipattern, so the spec authors worked around that by using a frozen array, but cheated by not actually returning it by reference like you're supposed to do with a frozen array". Incidentally, the spec is nonsense, because it's returning an infra list, not an actual FrozenArray value, for something of type FrozenArray, which is being forced on them by their broken workaround.

tl;dr: even if we implemented FrozenArray as defined in the IDL spec you would not be able to implement this spec as written with it. The right course of action is to file a spec issue: this spec should have a static operation that returns sequence, if it wants to return new values each time. Do you want to file that spec issue, or do you want me to?

Flags: needinfo?(whawkins)

Thanks for your reply, :bzbarsky. (For what it's worth, I hear that you are coming to the TO office for a workweek and I'm looking forward to meeting you!)

I would be happy to file the bug report, but I think that your arguments are really cogent and would carry more weight coming from you. There are several other aspects of the spec that are not fully baked and, after working through an implementation for FF, I will be filing some additional bugs.

Thanks again for your help with this!! And, of course, I didn't mean to imply that the spec was perfect and that we were wrong. I was simply trying to implement what they said.

Thanks again for your response!
Will

Flags: needinfo?(whawkins) → needinfo?(bzbarsky)

Will, looking forward to meeting you!

I filed https://github.com/w3c/performance-timeline/issues/117

In the meantime, if we really do want to implement what this spec says right now that is in fact possible: define the return type as object and then use ToJSValue on an nsTArray with the relevant strings to create the array, followed by freezing the array to create the value to return. It's not very pretty, but it's what you would have to do anyway even if we had FrozenArray support, if we implemented it per IDL spec.

Flags: needinfo?(bzbarsky)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #14)

Will, looking forward to meeting you!

I filed https://github.com/w3c/performance-timeline/issues/117

In the meantime, if we really do want to implement what this spec says right now that is in fact possible: define the return type as object and then use ToJSValue on an nsTArray with the relevant strings to create the array, followed by freezing the array to create the value to return. It's not very pretty, but it's what you would have to do anyway even if we had FrozenArray support, if we implemented it per IDL spec.

Thanks again for the response! I don't know whether to be pleased or dismayed, but I generally understand the method you suggest. I will work through it tomorrow.

Look forward to meeting you in a few weeks!

Component: DOM: Core & HTML → DOM: Bindings (WebIDL)
Blocks: 1581701
Type: defect → task
Severity: normal → S3
See Also: → 1891784
You need to log in before you can comment on or make changes to this bug.