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

Change MediaSource handle getter value to null from exception #316

Conversation

wolenetz
Copy link
Member

@wolenetz wolenetz commented Sep 8, 2022

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

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.
@wolenetz
Copy link
Member Author

wolenetz commented Sep 8, 2022

@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
Copy link

karlt commented Sep 9, 2022

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
Copy link
Member Author

wolenetz commented Sep 9, 2022

@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: whatwg/webidl#559, another for general utility of it: whatwg/webidl#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 wolenetz mentioned this pull request Sep 9, 2022
@wolenetz
Copy link
Member Author

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
Copy link
Member Author

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).

@wolenetz wolenetz closed this Sep 20, 2022
wolenetz added a commit to wolenetz/media-source that referenced this pull request Sep 20, 2022
Removes the MediaSource handle attribute visibility from the main Window
context, leaving it visible only in dedicated worker contexts. Also
removes the conditional throwing of NotSupportedError exceptions on
getting this attribute's value if the implementation doesn't support
that.

Overall, this change is to undo the backwards incompatibility issue
found when older versions of MSE web app libraries enumerated main
thread MediaSource attribute values, hitting exception in the Chromium
feature implementation through milestone 107. This change replaces that
with simpler and safer visibility restriction of this getter to just the
context where it must be supported if the implementation supports
MSE-in-Workers.

Note, this change replaces the alternative fix discussed in
w3c/media-source PR w3c#316.
wolenetz added a commit that referenced this pull request Sep 21, 2022
Removes the MediaSource handle attribute visibility from the main Window
context, leaving it visible only in dedicated worker contexts. Also
removes the conditional throwing of NotSupportedError exceptions on
getting this attribute's value if the implementation doesn't support
that.

Overall, this change is to undo the backwards incompatibility issue
found when older versions of MSE web app libraries enumerated main
thread MediaSource attribute values, hitting exception in the Chromium
feature implementation through milestone 107. This change replaces that
with simpler and safer visibility restriction of this getter to just the
context where it must be supported if the implementation supports
MSE-in-Workers.

Note, this change replaces the alternative fix discussed in
w3c/media-source PR #316.

Reference feature tracking spec issue for MSE-in-Workers w3c/media-source issue #175.
@wolenetz wolenetz deleted the change-unsupported-handle-getter-result-to-be-null-NOT-exception branch September 21, 2022 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants