Closed Bug 1081755 Opened 7 years ago Closed 7 years ago

[EME] Implement keyschange event

Categories

(Core :: Audio/Video, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

Dispatch a "keyschange" event whenever the CDM changes the usable keyIds on a MediaKeySession. We can also add a test for MediaKeySession.usableKeyIds once we have this event.
Summary: [EME] Support keyschange event → [EME] Implement keyschange event
Attached patch Patch v1Splinter Review
* Have CDMCapabilities report when setting a key usable or unusable would require a keyschange event (i.e. handle duplicate set[un]usable messages by CDM).
* Dispatch said keyschange event when usable keys change.
* Add test for MediaKeySession.usableKeyIds and keyschange event.

This patch is based on top of bug 1060179 and bug 1067216.
Attachment #8503826 - Flags: review?(edwin)
Attachment #8503826 - Flags: review?(bzbarsky)
Note: there's some discussion about changing the keyschange spec, but Adobe asked for it and we need it to reliably test MediaKeySession.usableKeyIds. We may need to revise our implementation once the current W3C discussion has resolved. But I think it's worth adding now so that we get test coverage for usableKeyIds.
Comment on attachment 8503826 [details] [diff] [review]
Patch v1

What are the outstanding spec issues?

>+  auto key = UsableKey(aKeyId, aSessionId);

That's an odd and roundabout way of writing:

  UsableKey key(aKeyId, aSessionId);

I'd really prefer the non-auto way here.  Both places.

>+  if (mData.mUsableKeyIds.Contains(key)) {

We don't expect this array to get long?

>+MediaKeys::OwnerDoc() const

Lack of Get prefix means null is never returned.  Which is clearly not the case here, right?

Call this GetOwnerDoc() please.  And maybe document what it actually returns?

r=me
Attachment #8503826 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #3)
> Comment on attachment 8503826 [details] [diff] [review]
> Patch v1
> 
> What are the outstanding spec issues?

We need to track any changes that come out of this W3 bug:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26372
I see, thanks.
(In reply to Boris Zbarsky [:bz] from comment #3)
> >+  if (mData.mUsableKeyIds.Contains(key)) {
> 
> We don't expect this array to get long?

I wouldn't expect it to contain more than a few keys at once.
https://hg.mozilla.org/mozilla-central/rev/43ca41b136f3
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Mass update firefox-status to track EME uplift.
You need to log in before you can comment on or make changes to this bug.