Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Description • 6 years ago

As noticed the ProductAddonChecker downloads the video codec plugins after some minutes of the first start. Sadly the created temporary files aren't removed. See bug 1467247.

But given that those codecs are not necessary for most of the audience of Selenium and geckodriver users, we should just disable the download.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 1 • 6 years ago

Nils, can you please help and tell us which preferences would be necessary to set to stop downloading those plugins? Thanks.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 2 • 6 years ago

Actually gmp seems to be the manager, and what we need to stop are widevine and h264. So far I was still not able to figure out the prefs which stop them all.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 3 • 6 years ago

This actually shouldn't block the 0.21 release given that we live with that already for a long time.

	Nils Ohlmeier [:drno]
	Comment 4 • 6 years ago

Based on quick code search and a brief test it looks to me like setting media.gmp-provider.enabled to false disables all GMP downloads.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 5 • 6 years ago

(In reply to Nils Ohlmeier [:drno] from comment #4)
> Based on quick code search and a brief test it looks to me like setting
> media.gmp-provider.enabled to false disables all GMP downloads.

No, both widevine and h264 are still getting downloaded. I only had to wait about 5 minutes. Maybe this is a bug then?

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 6 • 6 years ago

Nils and I checked that again and there is not a single pref to stop the gmp provider to download the plugins. As alternative we could set `media.gmp-manager.url` to an invalid URL.

For EME plugins we basically need another pref which is `media.eme.enabled`.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 7 • 6 years ago

(In reply to Henrik Skupin (:whimboo) from comment #6)
> For EME plugins we basically need another pref which is `media.eme.enabled`.

Interesting that the addon manager repository lists all of the above mentioned plugins as EME:

https://dxr.mozilla.org/mozilla-central/rev/75a32b57132f8cba42779555662a057a0416a313/toolkit/mozapps/extensions/internal/ProductAddonChecker.jsm#9-15

So why does disabling EME not also disable the openh264 plugin? Also I can set "media.gmp-manager.url.override" to an empty string or "media.gmp-gmpopenh264.enabled" to false, but Firefox always downloads this plugin.

There is something really wrong here in Firefox I assume. Personally I would only expect to have to set `media.eme.enabled` to `false` to stop all downloads. But as it looks like it is `media.gmp-manager.updateEnabled` which has to be set:

https://dxr.mozilla.org/mozilla-central/rev/75a32b57132f8cba42779555662a057a0416a313/toolkit/mozapps/extensions/internal/ProductAddonChecker.jsm#231-234

Stephen, maybe you could have another look? Let me know if I should file a bug for Firefox. Thanks.

	Stephen A Pohl [:spohl] (OOO until 9/8)
	Comment 8 • 6 years ago

:cpearce may be able to answer this. If not, I can take another look.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 10 • 6 years ago

The attached patch would be the solution which currently works for Nightly. It would still be good to get the feedback from Chris if this is the correct solution, or if something isn't working correctly in Firefox.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 11 • 6 years ago

(In reply to Henrik Skupin (:whimboo) from comment #7)
> (In reply to Henrik Skupin (:whimboo) from comment #6)
> > For EME plugins we basically need another pref which is `media.eme.enabled`.
> 
> Interesting that the addon manager repository lists all of the above
> mentioned plugins as EME:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 75a32b57132f8cba42779555662a057a0416a313/toolkit/mozapps/extensions/internal/
> ProductAddonChecker.jsm#9-15

This list is to provide a download location in the case where we can't connect to the GMP update server/AUS to get the latest list of GMP/CDM download locations. OpenH264 is not an EME plugin, so I think this list is misleadingly named; it's deliberately supposed to include both Widevine and OpenH264, and only one of those in an EME plugin.


> So why does disabling EME not also disable the openh264 plugin? Also I can
> set "media.gmp-manager.url.override" to an empty string or
> "media.gmp-gmpopenh264.enabled" to false, but Firefox always downloads this
> plugin.

OpenH264 is not an EME plugin, so it shouldn't be disabled when EME is disabled. OpenH264 is used by WebRTC, not EME.

If the download of the plugin list from AUS fails, we'll use the LOCAL_EME_SOURCES list to find a location to download the plugin from. (AUS was down for 3 weeks once, and that stopped new installs of Firefox from being able to play Netflix, which is why we have this fallback path for GMP/CDM downloads).

 
> There is something really wrong here in Firefox I assume. Personally I would
> only expect to have to set `media.eme.enabled` to `false` to stop all
> downloads. But as it looks like it is `media.gmp-manager.updateEnabled`
> which has to be set:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 75a32b57132f8cba42779555662a057a0416a313/toolkit/mozapps/extensions/internal/
> ProductAddonChecker.jsm#231-234
> 
> Stephen, maybe you could have another look? Let me know if I should file a
> bug for Firefox. Thanks.

Based on a quick look at the AddonChecker code, it seems that media.gmp-manager.updateEnabled is what you need to set. So I think your patch is the right approach.

I think the only thing wrong here is that LOCAL_EME_SOURCES is misleadingly named. I think it would be a good idea to rename it to LOCAL_GMP_SOURCES (or somesuch) while you're here to clear up future confusion.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 12 • 6 years ago

(In reply to Chris Pearce (:cpearce) from comment #11)
> If the download of the plugin list from AUS fails, we'll use the
> LOCAL_EME_SOURCES list to find a location to download the plugin from. (AUS
> was down for 3 weeks once, and that stopped new installs of Firefox from
> being able to play Netflix, which is why we have this fallback path for
> GMP/CDM downloads).

I see. Thank you for the explanation. But one more question... shouldn't `media.gmp-gmpopenh264.enabled` set to false disable the openh264 plugin, and including it's download and install?

> Based on a quick look at the AddonChecker code, it seems that
> media.gmp-manager.updateEnabled is what you need to set. So I think your
> patch is the right approach.

Good to know. So we will go ahead with this solution then.

> I think the only thing wrong here is that LOCAL_EME_SOURCES is misleadingly
> named. I think it would be a good idea to rename it to LOCAL_GMP_SOURCES (or
> somesuch) while you're here to clear up future confusion.

I can try to fix it on another bug or at least report it, once the other question from above has been answered.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 13 • 6 years ago

Oh, and something else. Do I have to set `media.gmp-manager.updateEnabled` to false before starting Firefox? As it looks like it's not evaluated again while Firefox is running.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 14 • 6 years ago

(In reply to Henrik Skupin (:whimboo) from comment #12)
> (In reply to Chris Pearce (:cpearce) from comment #11)
> > If the download of the plugin list from AUS fails, we'll use the
> > LOCAL_EME_SOURCES list to find a location to download the plugin from. (AUS
> > was down for 3 weeks once, and that stopped new installs of Firefox from
> > being able to play Netflix, which is why we have this fallback path for
> > GMP/CDM downloads).
> 
> I see. Thank you for the explanation. But one more question... shouldn't
> `media.gmp-gmpopenh264.enabled` set to false disable the openh264 plugin,
> and including it's download and install?

Yes, if that's not happening, then that's a bug.

(In reply to Henrik Skupin (:whimboo) from comment #13)
> Oh, and something else. Do I have to set `media.gmp-manager.updateEnabled`
> to false before starting Firefox? As it looks like it's not evaluated again
> while Firefox is running.

We should be checking for updates upon startup, and then once every 24 hours after that.

	Stephen A Pohl [:spohl] (OOO until 9/8)
	Comment 15 • 6 years ago

(In reply to Chris Pearce (:cpearce) from comment #14)
> (In reply to Henrik Skupin (:whimboo) from comment #12)
> > (In reply to Chris Pearce (:cpearce) from comment #11)
> > > If the download of the plugin list from AUS fails, we'll use the
> > > LOCAL_EME_SOURCES list to find a location to download the plugin from. (AUS
> > > was down for 3 weeks once, and that stopped new installs of Firefox from
> > > being able to play Netflix, which is why we have this fallback path for
> > > GMP/CDM downloads).
> > 
> > I see. Thank you for the explanation. But one more question... shouldn't
> > `media.gmp-gmpopenh264.enabled` set to false disable the openh264 plugin,
> > and including it's download and install?
> 
> Yes, if that's not happening, then that's a bug.

Looking at the code, this should still work as expected[1]. Was this pref set before or after startup? If this is set after startup, this check will have occurred before the pref was set and therefore the GMP would get installed.

[1] https://dxr.mozilla.org/mozilla-central/rev/e429320fcdd2d5236bb4713e6c435456146e42b9/toolkit/modules/GMPInstallManager.jsm#148-150

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 16 • 6 years ago

(In reply to Stephen A Pohl [:spohl] from comment #15)
> > Yes, if that's not happening, then that's a bug.
> 
> Looking at the code, this should still work as expected[1]. Was this pref
> set before or after startup? If this is set after startup, this check will
> have occurred before the pref was set and therefore the GMP would get
> installed.

Oh, it was set afterward. So it would explain it. Doing it before starting Firefox works now. But is there a reason why we do not have default preferences set for `media.gmp-gmpopenh264.enabled` and `media.gmp-gmpopenh264.visible` in firefox.js similar to widevine?

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 17 • 6 years ago

(In reply to Chris Pearce (:cpearce) from comment #14)
> (In reply to Henrik Skupin (:whimboo) from comment #13)
> > Oh, and something else. Do I have to set `media.gmp-manager.updateEnabled`
> > to false before starting Firefox? As it looks like it's not evaluated again
> > while Firefox is running.
> 
> We should be checking for updates upon startup, and then once every 24 hours
> after that.

Ok, in that case the patch is doing the right thing. Thanks!

	Andreas Tolfsen ❲:ato❳
	Comment 18 • 6 years ago
mozreview-review

Comment on attachment 8986002 [details]
Bug 1467700 - Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver.

https://reviewboard.mozilla.org/r/251388/#review258410

::: testing/geckodriver/src/prefs.rs:120
(Diff revision 1)
> +        // Disable download and usage of media DRM, h264, and Widevine plugins
> +        ("media.gmp-manager.updateEnabled", Pref::new(false)),
> +        ("media.gmp-manager.url.override", Pref::new("http://%(server)s/dummy/gmp/")),

I don’t think we need this at startup?

https://searchfox.org/mozilla-central/rev/d0a41d2e7770fc00df7844d5f840067cc35ba26f/toolkit/mozapps/extensions/internal/ProductAddonChecker.jsm#425

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 19 • 6 years ago
mozreview-review-reply

Comment on attachment 8986002 [details]
Bug 1467700 - Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver.

https://reviewboard.mozilla.org/r/251388/#review258410

> I don’t think we need this at startup?
> 
> https://searchfox.org/mozilla-central/rev/d0a41d2e7770fc00df7844d5f840067cc35ba26f/toolkit/mozapps/extensions/internal/ProductAddonChecker.jsm#425

Please see the recent converation on this bug. It is needed during startup.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 20 • 6 years ago
mozreview-review

Comment on attachment 8986002 [details]
Bug 1467700 - Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver.

https://reviewboard.mozilla.org/r/251388/#review258420

::: testing/geckodriver/src/prefs.rs:120
(Diff revision 1)
>          ("hangmonitor.timeout", Pref::new(0)),
>  
>          // Show chrome errors and warnings in the error console
>          ("javascript.options.showInConsole", Pref::new(true)),
>  
> +        // Disable download and usage of media DRM, h264, and Widevine plugins

This comment may be misinterpreted as saying that this disables our Android's MediaDRM playback infrastructure, and I don't think it does. So I think it would be clearest if this comment instead read "Disable download and install of OpenH264 and Widevine plugins."

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 21 • 6 years ago

(In reply to Chris Pearce (:cpearce) from comment #20)
> This comment may be misinterpreted as saying that this disables our
> Android's MediaDRM playback infrastructure, and I don't think it does. So I
> think it would be clearest if this comment instead read "Disable download
> and install of OpenH264 and Widevine plugins."

Sure, I will update that part.

	Andreas Tolfsen ❲:ato❳
	Comment 23 • 6 years ago
mozreview-review-reply

Comment on attachment 8986002 [details]
Bug 1467700 - Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver.

https://reviewboard.mozilla.org/r/251388/#review258410

> Please see the recent converation on this bug. It is needed during startup.

Well we also set it on startup, so I guess it depends on whichever
of mozapps vs. Marionette is loaded first.  Since the load order
isn’t guaranteed I guess we are forced to set it in the profile.
Yuk. )-:

	Andreas Tolfsen ❲:ato❳
	Comment 24 • 6 years ago
mozreview-review

Comment on attachment 8986002 [details]
Bug 1467700 - Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver.

https://reviewboard.mozilla.org/r/251388/#review258440

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 25 • 6 years ago
mozreview-review-reply

Comment on attachment 8986002 [details]
Bug 1467700 - Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver.

https://reviewboard.mozilla.org/r/251388/#review258410

> Well we also set it on startup, so I guess it depends on whichever
> of mozapps vs. Marionette is loaded first.  Since the load order
> isn’t guaranteed I guess we are forced to set it in the profile.
> Yuk. )-:

Given from the log files Marionette is always enabled later than any Add-on initialization, this includes setting the recommended prefs. So yes, that is the problem here.

But good thing is that we do not have to override the gmp manager URL. So I removed that part.

	Pulsebot
	Comment 26 • 6 years ago

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e407c763b97d
Disable download of OpenH264 and Widevine plugins for Marionette and geckodriver. r=ato

	Stephen A Pohl [:spohl] (OOO until 9/8)
	Comment 27 • 6 years ago

(In reply to Henrik Skupin (:whimboo) from comment #16)
> (In reply to Stephen A Pohl [:spohl] from comment #15)
> > > Yes, if that's not happening, then that's a bug.
> > 
> > Looking at the code, this should still work as expected[1]. Was this pref
> > set before or after startup? If this is set after startup, this check will
> > have occurred before the pref was set and therefore the GMP would get
> > installed.
> 
> Oh, it was set afterward. So it would explain it. Doing it before starting
> Firefox works now. But is there a reason why we do not have default
> preferences set for `media.gmp-gmpopenh264.enabled` and
> `media.gmp-gmpopenh264.visible` in firefox.js similar to widevine?

The reason behind this may have been more obvious in the past with more EME modules, but bug 1276132 and others make it clear that we wanted to display those prefs for EME modules, which is a subset of all the GMP modules. As :cpearce mentioned in comment 11, OpenH264 is not an EME module, so we don't display the pref by default.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 28 • 6 years ago

I see. Thanks.

	Natalia Csoregi [:nataliaCs]
	Comment 29 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/e407c763b97d