1146565 - [EME] EME-free Firefox fails to uninstall any pre-existing CDMs

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Stephen A Pohl [:spohl]]

The EME-free Firefox repack planned in bug 1144903 will be unable to detect and uninstall any CDMs that have been downloaded to the profile. This can occur if the user ever runs a version of Firefox that's EME-enabled.

Although uninstall of CDMs has been implemented and landed in Firefox 38 (bug 1145694), this only works if the global EME pref is toggled at runtime. The repack toggles this pref before Firefox is launched, so we will be unable to detect the switch and will fail to uninstall any CDMs.

This could, in theory, also impact users that toggle this pref manually in their prefs.js file before launching Firefox. However, this scenario is assumed to be pretty rare.

Comment 1

[Stephen A Pohl [:spohl]]

Attachment: Patch

Comment 2

[Stephen A Pohl [:spohl]]

[Tracking Requested - why for this release]:

The EME-free repack (planned in bug 1144903) should be able to remove EME CDMs from disk if any are pre-existing. This is to satisfy the promise of actually being DRM-free.

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8582458 [details] [diff] [review]
Patch

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

Tests?

Comment 4

[Stephen A Pohl [:spohl]]

Thanks, Dave! As discussed on IRC I'll file a new bug to write more tests for uninstall of GMPs in general, and this bug in particular.

Comment 5

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/fx-team/rev/b78aa57b284e

Comment 6

[Stephen A Pohl [:spohl]]

(In reply to Stephen Pohl [:spohl] from comment #4)
> Thanks, Dave! As discussed on IRC I'll file a new bug to write more tests
> for uninstall of GMPs in general, and this bug in particular.

Filed bug 1147008 (blocking our meta uninstall bug 1142655).

Comment 7

[Stephen A Pohl [:spohl]]

Attachment: Patch for aurora

Approval Request Comment
[Feature/regressing bug #]: Adobe EME
[User impact if declined]: The DRM-free repack (bug 1144903) would be unable to remove any pre-existing CDMs and may fall short of the promise to deliver a DRM-free experience.
[Describe test coverage new/current, TreeHerder]: Local testing confirms proper deletion of pre-existing CDMs. We have existing mochitest and xpcshell tests to verify continued proper functioning of our GMP downloader and addons manager integration.
[Risks and why]: minimal
[String/UUID change made/needed]: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Backed out for test_blocklistchange.js xpcshell failures.
https://hg.mozilla.org/integration/fx-team/rev/79d8deba1d30

https://treeherder.mozilla.org/logviewer.html#?job_id=2390795&repo=fx-team

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Sorry, I grabbed the wrong cset. Re-landed.
https://hg.mozilla.org/integration/fx-team/rev/068c46a5ad3f

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/068c46a5ad3f

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 38 and 39. 

It would be good to do extra verification on this to make sure it's working smoothly on 38.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bc0f9cfd2831

Comment 13

[Ada [:adalucinet]]

Reproduced with Nightly 2015-03-24 under Windows 7 x64: gmp-eme-adobe\5 is still present in Profile Folder after reopening browser with EME disabled (via prefs.js)

Verified with Firefox 38 beta 1 (Build ID: 20150330154247) under Windows 7 x64 and 8.1 x32 with the above approach and, also, by toggling "Play DRM" in Preferences (bug 1145694) -> the gmp-eme-adobe folder is empty, but still present in profile folder - is this expected? shouldn't it be vanished too?

Comment 14

[Stephen A Pohl [:spohl]]

We currently use the same code path to remove GMPs as during updates, which didn't have a need to remove the top-level gmp-eme-adobe directory. So, this is expected, although unintentionally.

We've previously discussed adding a "GMP cleanup" feature, which would remove old GMPs that somehow were left on disk as well as remove empty directories etc. We don't currently have a bug filed for this because it became less of a priority once we started uninstalling GMPs during updates.

Justin, should we go ahead and file a "GMP cleanup" bug and remove the empty gmp-eme-adobe directory there? Or do you think we have to have this implemented for MVP?

Comment 15

[Ada [:adalucinet]]

Based on comment 14 (thanks, Stephen!), marking accordingly.
Also verified as fixed with latest Developer Edition 39.0a2 (from 2015-04-01) on Windows Vista x32 and Windows 7 x64.

Comment 16

[Justin Dolske [:Dolske]]

(In reply to Stephen Pohl [:spohl] from comment #14)

> Justin, should we go ahead and file a "GMP cleanup" bug and remove the empty
> gmp-eme-adobe directory there? Or do you think we have to have this
> implemented for MVP?

As long as it doesn't break anything, I think that's fine for MVP. (Eg, if you disable EME, we enter this state, and then the user reenables EME... Presumably nothing is confused or bothered by an existing/empty directory).

I don't think it's a significant issue from the perspective of "no DRM on my system!" users if there's an empty directory left over.

But would be good to have a bug on file, best to keep things tidy when possible.

Comment 17

[Stephen A Pohl [:spohl]]

Thanks, Justin!

(In reply to Justin Dolske [:Dolske] from comment #16)
> (In reply to Stephen Pohl [:spohl] from comment #14)
> 
> > Justin, should we go ahead and file a "GMP cleanup" bug and remove the empty
> > gmp-eme-adobe directory there? Or do you think we have to have this
> > implemented for MVP?
> 
> As long as it doesn't break anything, I think that's fine for MVP. (Eg, if
> you disable EME, we enter this state, and then the user reenables EME...
> Presumably nothing is confused or bothered by an existing/empty directory).

The empty folder did not seem to cause any problems when I tested it, i.e. re-enabling DRM downloaded and registered Adobe EME successfully. I also haven't seen any bug reports come in.

> I don't think it's a significant issue from the perspective of "no DRM on my
> system!" users if there's an empty directory left over.
> 
> But would be good to have a bug on file, best to keep things tidy when
> possible.

Filed bug 1152466.