1146201 - [EME] Delay navigator.requestMediaKeySystemAccess while we wait for CDM to download

Status: RESOLVED FIXED

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

Description

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

If an EME CDM is enabled, but not yet downloaded, we should delay the resolution of the promise returned by navigator.requestMediaKeySystemAccess() until the CDM has finished downloading.

We can do this by just inspecting the prefs set by the GMP downloading code.

Comment 1

[Chris Peterson [:cpeterson]]

Note that our beta test partner chooses EME or Silverlight based on the User-Agent's version, not client-side feature detection. If they don't want to change their EME detection, can we postpone this fix until after MVP?

Comment 2

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

If we do this fix now, then a page using EME will eventually work, with a way for the streaming partner to detect this condition. If we don't do anything, the user will see an error instead, though the page will work if they refresh often enough to pass the 60s window.

It seems to me that adding an extra delay and having the page eventually work will probably be a better UX than having the page fail with an error.

Comment 3

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

We'll need to uplift this.

Comment 4

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

STR (32bit Firefox on Windows only):
1. Open Options > Content and toggle "Play DRM Content" off and on again.
2. Quit Firefox quickly (so it doesn't have time to download the Adobe CDM again).
3. Start Firefox.
4. Open http://drmtest2.adobe.com/HTML5AAXSPlayer/2/mse-access/ and press "Play" button
5. Observe "Firefox is installing components requried to play" drop down, and observe video playback doesn't start.

Expected result, eventually playback starts...

Comment 5

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

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

Comment 6

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

Attachment: Patch 1v1: Delay navigator.requestMediaKeySystemAccess until CDM downloads

* Create a delegate to delay creating/returning a MediaKeySystemAccess until the CDM is ready.
* When we get a "CDM needs update" or "CDM not yet installed", we'll wait for 60 seconds, or until the CDM is added to the GMPService by the GMP downloader (in Chrome code).
* If we retry a request after the GMPService reports its added a new GMP and the Adobe CDM still isn't available, we'll fail. I think this could only cause an unnecessary fail if we end up having an OpenH264 and Adobe CDM update happening at the same time, but that's probably acceptable.

Ehsan: Can you check the cycle collector stuff in MediaKeySystemAccessManager.cpp please and Navigator::Invalidate() changes? I'm confident JW can handle the rest.

Comment 7

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

Attachment: Patch 2v1: Make GMPProvider respond when updates are needed

Make GMPProvider listen for "mediakeys-request" observer service notification, and respond to "cdm-not-installed" or "cdm-insufficient-version" statuses by triggering a new update check.

Will the observer service work in e10s?

Comment 8

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

Attachment: Patch 3v1: Hide "can't play EME notification box" when we can play

With the above two patches, we can end up displaying a drmContentCDMInsufficientVersion or drmContentCDMInstalling and then recovering and playing EME content. It's weird to leave those notification boxes displaying when that happens so this patch hides them.

Comment 9

[Stephen A Pohl [:spohl]]

Comment on attachment 8584826 [details] [diff] [review]
Patch 2v1: Make GMPProvider respond when updates are needed

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

(In reply to Chris Pearce (:cpearce) from comment #7)
> Will the observer service work in e10s?

I believe so, but you should double-check with one of our e10s pros.

::: toolkit/mozapps/extensions/internal/GMPProvider.jsm
@@ +369,5 @@
> +      }
> +      this._isUpdateCheckPending = false;
> +    }, delay);
> +  },
> +  

nit: whitespace

@@ +374,5 @@
> +  observe: function(subject, topic, data) {
> +    let parsedData;
> +    try {
> +      parsedData = JSON.parse(data);
> +    } catch (ex) {

nit: no space between catch and (ex)

@@ +375,5 @@
> +    let parsedData;
> +    try {
> +      parsedData = JSON.parse(data);
> +    } catch (ex) {
> +      Cu.reportError("Malformed EME video message with data: " + data);

We should use the GMP logging here instead.

@@ +378,5 @@
> +    } catch (ex) {
> +      Cu.reportError("Malformed EME video message with data: " + data);
> +      return;
> +    }
> +    let {status: status, keySystem: keySystem} = parsedData;    

nit: whitespace

@@ +383,5 @@
> +    if (status == "cdm-not-installed" || status == "cdm-insufficient-version") {
> +      this.checkForUpdates(0);
> +    }
> +  },
> +  

nit: whitespace

Comment 10

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

Comment on attachment 8584822 [details] [diff] [review]
Patch 1v1: Delay navigator.requestMediaKeySystemAccess until CDM downloads

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

LGTM.

::: dom/base/Navigator.cpp
@@ +2636,4 @@
>    if (aRv.Failed()) {
>      return nullptr;
>    }
> +  if (!mMediaKeySystemAccessManager) {

Since we want to initialize mMediaKeySystemAccessManager lazily, move this if statement down to initialize mMediaKeySystemAccessManager as lazy as possible.

@@ +2641,2 @@
>    }
> +  if (aKeySystem.IsEmpty() || aOptions.WasPassed() && aOptions.Value().IsEmpty()) {

Warning: -Wparentheses in dom/base/Navigator.cpp: suggest parentheses around ‘&&’ within ‘||’

@@ +2645,3 @@
>    }
>  
> +  auto options = aOptions.WasPassed() ? aOptions.Value()

Take const reference instead of a value copy.

::: dom/base/Navigator.h
@@ +335,5 @@
>    RequestMediaKeySystemAccess(const nsAString& aKeySystem,
>                                const Optional<Sequence<MediaKeySystemOptions>>& aOptions,
>                                ErrorResult& aRv);
> +private:
> +  void RequestMediaKeySystemAccess(Promise* aPromise,

Where is this function defined?

@@ +339,5 @@
> +  void RequestMediaKeySystemAccess(Promise* aPromise,
> +                                   const nsAString& aKeySystem,
> +                                   const Optional<Sequence<MediaKeySystemOptions>>& aOptions);
> +
> +  nsRefPtr<MediaKeySystemAccessManager> mMediaKeySystemAccessManager;

Don't we need to add it to the cycle collection traverse?

::: dom/media/eme/MediaKeySystemAccessManager.cpp
@@ +110,5 @@
> +    aPromise->MaybeReject(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
> +    return;
> +  }
> +
> +  if (aOptions.IsEmpty() ||

We should move all checks for options (including those in Navigator::RequestMediaKeySystemAccess()) into MediaKeySystemAccess::IsSupported() so we have only one place to decide what options are valid and what are not.

@@ +171,5 @@
> +
> +void
> +MediaKeySystemAccessManager::RetryRequest(PendingRequest& aRequest)
> +{
> +  if (aRequest.mTimer) {

How could mTimer be null here?

@@ +230,5 @@
> +
> +  nsTArray<PendingRequest> requests(Move(mRequests));
> +  for (PendingRequest& request : requests) {
> +    // Cancel all requests; we're shutting down.
> +    if (request.mPromise) {

Is it possible for mPromise to be null?

@@ +233,5 @@
> +    // Cancel all requests; we're shutting down.
> +    if (request.mPromise) {
> +      request.mPromise->MaybeReject(NS_ERROR_DOM_INVALID_ACCESS_ERR);
> +    }
> +    if (request.mTimer) {

Ditto.

::: dom/media/eme/MediaKeySystemAccessManager.h
@@ +2,5 @@
> +* License, v. 2.0. If a copy of the MPL was not distributed with this
> +* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef mozilla_dom_MediaKeySystemAccessManager_h
> +#define mozilla_dom_MediaKeySystemAccessManager_h

The naming convention is inconsistent with the rest headers in dom/media/eme that don't include name space.

@@ +66,5 @@
> +
> +  void RetryRequest(PendingRequest& aRequest);
> +
> +  nsTArray<PendingRequest> mRequests;
> + 

space.

::: dom/media/gmp/GMPService.cpp
@@ +964,5 @@
>  
>    return gmp.get();
>  }
>  
> +class GMPAddedTask : public nsRunnable {

The class can be merged with StorageClearedTask by passing the topic string.

Comment 11

[(no longer active)]

Comment on attachment 8584822 [details] [diff] [review]
Patch 1v1: Delay navigator.requestMediaKeySystemAccess until CDM downloads

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

r- for the cycle collection issues.

::: dom/base/Navigator.h
@@ +339,5 @@
> +  void RequestMediaKeySystemAccess(Promise* aPromise,
> +                                   const nsAString& aKeySystem,
> +                                   const Optional<Sequence<MediaKeySystemOptions>>& aOptions);
> +
> +  nsRefPtr<MediaKeySystemAccessManager> mMediaKeySystemAccessManager;

Yes, this should indeed be added to the Navigator's cycle collection participant.

::: dom/media/eme/MediaKeySystemAccessManager.cpp
@@ +1,1 @@
> +#include "MediaKeySystemAccessManager.h"

Did you forget the license header of this file?

@@ +10,5 @@
> +namespace dom {
> +
> +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(MediaKeySystemAccessManager)
> +  NS_INTERFACE_MAP_ENTRY(nsIObserver)
> +  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIObserver)

Hmm, why can't you just use NS_INTERFACE_MAP_ENTRY(nsISupports)?  There is only one nsISupports base class here, so there should be no ambiguity.

@@ +19,5 @@
> +
> +NS_IMPL_CYCLE_COLLECTION_CLASS(MediaKeySystemAccessManager)
> +
> +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(MediaKeySystemAccessManager)
> +  tmp->Shutdown();

You should unlink the stuff that you traverse here.  Shutdown() doesn't seem to do that.  If you specialized ImplCycleCollectionUnlink for PendingRequest similar to what I described below, you could use the macros after you called Shutdown().

There is another issue here.  Cycle collected objects are not guaranteed to die through the cycle collector.  It's possible that the object gets deleted normally when its last reference is dropped.  So you should call Shutdown() in the destructor as well.

@@ +24,5 @@
> +NS_IMPL_CYCLE_COLLECTION_UNLINK_END
> +
> +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(MediaKeySystemAccessManager)
> +  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mWindow)
> +  for (size_t i = 0; i < tmp->mRequests.Length(); ++i) {

<https://dxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.h#1784> is a helper that lets you iterate over the array.  It would be nicer if you specialized ImplCycleCollectionTraverse for PendingRequest and get it to call ImplCycleCollectionTraverse on mPromise.  Then you could just do NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRequests) here and everything would work.

@@ +223,5 @@
> +  return mAddedObservers;
> +}
> +
> +void
> +MediaKeySystemAccessManager::Shutdown()

Once you call Shutdown() in the dtor, you should also ensure that it's safe to be called multiple times.  But I think that property holds for this function as it is.  Please just double check that.

::: dom/media/gmp/GMPService.cpp
@@ +964,5 @@
>  
>    return gmp.get();
>  }
>  
> +class GMPAddedTask : public nsRunnable {

Please make this final and add a private destructor.

@@ +966,5 @@
>  }
>  
> +class GMPAddedTask : public nsRunnable {
> +public:
> +  NS_IMETHOD Run() {

Please mark this as override.

Comment 12

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

Attachment: Patch 1v2: Delay navigator.requestMediaKeySystemAccess until CDM downloads

Reworked for Ehsan's and JW's comments.

Comment 13

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

Attachment: Interdiff of Patch 1 v1 vs v2

Interdiff of Patch 1 between v1 and v2.

Comment 14

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

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

Comment 15

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

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

Comment 16

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

> (In reply to Chris Pearce (:cpearce) from comment #7)
> > Will the observer service work in e10s?

Apparently the observer service does not work in e10s.

Comment 17

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

Attachment: Patch 2vx: Make GMPProvider respond when updates are needed

Review comments addressed. Carry forward r=spohl.

Comment 18

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

Attachment: Patch 4: Make GMPWrapper e10s compatible

Fix up my patch 2, so that we use the global message manager instead of the observer service. We should then work in e10s Firefox too.

Comment 19

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8585251 [details] [diff] [review]
Patch 4: Make GMPWrapper e10s compatible

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

looks fine to me.

Comment 20

[Carsten Book [:Tomcat]]

Hi, seems part 1 didn't apply cleanly:

renamed 1146201 -> delay-promise-resolution.patch
applying delay-promise-resolution.patch
patching file dom/base/Navigator.h
Hunk #2 FAILED at 103
1 out of 3 hunks FAILED -- saving rejects to file dom/base/Navigator.h.rej
patching file dom/media/eme/MediaKeySystemAccess.h
Hunk #1 FAILED at 16
1 out of 1 hunks FAILED -- saving rejects to file dom/media/eme/MediaKeySystemAccess.h.rej

Comment 21

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

Comment on attachment 8584827 [details] [diff] [review]
Patch 3v1: Hide "can't play EME notification box" when we can play

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

Florian is on PTO, Gigs (maybe also on PTO?) or Dolske, can you review this?

Comment 22

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

Attachment: Patch 1v3: Delay navigator.requestMediaKeySystemAccess until CDM downloads

Rebased to fix bitrot.

We still need ehsan's review before we can land.

Comment 23

[(no longer active)]

Comment on attachment 8585373 [details] [diff] [review]
Patch 1v3: Delay navigator.requestMediaKeySystemAccess until CDM downloads

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

::: dom/media/eme/MediaKeySystemAccessManager.cpp
@@ +51,5 @@
> +}
> +
> +MediaKeySystemAccessManager::~MediaKeySystemAccessManager()
> +{
> +  Shutdown();

Hmm, not sure what changed to make this unnecessary during Unlink now.  Note that Unlink clears out the array so you won't have a chance to reject the promises and whatnot if the object gets freed through the CC, which makes this seem incorrect to me.

Comment 24

[:Gijs (he/him)]

Comment on attachment 8584827 [details] [diff] [review]
Patch 3v1: Hide "can't play EME notification box" when we can play

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

(In reply to Chris Pearce (:cpearce) from comment #21)
> Gigs

I love autocomplete. Can you tell me which phone manufacturer I should file a bug with? :-)

::: browser/base/content/browser-eme.js
@@ +163,5 @@
> +     "drmContentCDMInstalling"
> +     ].forEach(function (value) {
> +        var notification = box.getNotificationWithValue(value);
> +        if (notification)
> +          box.removeNotification(notification);

I wonder if we should pass the second parameter. You can pass |true| in order to skip the animation for the closing, which might be more fluent if there are situations where we only show the notification bar very briefly?

Maybe I'm overthinking this, though - despite the loop, only one of these will ever show at the same time, and I don't know if there are cases where we show the one notification but only briefly...

@@ +164,5 @@
> +     ].forEach(function (value) {
> +        var notification = box.getNotificationWithValue(value);
> +        if (notification)
> +          box.removeNotification(notification);
> +      });    

Nit: trailing whitespace

Comment 25

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

Attachment: Patch 1v4: Delay navigator.requestMediaKeySystemAccess until CDM downloads

I'm having a hard time hitting ImplCycleCollectionTraverse etc specializations in a debugger, so I'll stick with a manual loop in the traverse and unlink...

Comment 26

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

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

Comment 27

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

(In reply to :Gijs Kruitbosch from comment #24)
> Comment on attachment 8584827 [details] [diff] [review]
> Patch 3v1: Hide "can't play EME notification box" when we can play
> 
> Review of attachment 8584827 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Chris Pearce (:cpearce) from comment #21)
> > Gigs
> 
> I love autocomplete. Can you tell me which phone manufacturer I should file
> a bug with? :-)

Error between sleep-deprived user and keyboard. ;)


> ::: browser/base/content/browser-eme.js
> @@ +163,5 @@
> > +     "drmContentCDMInstalling"
> > +     ].forEach(function (value) {
> > +        var notification = box.getNotificationWithValue(value);
> > +        if (notification)
> > +          box.removeNotification(notification);
> 
> I wonder if we should pass the second parameter. You can pass |true| in
> order to skip the animation for the closing, which might be more fluent if
> there are situations where we only show the notification bar very briefly

In my opinion we may as well animate it so that it's smooth and not jarring. I think this is indeed an edge case, so spending a lot of time on it isn't worthwhile. One option would be to set a timer in the "drmContentCDMInsufficientVersion" and "drmContentCDMInstalling" case and only show the notification after some number of elapsed seconds.

Up to you though, I'm not a Firefox dev. ;)

Comment 28

[(no longer active)]

Comment on attachment 8585819 [details] [diff] [review]
Patch 1v4: Delay navigator.requestMediaKeySystemAccess until CDM downloads

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

r=me with the below fixed.

::: dom/media/eme/MediaKeySystemAccessManager.cpp
@@ +28,5 @@
> +  for (size_t i = 0; i < tmp->mRequests.Length(); i++) {
> +    tmp->mRequests[i].RejectPromise();
> +    tmp->mRequests[i].CancelTimer();
> +    NS_IMPL_CYCLE_COLLECTION_UNLINK(mRequests[i].mPromise)
> +  }

Please empty out mRequests here so that Shutdown() called from the dtor doesn't end up calling RejectPromise and CancelTimer twice.

Comment 29

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

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

Comment 30

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

Attachment: Patch 1v5: Delay navigator.requestMediaKeySystemAccess until CDM downloads

Review comments addressed, rebased.

Comment 31

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

Attachment: Patch 2v3: Make GMPProvider respond when updates are needed

Review comments addressed, rebased.

Comment 32

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1765069b683
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ff9e8c6318d
https://hg.mozilla.org/integration/mozilla-inbound/rev/9862a2e404b4
https://hg.mozilla.org/integration/mozilla-inbound/rev/d566600d5569


peterv: This will bitrot your e10s patches.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c1765069b683
https://hg.mozilla.org/mozilla-central/rev/0ff9e8c6318d
https://hg.mozilla.org/mozilla-central/rev/9862a2e404b4
https://hg.mozilla.org/mozilla-central/rev/d566600d5569

Comment 34

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

Comment on attachment 8584827 [details] [diff] [review]
Patch 3v1: Hide "can't play EME notification box" when we can play

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: If the user happens to install an EME capable Firefox, or creates a clear profile in an EME capable Firefox, and immediately tries to play EME content, they'll get a "I can't play DRM video", until the Adobe EME plugin downloads. By default we check for updates and download the Adobe EME plugin 60 seconds after startup. With the patches in this bug, we will initiate the update check as soon as a page requests EME. This means users won't have to wait for 60 seconds after startup before we start the EME plugin download, it'll happen immediately.
[Describe test coverage new/current, TreeHerder]: We have test coverage for the existing code paths that are modified here. We don't have unit tests to cover the new code paths added here yet, we're working with Adobe on an end-to-end test that will test downloading and installing the EME plugin and playing EME videos.
[Risks and why]: Seems pretty low risk; this adds new code paths to cover an edge case, and the existing code path that is modified is well tested.
[String/UUID change made/needed]: None.

Comment 35

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

Comment on attachment 8585251 [details] [diff] [review]
Patch 4: Make GMPWrapper e10s compatible

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: If the user happens to install an EME capable Firefox, or creates a clear profile in an EME capable Firefox, and immediately tries to play EME content, they'll get a "I can't play DRM video", until the Adobe EME plugin downloads. By default we check for updates and download the Adobe EME plugin 60 seconds after startup. With the patches in this bug, we will initiate the update check as soon as a page requests EME. This means users won't have to wait for 60 seconds after startup before we start the EME plugin download, it'll happen immediately.
[Describe test coverage new/current, TreeHerder]: We have test coverage for the existing code paths that are modified here. We don't have unit tests to cover the new code paths added here yet, we're working with Adobe on an end-to-end test that will test downloading and installing the EME plugin and playing EME videos.
[Risks and why]: Seems pretty low risk; this adds new code paths to cover an edge case, and the existing code path that is modified is well tested.
[String/UUID change made/needed]: None.

Comment 36

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

Comment on attachment 8585926 [details] [diff] [review]
Patch 1v5: Delay navigator.requestMediaKeySystemAccess until CDM downloads

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: If the user happens to install an EME capable Firefox, or creates a clear profile in an EME capable Firefox, and immediately tries to play EME content, they'll get a "I can't play DRM video", until the Adobe EME plugin downloads. By default we check for updates and download the Adobe EME plugin 60 seconds after startup. With the patches in this bug, we will initiate the update check as soon as a page requests EME. This means users won't have to wait for 60 seconds after startup before we start the EME plugin download, it'll happen immediately.
[Describe test coverage new/current, TreeHerder]: We have test coverage for the existing code paths that are modified here. We don't have unit tests to cover the new code paths added here yet, we're working with Adobe on an end-to-end test that will test downloading and installing the EME plugin and playing EME videos.
[Risks and why]: Seems pretty low risk; this adds new code paths to cover an edge case, and the existing code path that is modified is well tested.
[String/UUID change made/needed]: None.

Comment 37

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

Comment on attachment 8585927 [details] [diff] [review]
Patch 2v3: Make GMPProvider respond when updates are needed

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: If the user happens to install an EME capable Firefox, or creates a clear profile in an EME capable Firefox, and immediately tries to play EME content, they'll get a "I can't play DRM video", until the Adobe EME plugin downloads. By default we check for updates and download the Adobe EME plugin 60 seconds after startup. With the patches in this bug, we will initiate the update check as soon as a page requests EME. This means users won't have to wait for 60 seconds after startup before we start the EME plugin download, it'll happen immediately.
[Describe test coverage new/current, TreeHerder]: We have test coverage for the existing code paths that are modified here. We don't have unit tests to cover the new code paths added here yet, we're working with Adobe on an end-to-end test that will test downloading and installing the EME plugin and playing EME videos.
[Risks and why]: Seems pretty low risk; this adds new code paths to cover an edge case, and the existing code path that is modified is well tested.
[String/UUID change made/needed]: None.

Comment 38

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8585927 [details] [diff] [review]
Patch 2v3: Make GMPProvider respond when updates are needed

Taking it as I guess this scenario is going to happen a lot of time...

Should be in beta 2

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/780ccb188336
https://hg.mozilla.org/releases/mozilla-aurora/rev/60a94093038f
https://hg.mozilla.org/releases/mozilla-aurora/rev/f50fad2de4a6
https://hg.mozilla.org/releases/mozilla-aurora/rev/6f68c7109d95

Comment 40

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

https://hg.mozilla.org/releases/mozilla-beta/rev/b7e7470e83b3
https://hg.mozilla.org/releases/mozilla-beta/rev/9383010b69fe
https://hg.mozilla.org/releases/mozilla-beta/rev/7a04aad0ab5d
https://hg.mozilla.org/releases/mozilla-beta/rev/c481e8a84a6c