Closed Bug 1160382 Opened 9 years ago Closed 9 years ago

[EME] Check that GMP DLLs are present on disk before advertising GMP is available

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I'm concerned that anti-virus is intercepting our EME GMP downloads and preventing eme-adobe.dll from being deposited in its destination folder by the downloader.

We don't check that the DLL is there before adding the GMP to the list of GMPs we can instantiate, meaning MediaKeySystemAccess may think it can create Adobe EME's GMP, when actually the creation will fail.

GMPService::AddPluginDir() should verify that the GMP has its DLL or equivalent on disk before adding the GMP to the list of GMPs that can be instantiated.
Handling this anti-virus case is not a release blocker (for 38), but we should do something in 39. If beta testing on 39 shows it helps, we should ask to uplift it to 38.0.5, too.
We're seeing a small number of failures in our EME trials that indicate that Firefox/Gecko thinks the Adobe EME plugin is installed, but when Gecko comes to instantiate it, the DLL isn't there. 

So this patch makes us check whether the DLL is actually resident on disk before we instantiate it, toggles the prefs so Firefox will re-download the CDM next time the updater runs, and adds telemetry to report when it disappears.

We're unsure if this is an actual problem with anti-virus, or if the failure reports we got were from Adobe's QA messing with things, so that's why we want telemetry to determine whether this exact problem appears in the next trial.

r? edwin for MediaKeyAccess changes.
r? bsmedberg for adding telemetry.
Attachment #8601900 - Flags: review?(edwin)
Attachment #8601900 - Flags: review?(benjamin)
Attachment #8601900 - Flags: feedback?(spohl.mozilla.bugs)
Attachment #8601900 - Flags: feedback?(spohl.mozilla.bugs) → feedback+
Comment on attachment 8601900 [details] [diff] [review]
Patch: Check that Adobe's CDM is actually on disk before using it

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

We talked about the telemetry in the EME standup meeting; we'll do it differently, so I'll remove the telemetry from this patch, and land it. Dropping bsmedberg's review request.
Attachment #8601900 - Flags: review?(benjamin)
https://hg.mozilla.org/mozilla-central/rev/b3d61b6150fd
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
The patch that landed, with Telemetry removed.
Attachment #8601900 - Attachment is obsolete: true
Attachment #8603904 - Flags: review+
Comment on attachment 8603904 [details] [diff] [review]
Patch: Check that Adobe's CDM is actually on disk before using it

Requesting approval for uplift to 39. I've set the approval-beta? flag in case
this ends up not getting approved until after the m-c -> m-a merge next week.

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: We have some evidence to suggest that anti-virus software may be interfering with the download and install of Adobe's EME plugin. This patch makes Firefox only advertise that we support Adobe EME if the plugin DLL is physically present on disk at the time a site requests EME. Without this patch, users that have anti-virus which is interfering will probably try to start EME sessions that cannot succeed. With this patch, we won't advertise that Adobe EME is supported, and so the EME site should fallback to using an NPAPI plugin or somesuch.
[Describe test coverage new/current, TreeHerder]: We don't have a specific test for this, as we can't test third party plugins in our code base. However I am adding a test in my work in Bug 1156566 that should cover much of our behaviour here.
[Risks and why]: Low; we're just taking the existing "EME plugin not installed" code path when the DLL is missing
[String/UUID change made/needed]: None
Attachment #8603904 - Flags: approval-mozilla-beta?
Attachment #8603904 - Flags: approval-mozilla-aurora?
I need to not forget to uplift.
Flags: needinfo?(cpearce)
Comment on attachment 8603904 [details] [diff] [review]
Patch: Check that Adobe's CDM is actually on disk before using it

Approving for uplift to beta (39)
Attachment #8603904 - Flags: approval-mozilla-beta?
Attachment #8603904 - Flags: approval-mozilla-beta+
Attachment #8603904 - Flags: approval-mozilla-aurora?
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.