1306314 - Persistent CDM storage broken for Widevine

Status: VERIFIED FIXED

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

Description

[Kieron Allsop]

What did you do?
================
We are attempting to use setServerCertificate in section 5.1 Methods of the current EME W3C Candidate Recommendations by making use of the Queue a "message" Event algorithm (section 6.6.1) to obfuscate the device private data, in this case the private data is in the form of the Widevine Provider Client Token (PCT) which is generated server side.

1. Load this page
   https://urm.latens.com/hislop2
   (modified Shaka v.2 demo player which for a custom asset has had the player configuration altered to set persistentStateRequired = true)

2. Select custom asset from the drop down menu (last entry).

3. Apply these settings
	Username:               will
        Account:                will
        Portal UUID:            xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
        Custom manifest:        https://urm.latens.com/content/dashoutput/out.mpd
        Custom license server:  https://urm.latens.com/WvLicenseProxy

        Note: a list of provisioned Portal UUID is supplied below. 
	
4. Select Load.

5. Pause playback.

6. Select Load.

What happened?
==============
On second and subsequent loads there is no server certificate request/response happening and therefore the CDM is unable to pass the persisted application data to the Widevine proxy.

What should have happened?
==========================
After the initial playback, all subsequent playback attempts should result in the CDM making two requests to the proxy for each key it needs

1.)A request for the server certificate (public key) that is used to obfuscate the device private data (normally this is much shorter in length than the regular key request). Base64 encoded this equals CAQ=

2.)The key request itself with additional data.
On second and subsequent loads there should be a server certificate request which would enable playback. 

This is the behaviour in Chrome browser using the same CDM version.

Is there anything else we should know?
======================================
Related to Bug 1278198 - [Widevine] Use of EME defined 'persistentState' attribute not supported by the CDM on Firefox beta47

Provisioned Portal UUIDs (only one can be used per pysical device)

a99ed906-8646-11e6-ae22-56b6b6499611
a99edc80-8646-11e6-ae22-56b6b6499611
a99ee216-8646-11e6-ae22-56b6b6499611
a99ee3e2-8646-11e6-ae22-56b6b6499611
a99ee590-8646-11e6-ae22-56b6b6499611
a99ee720-8646-11e6-ae22-56b6b6499611
a99ee8e2-8646-11e6-ae22-56b6b6499611
a99eea72-8646-11e6-ae22-56b6b6499611
a99eefcc-8646-11e6-ae22-56b6b6499611
a99ef170-8646-11e6-ae22-56b6b6499611
a99ef328-8646-11e6-ae22-56b6b6499611
a99ef54e-8646-11e6-ae22-56b6b6499611
a99ef6de-8646-11e6-ae22-56b6b6499611
a99ef864-8646-11e6-ae22-56b6b6499611
a99efa12-8646-11e6-ae22-56b6b6499611
a99eff9e-8646-11e6-ae22-56b6b6499611
a99f0142-8646-11e6-ae22-56b6b6499611
a99f02c8-8646-11e6-ae22-56b6b6499611
a99f0444-8646-11e6-ae22-56b6b6499611
a99f05e8-8646-11e6-ae22-56b6b6499611

Comment 1

[karl.gallagher]

Hi,

I also have this issue - has this been confirmed as a bug yet ?

Karl.

Comment 2

[karl.gallagher]

Added Joey from the Google Widevine team

Comment 3

[joeyparrish]

I need to clarify one point.  In your bug report, you say you are using setServerCertificate, but you also say you are expecting a server certificate request.  These two are mutually exclusive.  If you call setServerCertificate, there is no need to request one from the server.

Are you actually using setServerCertificate?  Or are you only setting persistentStateRequired = true?

Comment 4

[karl.gallagher]

Hi,

Its the latter, sorry for the confusion.  The issue is that the (expected) automatically generated server certificate request event does not occur when persistentStateRequired = true

Comment 5

[joeyparrish]

Does this work as you expect in Chrome?  If you set persistentStateRequired = true in Chrome, do you get a server certificate request?  What happens in Chrome if persistentStateRequired = false?

Comment 6

[karl.gallagher]

