Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MediaKeyStatusMap: Replace maplike with explicit methods #39

Closed
ddorwin opened this issue Mar 9, 2015 · 16 comments

Comments

Projects
None yet
4 participants
@ddorwin
Copy link
Contributor

commented Mar 9, 2015

Issue #25 identified problems with our use of maplike - specifically that BufferSource objects will be compared rather than the key IDs they contain. (Issue is also #24 related to maplike.)

It does not sound like the maplike behavior will change in a way that solves #25 (see the thread at https://lists.w3.org/Archives/Public/public-script-coord/2015JanMar/0148.html for more discussion), so we should move forward without maplike. (If compatible functionality is later added, we should be able to switch back without breaking application compatibility.)

I propose replacing MediaKeyStatusMap's

readonly maplike<BufferSource, MediaKeyStatus>;

with (a subset of the) explicit members that readonly maplike added (entries, forEach, get, has, keys, values, @@iterator methods and a size getter).

Shall we include all methods and the size getter? Do we need @@iterator if the object does not have bindings to an ES Map?

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Mar 17, 2015

@bzbarsky: Any comments on this or answers to the questions in the last paragraph of my original comment?

@bzbarsky

This comment has been minimized.

Copy link

commented Mar 17, 2015

It really depends on how you want the object to be used. Do you want it to be iterable via a for-of loop? Then you need @@iterator.

Past that, I agree that explicit methods are the way to go here for now.

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Mar 17, 2015

Okay, we should probably add @@iterator then.

Perhaps we should use an iterable declaration. This provides

“entries”, “keys”, “values” and @@iterator properties

(I think "properties" should be "methods")

Those are some of the same methods as readonly maplike. The difference is the lack of forEach, get, has and size. We could add those (at least get and has) explicitly to MediaKeyStatusMap to supplement the iterable declaration.

There is no "readonly" iterable declaration, but iterable declaration seems to give more flexibility for specifying behavior in prose. This includes whether "the list of values or value pairs to iterate over is snapshotted at the time iteration begins..."

Thoughts?

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Mar 17, 2015

For future reference, Web MIDI replaced the custom methods with maplike in this change. Some of the removed text may be useful for our transition in the opposite direction.

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Mar 17, 2015

It does not appear that objects with an iterable declaration "represent an ordered list" like maplike (issue #24). Is that correct?

@bzbarsky

This comment has been minimized.

Copy link

commented Mar 17, 2015

There is no "readonly" iterable declaration because an iterable declaration does not provide any mutators. So all iterable declarations are readonly, really.

Objects with an iterable declaration obviously represent an ordered list, no? The order is whatever order things are returned in by the iterators.

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Mar 17, 2015

Is it acceptable to say the order is undefined or implementation dependent? The order really shouldn't matter, but as issue #24 points out, we should say something about it. I don't think it is worth defining or requiring implementation of a fixed order.

@bzbarsky

This comment has been minimized.

Copy link

commented Mar 17, 2015

Experience with the "undefined enumeration order" thing in JS suggests that web pages will come to depend on the order in whatever browser they happen to test in and everyone else will then have to reverse-engineer it.

So unless there are significant implementation burdens to a fixed order, it's better to just define a fixed order in the spec, assuming the order is detectable at all....

@mwatson2

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2015

Since the keys are basically opaque key ids, there does not seem to be any way an application could rely on a particular ordering, so it seems fine to me to say that the order is implementation-defined, so long as we do not need to support any kind of fancy rules for modification-during-iteration.

It would be fine for me if the effect of modification during iteration was also undefined (i.e. implementation-specific)

@bzbarsky

This comment has been minimized.

Copy link

commented Mar 24, 2015

there does not seem to be any way an application could rely on a particular ordering

We've been surprised by such things before on the web.

Most simply, consider an application that enumerates until it gets to the first key with a certain status and then does something based on how many keys it has seen so far. Is this a daft thing to do? Yes. Do I fully expect people to do something like this and think they're being very clever? Yes.

@steelejoe

This comment has been minimized.

Copy link
Contributor

commented May 26, 2015

We are seeing a difference in implementation between Firefox and Chrome. This code:

var map = session.keyStatuses;
for (var [key, val] of map.entries()) { ... }

Works in Firefox but does not seem to in Chromium. I don't have a strong opinion on who is right, but I would like the same code to work in both places.

@mwatson2 mwatson2 self-assigned this Jun 11, 2015

@mwatson2

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2015

I did a bad thing, which I promise to fix in due course: ReSpec does not support "iterable" so I have added in that one line of IDL programmatically in the post-processor.

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2015

@steelejoe wrote:

We are seeing a difference in implementation between Firefox and Chrome. This code:

var map = session.keyStatuses;
for (var [key, val] of map.entries()) { ... }

Works in Firefox but does not seem to in Chromium. I don't have a strong opinion on who is right, but I would like the same code to work in both places.

This is unrelated to EME or even maplike/iterable. The for loop uses ES6 destructuring, which is still in development in Chromium (status).

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2015

Note: The fix includes the text "sorted by key ID," which addresses the ordering discussion above (May 17-24).

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2015

I filed #69 to further clarify what it means to be "sorted by key ID" and #70 to define the behavior of get() when the key ID is not in the map.

@ddorwin

This comment has been minimized.

Copy link
Contributor Author

commented Jul 20, 2015

#75 describes issues with using the iterable declaration added in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.