1230857 - 64-bit Adobe CDM works in Firefox 42 but is missing in Nightly 45: "will be installed shortly", but never does

Status: RESOLVED FIXED

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

Description

[Chris Peterson [:cpeterson]]

I'm having trouble downloading the Adobe CDM again. I am running Windows 8.1 in a VM.

My 64-bit Firefox 42 has downloaded the 64-bit CDM and can play Netflix successfully.

But using the same profile in 64-bit Nightly 45, Netflix does not play. The Add-ons Manager says the CDM "will be installed shortly", but they are not downloaded. If I toggled the "Play DRM content" setting to force the CDM to download, the following error is logged to the browser console:

Toolkit.GMP	ERROR	GMPInstallManager.simpleCheckAndInstall Could not check for addons: {"status":"failed","results":[{"id":"gmp-eme-adobe","result":"failed"},{"id":"gmp-gmpopenh264","result":"succeeded"}]}

If I force "Check for updates", the following error is logged to the browser console:

Toolkit.GMP	ERROR	GMPWrapper(gmp-eme-adobe) findUpdates() - updateTask for gmp-eme-adobe threw: {"target":{"zipPath":"C:\\Users\\Chris\\AppData\\Local\\Temp\\tmpaddon-11","installToDirPath":"C:\\Users\\Chris\\AppData\\Roaming\\Mozilla\\Firefox\\Profiles\\ot518l0y.default\\gmp-eme-adobe\\16","_deferred":{"promise":{}}},"status":{},"type":"exception"}

Comment 1

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

WFM in a clean profile.

I've observed on a few machines sporadically that sometimes the installer gets into a state where it doesn't have permissions to create the new CDM files. Will play around and see if I can find a permutation which repros this.

Comment 2

[Chris Peterson [:cpeterson]]

If I "refresh" my profile in about:support, then the CDM downloads successfully.

I think the problem might be related to using the same profile with multiple versions of Firefox. In particular, some profile data might be lost when IndexedDB schema versions are bumped and then you jump back in time from Nightly to an earlier channel.

Comment 3

[Chris Peterson [:cpeterson]]

Someone on Reddit reported a similar problem where the CDM disappeared and was re-downloaded every time they restarted their browser. I don't know if they were switching channels like I was.

https://www.reddit.com/r/firefox/comments/3wfujm/major_plugin_issues_in_nightly_firefox/

Comment 4

[alex_mayorga]

¡Hola Chris!

This bite me on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20151226030212 CSet: 4a559a618d6798eb9a8fdc559f5a7a00085e2062 as described on http://forums.mozillazine.org/viewtopic.php?p=14455295#p14455295

The errors logged are a tad different here:

1451359298536	Toolkit.GMP	ERROR	GMPInstallManager.simpleCheckAndInstall Could not check for addons: {"status":"failed","results":[{"id":"gmp-eme-adobe","result":"failed"}]} Log.jsm:751:0

1451359381019	Toolkit.GMP	ERROR	GMPWrapper(gmp-eme-adobe) findUpdates() - updateTask for gmp-eme-adobe threw: {"target":{"zipPath":"C:\\Users\\alex\\AppData\\Local\\Temp\\tmpaddon-22","installToDirPath":"C:\\Users\\alex\\AppData\\Roaming\\Mozilla\\Firefox\\Profiles\\nra2q7q2.Nightly\\gmp-eme-adobe\\16","_deferred":{"promise":{}}},"status":{},"type":"exception"} Log.jsm:751:0

There are a few profiles but AFAIK I've not mixed them.

From about:profiles:

Profile: clean
Default Profile	no
Root Directory	C:\Users\alex\AppData\Roaming\Mozilla\Firefox\Profiles\jeyd76ql.Clean
Local Directory	C:\Users\alex\AppData\Local\Mozilla\Firefox\Profiles\jeyd76ql.Clean

Profile: Nightly
This is the profile in use and it cannot be deleted.
Default Profile	yes
Root Directory	C:\Users\alex\AppData\Roaming\Mozilla\Firefox\Profiles\nra2q7q2.Nightly
Local Directory	C:\Users\alex\AppData\Local\Mozilla\Firefox\Profiles\nra2q7q2.Nightly

Profile: dev-edition-default
Default Profile	no
Root Directory	C:\Users\alex\AppData\Roaming\Mozilla\Firefox\Profiles\921uvt0o.dev-edition-default
Local Directory	C:\Users\alex\AppData\Local\Mozilla\Firefox\Profiles\921uvt0o.dev-edition-default

Profile: default-1442514914845
Default Profile	no
Root Directory	C:\Users\alex\AppData\Roaming\Mozilla\Firefox\Profiles\kll9yp2m.default-1442514914845
Local Directory	C:\Users\alex\AppData\Local\Mozilla\Firefox\Profiles\kll9yp2m.default-1442514914845

Do let me know if there's anything I could gather from the broken Nightly to try and help solve this bugger.

¡Gracias!

Comment 5

[alex_mayorga]

FWIW manually deleting these from C:\Users\alex\AppData\Roaming\Mozilla\Firefox\Profiles\nra2q7q2.Nightly\gmp-eme-adobe\16 then trying to play something on Netflix made it work again:

eme-adobe.dll
eme-adobe.info
eme-adobe.voucher

Comment 6

[Kacper Michajłow [:kasper93]]

(In reply to Chris Pearce (:cpearce) from comment #1)
> WFM in a clean profile.
> 
> I've observed on a few machines sporadically that sometimes the installer
> gets into a state where it doesn't have permissions to create the new CDM
> files. Will play around and see if I can find a permutation which repros
> this.

I can reproduce it everytime by simply disabling and re-enabling DRM support in settings. Until I manually remove "gmp-eme-adobe" folder CDM will not be installed.

I'm using latest 64-bit Developer Edition 45.0a2 (2016-01-06). I use only this version with my dev-edition profile.

Comment 7

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

I can repro if I load a profile in 64bit Firefox, ensure CDM installs, then load the same profile in 32bit Firefox and ensure the CDM installs, then load the profile in 64bit Firefox again and check for CDM updates. The CDM installed the first time the profile ran in 64bit mode is not deleted when we re-start in 32 bit mode, and it blocks install in 64bit mode subsequently. e10s was on.

Comment 8

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

The problem is that the 64bit Adobe CDM zip file is has its files set to what gets translated to be read-only by our unzip code, whereas the 32bit CDM ZIP file does not.

We should change our GMP install and delete code to make sure we set the CDM files' permissions to non-read-only, so we can be sure the updater can delete them when we update.

Note this means existing Firefox clients which have installed the x64 CDM will have their install read-only, meaning they'll not be able to uninstall and then re-install the CDM unless we patch their Firefox to enforce non-readonly permissions on the CDM files.

Comment 9

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

Attachment: MozReview Request: Bug 1230857 - Ensure GMPService has sufficient file permissions to delete GMPs. r?gerald

Without this, existing installs of the Adobe x64 Windows GMP from the zip file
where the GMP files are read-only will not be able to be installed.

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

Comment 10

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

Attachment: MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible permissions on GMP files at install time. r=spohl

This ensures that GMP packages with bad permissions will still be usable. For
example, a GMP without execute/read permissions in its zip won't work without
this.

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

Comment 11

[Stephen A Pohl [:spohl]]

Comment on attachment 8707721 [details]
MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible permissions on GMP files at install time. r=spohl

https://reviewboard.mozilla.org/r/30843/#review27623

::: toolkit/modules/GMPInstallManager.jsm:426
(Diff revision 1)
> +        outFile.permissions = parseInt("0700", 8);

Wouldn't it be preferable to ensure execute, write and read permissions for the file owner, rather than setting permissions to 700? I.e. something like:

outFile.permissions |= 0o700;

Or is there a reason why group and others shouldn't have any permissions and that's why it's explicitly set to 0?

Comment 12

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

Comment on attachment 8707721 [details]
MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible permissions on GMP files at install time. r=spohl

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

Comment 13

[Stephen A Pohl [:spohl]]

Comment on attachment 8707721 [details]
MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible permissions on GMP files at install time. r=spohl

https://reviewboard.mozilla.org/r/30843/#review27631

Note that you may want to apply the same change to GMPServiceParent.cpp (get current permissions, OR with 0700, set new permissions).

Comment 14

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

(In reply to Stephen A Pohl [:spohl] from comment #13)
> Comment on attachment 8707721 [details]
> MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible
> permissions on GMP files at install time. r?spohl
> 
> https://reviewboard.mozilla.org/r/30843/#review27631
> 
> Note that you may want to apply the same change to GMPServiceParent.cpp (get
> current permissions, OR with 0700, set new permissions).

Well, that's right before we delete the GMP files, so it should only be an issue if Gecko runs as someone other than the user, in which case adding 0700 won't help us delete, right?

Comment 15

[Stephen A Pohl [:spohl]]

(In reply to Chris Pearce (:cpearce) from comment #14)
> (In reply to Stephen A Pohl [:spohl] from comment #13)
> > Comment on attachment 8707721 [details]
> > MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible
> > permissions on GMP files at install time. r?spohl
> > 
> > https://reviewboard.mozilla.org/r/30843/#review27631
> > 
> > Note that you may want to apply the same change to GMPServiceParent.cpp (get
> > current permissions, OR with 0700, set new permissions).
> 
> Well, that's right before we delete the GMP files, so it should only be an
> issue if Gecko runs as someone other than the user, in which case adding
> 0700 won't help us delete, right?

Yes, that makes sense. Does it ever happen that the |directory| itself doesn't have write permissions? It looks like you're iterating over all the descendants, but don't ensure write permission on the root directory itself.

Comment 16

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

It should be created with write permissions here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#420

I will add write permission on the GMP dir itself too just in case something changed it.

Comment 17

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

Comment on attachment 8707720 [details]
MozReview Request: Bug 1230857 - Ensure GMPService has sufficient file permissions to delete GMPs. r?gerald

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

Comment 18

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

Comment on attachment 8707721 [details]
MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible permissions on GMP files at install time. r=spohl

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

Comment 19

[Gerald Squelart [:gerald] (he/him)]

Comment on attachment 8707720 [details]
MozReview Request: Bug 1230857 - Ensure GMPService has sufficient file permissions to delete GMPs. r?gerald

https://reviewboard.mozilla.org/r/30841/#review27641

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd8089d85751
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8f9f7aaf276

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dd8089d85751
https://hg.mozilla.org/mozilla-central/rev/a8f9f7aaf276

Comment 22

[Chris Peterson [:cpeterson]]

Should we uplift this fix to Aurora 45? With the 44 release just a week away, it's too late to uplift to Beta 44.

Comment 23

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

(In reply to Chris Peterson [:cpeterson] from comment #22)
> Should we uplift this fix to Aurora 45? With the 44 release just a week
> away, it's too late to uplift to Beta 44.

I will request uplift to Aurora 45.

Comment 24

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

Comment on attachment 8707720 [details]
MozReview Request: Bug 1230857 - Ensure GMPService has sufficient file permissions to delete GMPs. r?gerald

Requesting uplift to Firefox 45

Approval Request Comment
[Feature/regressing bug #]: EME/GMP/Netflix on Win64
[User impact if declined]: Users who switch from Win32 to Win64 to Win32 and back to Win64 in the same Firefox profile will find that the latest Adobe GMP doesn't install. This means users who switch between 32bit and 64bit Firefox on Windows will end up with broken Netflix when they're running in Win64 Firefox. This is caused a a file permissions problem, which the patches in this bug address.
[Describe test coverage new/current, TreeHerder]: We have lots of EME/GMP tests.
[Risks and why]: Fairly low; this just sets the file permissions before running the existing file manipulation code.
[String/UUID change made/needed]: None.

Comment 25

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

Comment on attachment 8707721 [details]
MozReview Request: Bug 1230857 - Make GMPInstallManager enforce sensible permissions on GMP files at install time. r=spohl

Requesting uplift to Firefox 45

Approval Request Comment
[Feature/regressing bug #]: EME/GMP/Netflix on Win64
[User impact if declined]: Users who switch from Win32 to Win64 to Win32 and back to Win64 in the same Firefox profile will find that the latest Adobe GMP doesn't install. This means users who switch between 32bit and 64bit Firefox on Windows will end up with broken Netflix when they're running in Win64 Firefox. This is caused a a file permissions problem, which the patches in this bug address.
[Describe test coverage new/current, TreeHerder]: We have lots of EME/GMP tests.
[Risks and why]: Fairly low; this just sets the file permissions before running the existing file manipulation code.
[String/UUID change made/needed]: None.

Comment 26

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8707720 [details]
MozReview Request: Bug 1230857 - Ensure GMPService has sufficient file permissions to delete GMPs. r?gerald

Sure, taking it

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6f733e3446a0
https://hg.mozilla.org/releases/mozilla-aurora/rev/8e913ace1e19