With persistentStateRequired = false the behaviour on both Chrome and Firefox is that our Proxy will reject the license request due to the fact that we cannot track/manage the device, this is custom logic.

On Chrome we see the expected behaviour - in that once we create and pass a provider_client_token to the CDM it will always send a server certificate request before each subsequent license request so that it can embed the token securely.

On Firefox, whilst we can now create and pass back a provider_client_token (in beta version) - we never get a server certificate request prior to any further license requests and as such we don't get the token we created passed back to us in said license request.  We are assuming that it is related to the fact the the CDM can not encrypt the token. The behaviour this causes is that we create a new provider_client_token (for the same device), which results in undefined behaviour in our device management subsystems.

Comment 7

[joeyparrish]

Looks like a difference between the Firefox & Chrome integrations of the CDM, but I am not sure what the problem is.  CC'ing Chris.  Chris, please let me know if I can do anything to help.

Comment 8

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

Is this a regression in Firefox 52 Nightly? Do you see this bug in Firefox Release?

We're pushing to get the Web Platform Tests passing at the moment. We'll look at this once that's under control.

Comment 9

[karl.gallagher]

Chris,

I don't think this is a regression  - the issue was masked by https://bugzilla.mozilla.org/show_bug.cgi?id=1278198  since support for persistentState is a pre-requisite here.

Comment 10

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

There is not going to be a quick fix here.

The problem here is that Firefox has an assumption baked in that Widevine doesn't use persistent storage. We also can't handle running more than one CDM in the same process. So to solve for both of these constraints, we assign each CDM instance its own unique nodeId and storage container:

http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/dom/media/gmp/GMPServiceParent.cpp#1379

This means we'll create a new CDM with a unique storage location every time we create a CDM. i.e. persistent state for Widevine actually is persistented, but not able to be read-back at a later time.

We can't handle running multiple instances of the Widevine CDM same-origin concurrently because the Gecko Media Plugins API, which we adapt the Widevine CDM's implementation of the Chromium CDM API to, has separate interfaces for decoding and decryption, whereas the Chromium interface has one interface that does both.

We can't easily associate the decoding GMP interface to the corresponding decryption interface with the one Chromium CDM interface, so we just disallow running more than one CDM per process:

http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/dom/media/gmp/widevine-adapter/WidevineAdapter.cpp#97

So the fix is to re-architect our plugin harness so that it natively supports the Chromium CDM interface, rather than Adapting the CDM to our GMP APIs. I had wanted to do that anyway so that we could remove some excess copies of data. Here's another reason why we need to do that sooner rather than later.

Comment 11

[karl.gallagher]

Thanks for the detailed update Chris.

We are working with an operator who wishes to launch their service on Firefox. The content owners insist on device controls/management as part of the DRM system, which is currently blocking for us...

Can you perhaps provide a rough estimate of when the work required might be completed 

Thanks,
Karl.

Comment 12

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

I'd hope to get this fixed in Firefox 53, which ships on approximately 2017-04-18. 
https://wiki.mozilla.org/RapidRelease/Calendar

Comment 13

[karl.gallagher]

Thanks for the further update Chris.  We appreciate your help on this.

Comment 14

[Davy McAleer]

Hi Chris,
   Many thanks for your feedback and work on this issue to date. Given your last update around estimated timings(hopefully) for a fix for this problem, is it possible or likely that NPAPI support for Silverlight would remain past the March 2017 date that Benjamin Smedberg states here https://blog.mozilla.org/futurereleases/2016/07/20/reducing-adobe-flash-usage-in-firefox/ ?

Thanks
Davy

Comment 15

[Chris Peterson [:cpeterson]]

We plan to disable NPAPI support in Firefox 52 (bug 1269807), so it would be good to have this Widevine bug fixed in 52 or earlier so streamers can switch from Silverlight to Widevine. cpearce suggests fixing in 51 so streamers have time to test Widevine before 52 ships.

Changing priority from P3 to P1 because we need to fix this right away if we plan to uplift to Beta 51 (which starts next week).

Comment 16

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

Will try to split out a minimal fix that can be uplift, rather than tying fixing this to a large rearchitecturing of the CDM harness.

Comment 17

[Davy McAleer]

