1289942 - [EME] MediaKeyStatusMap.get() should return undefined for keys it doesn't have

Status: RESOLVED FIXED

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

Description

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

MediaKeyStatusMap.get() is defined by the spec to return undefined for keys it doesn't have.

https://w3c.github.io/encrypted-media/#dom-mediakeystatusmap-get

This is causing this EME WPT of Google's to fail:
https://w3c-test.org/encrypted-media/Google/encrypted-media-keystatuses-multiple-sessions.html

Comment 1

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

Attachment: Bug 1289942 - Make MediaKeyStatusMap.get() return undefined for unknown keys.

The spec requires the MediaKeyStatusMap.get(keyId) function to return an 'any'
type, which is undefined for known keys, or a MediaKeyStatus enum value for
known keys.

https://w3c.github.io/encrypted-media/#idl-def-mediakeystatusmap

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8775406 [details]
Bug 1289942 - Make MediaKeyStatusMap.get() return undefined for unknown keys.

https://reviewboard.mozilla.org/r/67592/#review64834

::: dom/media/eme/MediaKeyStatusMap.h:49
(Diff revision 1)
>  
>    JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
>  
> -  MediaKeyStatus Get(const ArrayBufferViewOrArrayBuffer& aKey) const;
> +  void Get(JSContext* cx,
> +           const ArrayBufferViewOrArrayBuffer& aKey,
> +           JS::Rooted<JS::Value>* aOutValue) const;

That last argument should be a `JS::MutableHandle<JS::Value>`.  See https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#any

::: dom/media/eme/MediaKeyStatusMap.cpp:46
(Diff revision 1)
>  {
>    return mParent;
>  }
>  
> -MediaKeyStatus
> -MediaKeyStatusMap::Get(const ArrayBufferViewOrArrayBuffer& aKey) const
> +void
> +MediaKeyStatusMap::Get(JSContext* cx,

aCx, please.

Also, you will need an ErrorResult here (see below), so this method needs to become [Throws] in the IDL.

::: dom/media/eme/MediaKeyStatusMap.cpp:52
(Diff revision 1)
> +                       const ArrayBufferViewOrArrayBuffer& aKey,
> +                       JS::Rooted<JS::Value>* aOutValue) const
>  {
>    ArrayData keyId = GetArrayBufferViewOrArrayBufferData(aKey);
>    if (!keyId.IsValid()) {
> -    return MediaKeyStatus::Internal_error;
> +    aOutValue->setUndefined();

And then this would be `aOutValue.setUndefined()`.

::: dom/media/eme/MediaKeyStatusMap.cpp:57
(Diff revision 1)
> +    return;
>    }
> -
>    for (const KeyStatus& status : mStatuses) {
>      if (keyId == status.mKeyId) {
> -      return status.mStatus;
> +      const char* value =

Please don't reinvent the code-generated wheel.  This should be:

    bool ok = ToJSValue(aCx, status.mStatus, aOutValue);
    if (!ok) {
      aRv.NoteJSContextException(aCx);
    }
    return;

::: dom/media/eme/MediaKeyStatusMap.cpp:64
(Diff revision 1)
> +      JSString* s = JS_NewStringCopyZ(cx, value);
> +      aOutValue->setString(s);
> +      return;
>      }
>    }
> -
> +  aOutValue->setUndefined();

aOutValue.setUndefined();

::: dom/webidl/MediaKeyStatusMap.webidl:28
(Diff revision 1)
>  [Pref="media.eme.apiVisible"]
>  interface MediaKeyStatusMap {
>    iterable<ArrayBuffer,MediaKeyStatus>;
>    readonly attribute unsigned long size;
>    boolean has (BufferSource keyId);
> -  MediaKeyStatus get (BufferSource keyId);
> +  any get (BufferSource keyId);

Needs [Throws].

r=me with those fixed.

Comment 3

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

Comment on attachment 8775406 [details]
Bug 1289942 - Make MediaKeyStatusMap.get() return undefined for unknown keys.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67592/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/704aa782be4c
Make MediaKeyStatusMap.get() return undefined for unknown keys. r=bz

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/704aa782be4c