1128379 - [EME] improve error handling in MediaKeys::CreateSession

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[JW Wang [:jwwang] [:jw_wang]]

Per bug 1121332 comment 63, we would like to throw an error in MediaKeys::CreateSession() if any error occurs during new MediaKeySession(...).

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1128379_improve_error_handling_in_MediaKeys.createSession-v1.patch

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8557711 [details] [diff] [review]
1128379_improve_error_handling_in_MediaKeys.createSession-v1.patch

Review of attachment 8557711 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/eme/MediaKeyStatusMap.cpp
@@ +47,2 @@
>    if (NS_WARN_IF(!mMap)) {
> +    aRv.Throw(NS_ERROR_OUT_OF_MEMORY);

Throw an error if any failures during construction.

@@ +203,5 @@
>  nsresult
>  MediaKeyStatusMap::UpdateInternal(const nsTArray<CDMCaps::KeyStatus>& keys)
>  {
>    AutoJSAPI jsapi;
> +  if (NS_WARN_IF(!jsapi.Init(mParent))) {

Now we don't need to handle failures in JS::NewMapObject by checking |mMap| is not null.

::: dom/media/eme/MediaKeys.cpp
@@ +358,5 @@
>    }
>  }
>  
>  already_AddRefed<MediaKeySession>
> +MediaKeys::CreateSession(JSContext* aCx,

|aCx| will be passed all the way to the constructor of MediaKeyStatusMap so that we don't need an AutoJSAPI.

Comment 3

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

Comment on attachment 8557711 [details] [diff] [review]
1128379_improve_error_handling_in_MediaKeys.createSession-v1.patch

Review of attachment 8557711 [details] [diff] [review]:
-----------------------------------------------------------------

Probably best to get bz to look over this; I don't know much about JSContexts.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

try: https://tbpl.mozilla.org/?tree=Try&rev=29153744b0a3

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8557711 [details] [diff] [review]
1128379_improve_error_handling_in_MediaKeys.createSession-v1.patch

Looks good, thanks!

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Try is green: https://tbpl.mozilla.org/?tree=Try&rev=29153744b0a3

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7412d0c1968f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7412d0c1968f

Comment 9

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

https://hg.mozilla.org/releases/mozilla-beta/rev/88ab5bafc85a

Comment 10

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

Attachment: Beta patch

Patch for beta branch as part of EME platform uplift.

Comment 11

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

Comment on attachment 8572342 [details] [diff] [review]
Beta patch

Requesting retroactive approval for Beta landing as part of EME platform uplift.

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8572342 [details] [diff] [review]
Beta patch

Approved for Beta as part of EME platform uplift.