w3c / media-source

Media Source Extensions
https://w3c.github.io/media-source/
Other
267 stars 59 forks source link

Change MediaSource handle getter value to null from exception #316

Closed wolenetz closed 1 year ago

wolenetz commented 1 year ago

Due to compatibility issues with existing web app libraries that enumerate MediaSource attributes, throwing an exception on attempted read of a MediaSource object's handle attribute was unfortunately a breaking change.

This change updates the spec to instead make the MediaSource handle attribute readonly [SameObject] getter return null instead of throwing an exception if the implementation doesn't support creating a MediaSourceHandle for that MediaSource object instance.


Preview | Diff

wolenetz commented 1 year ago

@karlt @jan-ivar @mwatson2 @jernoble : We had to disable the implementation of MSE-in-Workers in Chrome 105 and 106 due to this breaking change behavior's impact on older MediaSource app libraries. Please take a look, note there is also another option and a question:

Other option: Simplify to just not expose the handle getter on Window, just on DedicatedWorker (and in future, wherever it might be supported, including Window, if we spec that.)

Question on the current option in this PR: Are "nullable" [SameObject] readonly attribute types like MediaSourceHandle? allowed by WedIDL? The Chromium blink bindings generator didn't complain, but the WebIDL SameObject spec might not allow this (I'm uncertain): "The [SameObject] extended attribute must not be used on anything other than a read only attribute whose type is an interface type or object."

Please comment on preferred option and your understanding of the WebIDL question for the this nullable option.

karlt commented 1 year ago

Yes, if you read the WebIDL spec literally it would exclude nullable types, but I don't know whether that was the intention. The only precedent that I found for [SameObject] with a nullable type was AuthenticatorAssertionResponse.userHandle.

Given the handle property does not ever (currently) return anything useful on Window, I'd be inclined to go with [Exposed=DedicatedWorker] for handle. There is plenty of precedent for exposing members only to contexts where they are useful.

wolenetz commented 1 year ago

@karlt - Thank you for your input. I suspect that if we continued with the "nullable" option, we would need to drop the [SameObject] from the handle's IDL, and replace it with similar normative text that also allows "null" to be returned. There are a couple of long-standing issues in WebIDL around [SameObject] (one for nullable: https://github.com/whatwg/webidl/issues/559, another for general utility of it: https://github.com/whatwg/webidl/issues/212).

Initially, I was leaning toward the nullable option (instead of restricting visibility). Details:

  1. I do not want a repeat of some unexpected MSE app lib breaking change due to some variance in MSE object property visibility across contexts (in future, obviously, since right now, such exposure is new to the worker context). But, on further thought, this is a new exection context for MSE so it appears such risk is low right now. Further, it could enable, in future, rapid detection of Window/Main context availability of some (not currently spec'ed) way of getting a MediaSourceHandle from a main thread owned MediaSource just by adding visibility to the getter on the main thread.
  2. Like throwing an exception, returning null if unsupported gives more flexibility to implementations about when to decide whether or not a handle is available from a MediaSource. On further thought, I'm uncertain if there is any need for this flexibility. In Chrome, there isn't for MSE-in-Workers.
  3. On a preliminary CL in Chromium that would employ visibility restriction, a blink API owner expressed preference instead for the null return option. I'm querying for more details right now on that preference.

I'll await a bit longer for further responses from @jernoble @mwatson2 and @jan-ivar (or anyone else following along on this), as well as hopefully more detail on the preference mentioned by the blink API owner.

(Reference tracking feature issue #175)

wolenetz commented 1 year ago

Friendly ping @mwatson2 @jernoble @jan-ivar to please provide input (I'm now leaning towards restricting handle getter visibility to DedicatedWorker context only for now, and removing ability of that getter to throw exception), based on feedback received so far.

wolenetz commented 1 year ago

I'll be replacing this PR today with the approach agreed upon in conversations, above, and at the Media Working Group session last Friday at TPAC (link to relevant portion of meeting notes).