We want to make it easier for CDM developers (Adobe, Google) to test their CDMs without having to build Firefox themselves with their keysystem configured on in their build config. So we should compile in the Widevine and Adobe specific code by default, but make them hidden behind a hidden pref, so that the keysystems aren't visible by default, but can be preffed on. The build config flags then will control turning on this pref. That means, users who want to test Widevine don't need an official Mozilla Firefox build, they can just pref it on. We need separate "visibility" and "enabled" flags, because for builds that don't have EME turned on in their mozconfig, we don't want the "click here to enable EME" UI to show.

Repurpose the media.gmp-*.forcevisible pref to control whether the corresponding GMP is visible in the addons manager UI. The pref has to be true for the GMP to be usable. The pref is enabled and not hidden when the corresponding EME keysystem is enabled in the mozconfig. This means users can turn on EME without needing to recompile their build; they just need to create a hidden pref. This will be useful for CDM developers, and users on platforms where we've not enabled EME yet but users want to test it (Linux). Review commit: https://reviewboard.mozilla.org/r/56448/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56448/

Instead of controlling visibility of EME keysystems by build config, do it by preference. This means keysystems can be turned on easier. Review commit: https://reviewboard.mozilla.org/r/56450/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56450/

This means we'll only show the EME UI for keysystems that are explicitly turned on in the build config, or those that are enabled after the build via prefs. Review commit: https://reviewboard.mozilla.org/r/56452/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56452/

Fixes compile errors on Linux when Widevine is compiled. Review commit: https://reviewboard.mozilla.org/r/56454/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56454/

Comment on attachment 8758073 [details] MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r?spohl https://reviewboard.mozilla.org/r/56448/#review53008 ::: toolkit/modules/GMPUtils.jsm:95 (Diff revision 1) > > return true; > }, > > /** > * Checks whether or not a given plugin is forced visible. This can be used This function comment should be updated as well.

Comment on attachment 8758074 [details] MozReview Request: Bug 1276132 - Remove use of #ifdef MOZ_{KEYSYSTEM}_EME in dom/media code. r=jwwang https://reviewboard.mozilla.org/r/56450/#review53032

Comment on attachment 8758076 [details] MozReview Request: Bug 1276132 - Make more Widevine class constructors explicit to keep gcc happy. r=jwwang https://reviewboard.mozilla.org/r/56454/#review53034

Comment on attachment 8758075 [details] MozReview Request: Bug 1276132 - Don't show EME 'enable' UI for non-visible keysystems. r=gijs https://reviewboard.mozilla.org/r/56452/#review53092 ::: browser/base/content/browser-media.js:39 (Diff revision 1) > + Services.prefs.getPrefType("media.gmp-eme-adobe.visible") && > + !Services.prefs.getBoolPref("media.gmp-eme-adobe.visible")) { > + return false; > + } > + if (keySystem == "com.widevine.alpha" && > + Services.prefs.getPrefType("media.gmp-widevinecdm.visible") && > + !Services.prefs.getBoolPref("media.gmp-widevinecdm.visible")) { I don't like being annoying, but if we're changing all these prefs now, can we change them to be consistent/predictable? We're using "cdm" in one, and "eme" in the other, in different parts of the pref name... it's a bit inconsistent. ::: browser/base/content/browser-media.js:41 (Diff revision 1) > + return false; > + } > + if (keySystem.startsWith("com.adobe") && > + Services.prefs.getPrefType("media.gmp-eme-adobe.visible") && > + !Services.prefs.getBoolPref("media.gmp-eme-adobe.visible")) { > + return false; Could simplify this and the next if statement to just: ``` if (keySystem.startsWith("com.adobe") && Services.prefs.getPrefType("media.gmp-eme-adobe.visible")) { return Services.prefs.getBoolPref("media.gmp-eme-adobe.visible"); ```