Many thanks Chris and Chris - appreciate the effort in prioritizing this. We'll look to retest once this is available in a beta/nightly build and give feedback from our point of view.

Thanks 
Davy

Comment 18

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

Attachment: Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

This enables us to identify GMPDecryptor instances in the child process, so that
in a later patch when we create a GMPVideoDecoder instance, we can associate it
with a GMPDecryptor. Then the cdm::ContentDecryptionModule8 instance that these
two actors are adapted to can know who it's supposed to respond to.

We use the IPDL PGMPDecryptorChild actor id as the GMPDecryptor's ID. This is unique
per GMP process, which is sufficient.

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

Comment 19

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

Attachment: Bug 1306314 - Pass decryptor ID to GMPVideoDecoder constructor.

Retrieve the ID of the GMPDecryptor from the GMPCDMProxy, and pass that
through to the GMPVideoDecoder's constructor.

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

Comment 20

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

Attachment: Bug 1306314 - Pipe decryptor ID through to WidevineAdapter.

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

Comment 21

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

Attachment: Bug 1306314 - Use decryptor ID in WidevineAdapter to link decryptors with decoders.

Store a mapping of decryptor ID to the CDM instance that the corresponding
WidevineDecryptor is using. This allows us to link GMPDecryptor instances
with the corresponding GMPVideoDecoder.

The CDM is stored inside the CDMWrapper, so that we destroy the CDM instance
when the last reference to the CDM is dropped.

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

Comment 22

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

Attachment: Bug 1306314 - Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage.

This change ensures that we don't create a new random node Id for every
MediaKeys object using Widevine - which has the effect of ensuring
Widevine CDMs that are same origin get created in the same process, and
that persistent storage can be used and retrieved.

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

Comment 23

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8810244 [details]
Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

https://reviewboard.mozilla.org/r/92586/#review92614

r+ with typos fixed, and a question:

::: dom/media/eme/CDMProxy.h:1
(Diff revision 1)
>  /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

In commit description:
- 'who' -> 'whom'
- 'actor id' -> 'actor ID' (for consistency, unless lowercase 'id' is intended in this case)

::: dom/media/gmp/GMPDecryptorChild.cpp:76
(Diff revision 1)
> +  // The ID of this decryptor is the IPDL actor ID. Note it's unique inside
> +  // the child process, but not necessaily across all gecko processes.

"not [unique] across processes" -- interesting information, but what are we doing about it? Are potential clashes handled elsewhere, or just deemed rare enough that it's not worth bothering?

