1304250 - [EME] Implement MediaKeySession.onmessage

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

This Web Platform test is failing because we don't implement MediaKeySession.onmessage:

https://w3c-test.org/encrypted-media/idlharness.html

Should be an easy fix.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Specification:
https://w3c.github.io/encrypted-media/#dom-mediakeysession-onmessage

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1304250 - Implement MediaKeySession.onmessage.

Review commit: https://reviewboard.mozilla.org/r/79942/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79942/

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8793150 [details]
Bug 1304250 - Implement MediaKeySession.onmessage.

https://reviewboard.mozilla.org/r/79942/#review78764

comments fixed, r+

::: dom/media/eme/MediaKeySession.cpp:509
(Diff revision 1)
> +MediaKeySession::GetOnmessage()
> +{
> +  EventListenerManager *elm = GetExistingListenerManager();
> +  return elm ? elm->GetEventHandler(nsGkAtoms::onmessage, EmptyString())
> +             : nullptr;
> +}

You could just have
return GetEventHandler(nsGkAtoms::onmessage, EmptyString());

::: dom/media/eme/MediaKeySession.cpp:512
(Diff revision 1)
> +  return elm ? elm->GetEventHandler(nsGkAtoms::onmessage, EmptyString())
> +             : nullptr;
> +}
> +
> +void
> +MediaKeySession::SetOnmessage(EventHandlerNonNull* handler)

aHandler or aCallback

::: dom/media/eme/MediaKeySession.cpp:518
(Diff revision 1)
> +{
> +  EventListenerManager *elm = GetOrCreateListenerManager();
> +  if (elm) {
> +    elm->SetEventHandler(nsGkAtoms::onmessage, EmptyString(), handler);
> +  }
> +}

You could just have
SetEventHandler(nsGkAtoms::onmessage, EmptyString(), aCallback);

::: dom/media/test/test_eme_playback.html:157
(Diff revision 1)
>        is(session.numKeystatuseschangeEvents, session.numOnkeystatuseschangeEvents,
>           TimeStamp(token) + " should get as many keystatuseschange as onkeystatuseschange");
> +
> +      ok(session.numMessageEvents > 0, TimeStamp(token) + " should get message events");
> +      is(session.numMessageEvents, session.numOnMessageEvents,
> +         TimeStamp(token) + " should get as many message as onmessage events");

well, technically the events are 'message' events, never with on-prefix. So
"Should have as many event listener calls as event handler calls."

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4da0c4036de3

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d569107d6c47

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/575aef952d535bf66d088cdb4e9745b0037fcea5
Bug 1304250 - Implement MediaKeySession.onmessage. r=smaug

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/575aef952d53

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/575aef952d53