(In reply to :Gijs Kruitbosch from comment #10) > Comment on attachment 8758075 [details] > MozReview Request: Bug 1276132 - Don't show EME 'enable' UI for non-visible > keysystems. r?gijs > > https://reviewboard.mozilla.org/r/56452/#review53092 > > ::: browser/base/content/browser-media.js:39 > (Diff revision 1) > > + Services.prefs.getPrefType("media.gmp-eme-adobe.visible") && > > + !Services.prefs.getBoolPref("media.gmp-eme-adobe.visible")) { > > + return false; > > + } > > + if (keySystem == "com.widevine.alpha" && > > + Services.prefs.getPrefType("media.gmp-widevinecdm.visible") && > > + !Services.prefs.getBoolPref("media.gmp-widevinecdm.visible")) { > > I don't like being annoying, but if we're changing all these prefs now, can > we change them to be consistent/predictable? We're using "cdm" in one, and > "eme" in the other, in different parts of the pref name... it's a bit > inconsistent. Unfortunately no. Due to a quirk of the original GMP implementation, the name matters and is used in the path to the GMP. So we can't change it without rewriting a bunch of stuff relating to how the GMPs are stored on disk and loaded.

Are we going to stop offering EME-free builds?

Comment on attachment 8758073 [details] MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r?spohl Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56448/diff/1-2/

Comment on attachment 8758074 [details] MozReview Request: Bug 1276132 - Remove use of #ifdef MOZ_{KEYSYSTEM}_EME in dom/media code. r=jwwang Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56450/diff/1-2/

Comment on attachment 8758075 [details] MozReview Request: Bug 1276132 - Don't show EME 'enable' UI for non-visible keysystems. r=gijs Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56452/diff/1-2/

Comment on attachment 8758076 [details] MozReview Request: Bug 1276132 - Make more Widevine class constructors explicit to keep gcc happy. r=jwwang Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56454/diff/1-2/

(In reply to Masatoshi Kimura [:emk] from comment #12) > Are we going to stop offering EME-free builds? No. Behaviour in the EME-free repacks EME will not change. EME will still be off by default and the user will not be prompted to enable it should they visit a site that tries to use EME.

Comment on attachment 8758073 [details] MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r?spohl Re-requesting review on this, since I've changed it enough that I think it warrants another look. (I thought mozreview would have requested review again... must have gotten lost in the post somehow?)

Comment on attachment 8758073 [details] MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r?spohl https://reviewboard.mozilla.org/r/56448/#review54456

https://hg.mozilla.org/integration/mozilla-inbound/rev/69052d4e3dbb457219dad902a1ff9f1d98300c57 Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r=spohl https://hg.mozilla.org/integration/mozilla-inbound/rev/09b9972e36f9fb309a1f86918beeae26d1888aa6 Bug 1276132 - Remove use of #ifdef MOZ_{KEYSYSTEM}_EME in dom/media code. r=jwwang https://hg.mozilla.org/integration/mozilla-inbound/rev/91b3cdd0640a29eefc8619bf06e431dafe22c6ed Bug 1276132 - Don't show EME 'enable' UI for non-visible keysystems. r=gijs https://hg.mozilla.org/integration/mozilla-inbound/rev/511a2389ca48a403386aeb8f0025f26bab1e882e Bug 1276132 - Make more Widevine class constructors explicit to keep gcc happy. r=jwwang

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/75f65d79283ffe8fbfa797d62e64d7b797d20f6c for https://treeherder.mozilla.org/logviewer.html#?job_id=29841174&repo=mozilla-inbound etc.

(In reply to Phil Ringnalda (:philor) from comment #21) > Backed out in > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > 75f65d79283ffe8fbfa797d62e64d7b797d20f6c for > https://treeherder.mozilla.org/logviewer.html#?job_id=29841174&repo=mozilla- > inbound etc. 01:37:38 INFO - 563 INFO TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_gmpProvider.js | Got add-on element:gmp-eme-adobe - null == true - JS frame :: chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_gmpProvider.js :: testNotInstalledDisabled :: line 141 This is because I forgot to add KEY_PLUGIN_FORCE_SUPPORTED pref sets in the places where FORCE_VISIBLE used to be in this test.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a92a5799ecbfd9088472607c07626831de6996b0 Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r=spohl https://hg.mozilla.org/integration/mozilla-inbound/rev/066879c46800ff0aae008d2fbf3981f1e2fe309c Bug 1276132 - Remove use of #ifdef MOZ_{KEYSYSTEM}_EME in dom/media code. r=jwwang https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6fcc76216c86e1d7a5fa2f53ca5d37bcb647c8 Bug 1276132 - Don't show EME 'enable' UI for non-visible keysystems. r=gijs https://hg.mozilla.org/integration/mozilla-inbound/rev/dabfb32450b9602b1fb9df53c33f9eddb26e0d60 Bug 1276132 - Make more Widevine class constructors explicit to keep gcc happy. r=jwwang

https://hg.mozilla.org/mozilla-central/rev/a92a5799ecbf https://hg.mozilla.org/mozilla-central/rev/066879c46800 https://hg.mozilla.org/mozilla-central/rev/ea6fcc76216c https://hg.mozilla.org/mozilla-central/rev/dabfb32450b9

Comment on attachment 8758073 [details] MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r?spohl Approval Request Comment [Feature/regressing bug #]: Widevine EME on Linux [User impact if declined]: This patch changes how our build config options an prefs affect EME. Without this, it's not easy for users and Linux distros to turn off/on Widevein. [Describe test coverage new/current, TreeHerder]: We have plenty of EME mochitests, which run on Linux. [Risks and why]: Low; this is tweaking how we build EME code, and it's been riding the trains for a while. [String/UUID change made/needed]: None

Comment on attachment 8758074 [details] MozReview Request: Bug 1276132 - Remove use of #ifdef MOZ_{KEYSYSTEM}_EME in dom/media code. r=jwwang Approval Request Comment [Feature/regressing bug #]: Widevine EME on Linux [User impact if declined]: This patch changes how our build config options an prefs affect EME. Without this, it's not easy for users and Linux distros to turn off/on Widevein. [Describe test coverage new/current, TreeHerder]: We have plenty of EME mochitests, which run on Linux. [Risks and why]: Low; this is tweaking how we build EME code, and it's been riding the trains for a while. [String/UUID change made/needed]: None

Comment on attachment 8758075 [details] MozReview Request: Bug 1276132 - Don't show EME 'enable' UI for non-visible keysystems. r=gijs Approval Request Comment [Feature/regressing bug #]: Widevine EME on Linux [User impact if declined]: This patch changes how our build config options an prefs affect EME. Without this, it's not easy for users and Linux distros to turn off/on Widevein. [Describe test coverage new/current, TreeHerder]: We have plenty of EME mochitests, which run on Linux. [Risks and why]: Low; this is tweaking how we build EME code, and it's been riding the trains for a while. [String/UUID change made/needed]: None

Comment on attachment 8758076 [details] MozReview Request: Bug 1276132 - Make more Widevine class constructors explicit to keep gcc happy. r=jwwang Approval Request Comment [Feature/regressing bug #]: Widevine EME on Linux [User impact if declined]: This patch changes how our build config options an prefs affect EME. Without this, it's not easy for users and Linux distros to turn off/on Widevein. [Describe test coverage new/current, TreeHerder]: We have plenty of EME mochitests, which run on Linux. [Risks and why]: Low; this is tweaking how we build EME code, and it's been riding the trains for a while. [String/UUID change made/needed]: None

Comment on attachment 8758073 [details] MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to media.gmp-*.visible, and set it when keysystems are enabled. r?spohl Tweaks for the build for Widevine support. OK for beta uplift. Chris, do you have extra plans to test the build options?

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #29) > Comment on attachment 8758073 [details] > MozReview Request: Bug 1276132 - Rename media.gmp-*.forcevisible to > media.gmp-*.visible, and set it when keysystems are enabled. r?spohl > > Tweaks for the build for Widevine support. OK for beta uplift. > Chris, do you have extra plans to test the build options? I'm going to wait for a try push with these patches in to ensure they work before landing. These patches are in 50, so I'm pretty sure they'll work. I just want to be sure before landing. https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a6015957ee3&selectedJob=25428221

https://hg.mozilla.org/releases/mozilla-beta/rev/c2222ca05ad1 https://hg.mozilla.org/releases/mozilla-beta/rev/7b8879ab0e02 https://hg.mozilla.org/releases/mozilla-beta/rev/11b64780a7f5 https://hg.mozilla.org/releases/mozilla-beta/rev/1ac47043ac28

https://hg.mozilla.org/releases/mozilla-beta/rev/c2222ca05ad1 https://hg.mozilla.org/releases/mozilla-beta/rev/7b8879ab0e02 https://hg.mozilla.org/releases/mozilla-beta/rev/11b64780a7f5 https://hg.mozilla.org/releases/mozilla-beta/rev/1ac47043ac28