Closed Bug 1046644 Opened 10 years ago Closed 10 years ago

Honor OpenH264 autoupdate pref for the initial GMP install check


(Firefox :: General, defect)

Not set



Firefox 34
Tracking Status
firefox33 --- verified
firefox34 --- verified


(Reporter: gfritzsche, Assigned: bbondy)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

(Nils Ohlmeier [:drno] from bug 1044268, comment 1)
> 1) The request for a user pref which allows certain users or entities to opt
> for not even downloading the binary code of the plugin to their machines.

We do have an option to turn off auto updates in the detail view of the OpenH264 plugin entry.
It looks like we don't honor the backing pref (media.gmp-gmpopenh264.autoupdate) for the initial download check though.
Flags: firefox-backlog+
Brian, can you take this?
Flags: needinfo?(netzen)
Whiteboard: [openh264-uplift]
Yep I can take this but I don't know of any pref that currently exists for this. You just want me to add a new one right? And it would be one pref per addon ID? (media.{full-id}.autoupdate)
Flags: needinfo?(netzen)
Assignee: nobody → netzen
We do have the above-mentioned media.gmp-gmpopenh264.autoupdate pref - unset means "default", i.e. true, here.
The addon manager options for the OpenH264 plugin set this pref.

I assume we'll keep the pattern when we start supporting more GMP plugins, but it's hard to say for sure now.
Attached patch Patch rev1. (obsolete) — Splinter Review
Attachment #8465911 - Flags: review?(georg.fritzsche)
Comment on attachment 8465911 [details] [diff] [review]
Patch rev1.

Review of attachment 8465911 [details] [diff] [review]:

Can you add a test that no update/install happens for simpleCheckAndInstall() with the autoupdate pref off?
r=me with that.
Attachment #8465911 - Flags: review?(georg.fritzsche) → review+
Ya I didn't because I don't currently have a test at all for simpleCheckAndInstall. I'll do this before landing though. Thanks for the review!
Attached patch Patch rev2Splinter Review
Carrying forward r+.

Changes for rev2:
- Added promise return for simpleCheckAndInstall
- Added 3 tests for simpleCheckAndInstall for the 3 cases which are not errors but that don't install addons.

Manual tests performed: 
- Ran xpcshell locally
- Verified addons off via addon manager leads to a log message that indicates that and no addon installed.
- Verified addons default via addon manager leads to a log message that indicates success and that addons manager reflects that too.
Attachment #8466455 - Flags: review+
Running through try now, will land once that passes.
Flags: in-testsuite+
Attachment #8465911 - Attachment is obsolete: true
Closed: 10 years ago
Resolution: --- → FIXED
Iteration: --- → 34.2
QA Whiteboard: [qa?]
QA Whiteboard: [qa?] → [qa+]
QA Contact: drno
Comment on attachment 8466455 [details] [diff] [review]
Patch rev2

Approval Request Comment
[Feature/regressing bug #]: OpenH264

[User impact if declined]: Users will be unable to stop the browser from downloading a non-free plugin.  This is an issue of principle for some of our users, and might be a bandwidth issue for a few on very limited/shared or expensive connections.

[Describe test coverage new/current, TBPL]: Tested by QA.  In central for a week or so.  Includes several tests.

[Risks and why]: I believe the risk is low, but I'm not the expert on the frontend code.  From what I can see it's straightforward.

[String/UUID change made/needed]: none
Attachment #8466455 - Flags: approval-mozilla-aurora?
Attachment #8466455 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Contact: drno → florin.mezei
QA Contact: florin.mezei → alexandra.lucinet
Checked that automatic updates options work as expected before openh264 plugin is installed and after the plugin is installed. Tested on Windows 7 64bit, Ubuntu 14.04 32bit and Mac OS X 10.9.4 using latest Nightly and Aurora. Logs can be seen bellow: 

Windows 7 64bit
Before install:

After install:

Ubuntu 14.04 32bit
Before install:

After install:

Mac OS X 10.9.4
Before install:

After install:

Windows 7 64bit
Before install:

After install:

Ubuntu 14.04 32bit
Before install:

After install:

Mac OS X 10.9.4
Before install:

After install:
QA Whiteboard: [qa+] → [qa!]
Blocks: 1057990
The whole update code is still broken. With Firefox 33 it will manipulate several pref values and mark them as "user set" in about:config.

Those are media.gmp-gmpopenh264.lastUpdate, media.gmp-gmpopenh264.version, media.gmp-manager.lastCheck

I think it is broken to abuse user preferences to store persistent variables.
There is nothing broken with that. The preferences are used for storing state all over the place, "user set" just means "non-default value".
You need to log in before you can comment on or make changes to this bug.