Closed Bug 1289942 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

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
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/
Attachment #8775406 - Flags: review?(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.
Attachment #8775406 - Flags: review?(bzbarsky) → review+
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/
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/704aa782be4c
Make MediaKeyStatusMap.get() return undefined for unknown keys. r=bz
https://hg.mozilla.org/mozilla-central/rev/704aa782be4c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.