::: dom/media/gmp/GMPDecryptorChild.cpp:77
(Diff revision 1)
>  GMPDecryptorChild::Init(GMPDecryptor* aSession)
>  {
>    MOZ_ASSERT(aSession);
>    mSession = aSession;
> +  // The ID of this decryptor is the IPDL actor ID. Note it's unique inside
> +  // the child process, but not necessaily across all gecko processes.

'necessaily' -> 'necessarily'

Comment 24

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

(In reply to Gerald Squelart [:gerald] from comment #23)
> Comment on attachment 8810244 [details]
> Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.
> ::: dom/media/gmp/GMPDecryptorChild.cpp:76
> (Diff revision 1)
> > +  // The ID of this decryptor is the IPDL actor ID. Note it's unique inside
> > +  // the child process, but not necessaily across all gecko processes.
> 
> "not [unique] across processes" -- interesting information, but what are we
> doing about it? Are potential clashes handled elsewhere, or just deemed rare
> enough that it's not worth bothering?

We're not going to get conflicts because we segregate the actors via nodeid, and nodeids are separated by origin. So we shouldn't get ids from different processes being used together, as we segregate the nodeids already.

Comment 25

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8c0edf11d35509c9518a954ff0dba2cfd25663f&selectedJob=30972990

Comment 26

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8810245 [details]
Bug 1306314 - Pass decryptor ID to GMPVideoDecoder constructor.

https://reviewboard.mozilla.org/r/92588/#review92616

Comment 27

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8810246 [details]
Bug 1306314 - Pipe decryptor ID through to WidevineAdapter.

https://reviewboard.mozilla.org/r/92590/#review92618

Comment 28

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8810247 [details]
Bug 1306314 - Use decryptor ID in WidevineAdapter to link decryptors with decoders.

https://reviewboard.mozilla.org/r/92592/#review92620

Comment 29

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8810248 [details]
Bug 1306314 - Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage.

https://reviewboard.mozilla.org/r/92594/#review92622

Comment 30

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

Comment on attachment 8810244 [details]
Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

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

Comment 31

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

Comment on attachment 8810245 [details]
Bug 1306314 - Pass decryptor ID to GMPVideoDecoder constructor.

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

Comment 32

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

Comment on attachment 8810246 [details]
Bug 1306314 - Pipe decryptor ID through to WidevineAdapter.

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

Comment 33

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

Comment on attachment 8810247 [details]
Bug 1306314 - Use decryptor ID in WidevineAdapter to link decryptors with decoders.

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

Comment 34

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

Comment on attachment 8810248 [details]
Bug 1306314 - Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage.

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

Comment 35

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

Comment on attachment 8810244 [details]
Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

https://reviewboard.mozilla.org/r/92586/#review92626

::: dom/media/gmp/GMPDecryptorChild.cpp:76
(Diff revision 1)
> +  // The ID of this decryptor is the IPDL actor ID. Note it's unique inside
> +  // the child process, but not necessaily across all gecko processes.

I'll add a comment explaining why this is OK.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e9727bdc484
Add an ID to GMPDecryptor instances, reflect that on CDMProxy. r=gerald
https://hg.mozilla.org/integration/autoland/rev/9343cb73c218
Pass decryptor ID to GMPVideoDecoder constructor. r=gerald
https://hg.mozilla.org/integration/autoland/rev/e2a5702d96b0
Pipe decryptor ID through to WidevineAdapter. r=gerald
https://hg.mozilla.org/integration/autoland/rev/d37e28309560
Use decryptor ID in WidevineAdapter to link decryptors with decoders. r=gerald
https://hg.mozilla.org/integration/autoland/rev/5c5600e1f30f
Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage. r=gerald

Comment 37

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2628e8fc7728
Backed out changeset 5c5600e1f30f for bustage
https://hg.mozilla.org/integration/autoland/rev/efb318045e32
Backed out changeset d37e28309560 
https://hg.mozilla.org/integration/autoland/rev/6adbbb16e04a
Backed out changeset e2a5702d96b0 
https://hg.mozilla.org/integration/autoland/rev/e2d9ef40d002
Backed out changeset 9343cb73c218 
https://hg.mozilla.org/integration/autoland/rev/852fd9e5d5fc
Backed out changeset 9e9727bdc484

Comment 38

[Iris Hsiao [:ihsiao]]

Sorry had to back out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=6561209&repo=autoland#L15076

Comment 39

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

Comment on attachment 8810244 [details]
Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

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

Comment 40

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

Comment on attachment 8810245 [details]
Bug 1306314 - Pass decryptor ID to GMPVideoDecoder constructor.

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

Comment 41

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

Comment on attachment 8810246 [details]
Bug 1306314 - Pipe decryptor ID through to WidevineAdapter.

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

Comment 42

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

Comment on attachment 8810247 [details]
Bug 1306314 - Use decryptor ID in WidevineAdapter to link decryptors with decoders.

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

Comment 43

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

Comment on attachment 8810248 [details]
Bug 1306314 - Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage.

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

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b0c612e8723
Add an ID to GMPDecryptor instances, reflect that on CDMProxy. r=gerald
https://hg.mozilla.org/integration/autoland/rev/893b62158c45
Pass decryptor ID to GMPVideoDecoder constructor. r=gerald
https://hg.mozilla.org/integration/autoland/rev/e183a3804fbb
Pipe decryptor ID through to WidevineAdapter. r=gerald
https://hg.mozilla.org/integration/autoland/rev/6a563a098351
Use decryptor ID in WidevineAdapter to link decryptors with decoders. r=gerald
https://hg.mozilla.org/integration/autoland/rev/b856b7987192
Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage. r=gerald

Comment 45

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2b0c612e8723
https://hg.mozilla.org/mozilla-central/rev/893b62158c45
https://hg.mozilla.org/mozilla-central/rev/e183a3804fbb
https://hg.mozilla.org/mozilla-central/rev/6a563a098351
https://hg.mozilla.org/mozilla-central/rev/b856b7987192

Comment 46

[Kieron Allsop]

Chris

Can you give me a link to the fixed version of Nightly so that I may test against our systems.

Thanks, Kieron

Comment 47

[Loic]

(In reply to Kieron Allsop from comment #46)
> Chris
> 
> Can you give me a link to the fixed version of Nightly so that I may test
> against our systems.
> 
> Thanks, Kieron

Daily Nightly is here: https://nightly.mozilla.org/

Comment 48

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

Kieron, Davy: Please can you download the latest Nightly from https://nightly.mozilla.org/ and test that? If you can confirm that this works, I'll uplift fixes so that they get ahead and into a version of Firefox which will ship sooner (51 hopefully).

Comment 49

[Kieron Allsop]

Chris, I can confirm that the latest Nightly works as expected in Windows 7 Professional SP1 32-bit, Windows 10 Professional 64-bit, and OSX 10.11.

Comment 50

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

Aurora Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff1f5c6bb60f5435e3415a4325e293db05c73944

Beta Try push: (including bug 1312540)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=57b0f61b0e6843af549bcd516792523686c5fdd4

Comment 51

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

Comment on attachment 8810244 [details]
Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

Approval Request Comment
[Feature/regressing bug #]: Widevine persistent state support
[User impact if declined]: Some sites that require "persistent state" for EME sessions will think they can use persistent state but actually fail to be able to use it, and thus fail to playback. Without this, some sites won't be able to find a non-Silverlight DRM playback solution which satisfies their streaming contracts' robustness requirements before we remove Silverlight support in 52. So this patch makes persistent state actually work for Widevine EME.
[Describe test coverage new/current, TreeHerder]: We have partial coverage here. It's hard to get complete coverage, as it would require having a licensed Widevine server.
[Risks and why]: Low; I've tried hard to minimize the changes required to make this work.
[String/UUID change made/needed]: None

Comment 52

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

Comment on attachment 8810245 [details]
Bug 1306314 - Pass decryptor ID to GMPVideoDecoder constructor.

Approval Request Comment
[Feature/regressing bug #]: Widevine persistent state support
[User impact if declined]: Some sites that require "persistent state" for EME sessions will think they can use persistent state but actually fail to be able to use it, and thus fail to playback. Without this, some sites won't be able to find a non-Silverlight DRM playback solution which satisfies their streaming contracts' robustness requirements before we remove Silverlight support in 52. So this patch makes persistent state actually work for Widevine EME.
[Describe test coverage new/current, TreeHerder]: We have partial coverage here. It's hard to get complete coverage, as it would require having a licensed Widevine server.
[Risks and why]: Low; I've tried hard to minimize the changes required to make this work.
[String/UUID change made/needed]: None

Comment 53

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

Comment on attachment 8810246 [details]
Bug 1306314 - Pipe decryptor ID through to WidevineAdapter.

Approval Request Comment
[Feature/regressing bug #]: Widevine persistent state support
[User impact if declined]: Some sites that require "persistent state" for EME sessions will think they can use persistent state but actually fail to be able to use it, and thus fail to playback. Without this, some sites won't be able to find a non-Silverlight DRM playback solution which satisfies their streaming contracts' robustness requirements before we remove Silverlight support in 52. So this patch makes persistent state actually work for Widevine EME.
[Describe test coverage new/current, TreeHerder]: We have partial coverage here. It's hard to get complete coverage, as it would require having a licensed Widevine server.
[Risks and why]: Low; I've tried hard to minimize the changes required to make this work.
[String/UUID change made/needed]: None

Comment 54

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

Comment on attachment 8810247 [details]
Bug 1306314 - Use decryptor ID in WidevineAdapter to link decryptors with decoders.

Approval Request Comment
[Feature/regressing bug #]: Widevine persistent state support
[User impact if declined]: Some sites that require "persistent state" for EME sessions will think they can use persistent state but actually fail to be able to use it, and thus fail to playback. Without this, some sites won't be able to find a non-Silverlight DRM playback solution which satisfies their streaming contracts' robustness requirements before we remove Silverlight support in 52. So this patch makes persistent state actually work for Widevine EME.
[Describe test coverage new/current, TreeHerder]: We have partial coverage here. It's hard to get complete coverage, as it would require having a licensed Widevine server.
[Risks and why]: Low; I've tried hard to minimize the changes required to make this work.
[String/UUID change made/needed]: None

Comment 55

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

Comment on attachment 8810248 [details]
Bug 1306314 - Allow Widevine CDM process to contain multiple CDM instances and to have persistent storage.

Approval Request Comment
[Feature/regressing bug #]: Widevine persistent state support
[User impact if declined]: Some sites that require "persistent state" for EME sessions will think they can use persistent state but actually fail to be able to use it, and thus fail to playback. Without this, some sites won't be able to find a non-Silverlight DRM playback solution which satisfies their streaming contracts' robustness requirements before we remove Silverlight support in 52. So this patch makes persistent state actually work for Widevine EME.
[Describe test coverage new/current, TreeHerder]: We have partial coverage here. It's hard to get complete coverage, as it would require having a licensed Widevine server.
[Risks and why]: Low; I've tried hard to minimize the changes required to make this work.
[String/UUID change made/needed]: None

Comment 56

[Gerry Chang [:gchang]]

Comment on attachment 8810244 [details]
Bug 1306314 - Add an ID to GMPDecryptor instances, reflect that on CDMProxy.

Since we plan to disable NPAPI support in Firefox 52 and in order to give streamers more time to switch from Silverlight to Widevine, we can take these patches in 51 early beta and see how it goes. Beta51 and Aurora52+. Should be in 51 beta 2.

Comment 57

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c767e0feceee
https://hg.mozilla.org/releases/mozilla-aurora/rev/465d8c6c7955
https://hg.mozilla.org/releases/mozilla-aurora/rev/25613f8d1f0f
https://hg.mozilla.org/releases/mozilla-aurora/rev/9c65ea0f38af
https://hg.mozilla.org/releases/mozilla-aurora/rev/95365d11eb81

Comment 58

[Carsten Book [:Tomcat]]

has problems to apply to beta

warning: conflicts while merging dom/media/gmp/GMPService.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Comment 59

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

https://hg.mozilla.org/releases/mozilla-beta/rev/8690a80013e0
https://hg.mozilla.org/releases/mozilla-beta/rev/274b64eef1cb
https://hg.mozilla.org/releases/mozilla-beta/rev/38ef1bd23de2
https://hg.mozilla.org/releases/mozilla-beta/rev/4d0a14c2c118
https://hg.mozilla.org/releases/mozilla-beta/rev/1bd1dd160782

Comment 60

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

Kieran Allsop, Karl Gallagher, this issue should be fixed in the next Firefox 51 beta build which is released Tuesday this week.

Firefox 51 ships to our release population on approximately 2017-01-24.

It would be great if you can download a Firefox 51 beta build on or after Wednesday and verify that persistent storage works as expected.

You can download Firefox beta here:
https://www.mozilla.org/en-US/firefox/channel/desktop/

Comment 61

[karl.gallagher]

I will provide feedback on this on Wednesday Chris.

Thanks,
Karl.

Comment 62

[karl.gallagher]

Chris.

This issued is confirmed fixed in Firefox 51 beta.

Tested on 

OSX 10.10
Windows 7 32-bit
Windows 10 64-bit

Comment 63

[chris.heathwood]

Confirmed here in Firefox 51 beta on OSX 10.11.6.

We will be testing this across platforms and will feedback if needed.

Thanks for this as it gives us a chance to do a good amount of testing before NPAPI is dropped.

Comment 64

[Ciprian Georgiu, Desktop QA]

Karl, Chris can you please confirm that this issue is fixed across platforms, in Firefox 52 (latest Aurora) and 53 (latest Nightly) as well? Thanks in advance.

Comment 65

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

Based on comment 49 and comment 62, I think we're good here.

Comment 66

[Ciprian Georgiu, Desktop QA]

It looks like I missed comment 42, sorry...
In this case, I will update the status flag for 51 and 53 and close this issue as verified fixed.