Georg Fritzsche [:gfritzsche] Assignee
	Description • 10 years ago

As a follow-up to bug 1009909, we need to trigger an update of OpenH264 explicitly from the OpenH264Provider via the API from bug 1009909.

	Brian R. Bondy [:bbondy]
	Comment 1 • 10 years ago

Note that I'm doing a check for update at most daily on Firefox startup on delayed Firefox startup.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 3 • 10 years ago

This should be roughly what we want.
I currently hit issues with the test mocking on browser_openH264.js, line 215:
> TypeError: OpenH264Scope.OpenH264Wrapper is undefined

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 4 • 10 years ago

https://tbpl.mozilla.org/?tree=Try&rev=6be0c4747304

	Blair McBride [:Unfocused] (UNAVAILABLE)
	Comment 5 • 10 years ago

Comment on attachment 8459672 [details] [diff] [review]
Triggering updates, test coverage

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

::: toolkit/mozapps/extensions/internal/OpenH264Provider.jsm
@@ +16,5 @@
>  Cu.import("resource://gre/modules/Preferences.jsm");
>  Cu.import("resource://gre/modules/osfile.jsm");
>  Cu.import("resource://gre/modules/Log.jsm");
> +Cu.import("resource://gre/modules/Task.jsm");
> +Cu.import("resource://gre/modules/GMPInstallManager.jsm");

Should lazy-load this.

@@ +171,5 @@
>        aListener.onNoCompatibilityUpdateAvailable(this);
>      if ("onNoUpdateAvailable" in aListener)
>        aListener.onNoUpdateAvailable(this);
>      if ("onUpdateFinished" in aListener)
>        aListener.onUpdateFinished(this);

I had assumed this was temporary - but then implementing AddonInstall for this is overkill.

Can replace all those listener calls with just:
  AddonManagerPrivate.callNoUpdateListeners(this, aListener);

@@ +173,5 @@
>        aListener.onNoUpdateAvailable(this);
>      if ("onUpdateFinished" in aListener)
>        aListener.onUpdateFinished(this);
> +
> +    if (aReason !== AddonManager.UPDATE_WHEN_USER_REQUESTED ||

Should let through UPDATE_WHEN_PERIODIC_UPDATE too - some people leave their browsers open for a Very Long Time, so the automatic check in browser.js doesn't help much there (this was mentioned somewhere in bug 1009816).

	Blair McBride [:Unfocused] (UNAVAILABLE)
	Comment 6 • 10 years ago

(In reply to Blair McBride [:Unfocused] from comment #5)
> Should let through UPDATE_WHEN_PERIODIC_UPDATE too - some people leave their
> browsers open for a Very Long Time, so the automatic check in browser.js
> doesn't help much there (this was mentioned somewhere in bug 1009816).

Forgot to mention - the background update should only go ahead if the on-startup check hasn't happened in the past day.

And you *DO* need to check the state of applyBackgroundUpdates when doing background update checks. AddonManager.shouldAutoUpdate() is a useful helper function here. AddonManager.jsm normally does this for you, but only if you use onUpdateAvailable and implement AddonInstall.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 7 • 10 years ago

(In reply to Blair McBride [:Unfocused] from comment #6)
> (In reply to Blair McBride [:Unfocused] from comment #5)
> > Should let through UPDATE_WHEN_PERIODIC_UPDATE too - some people leave their
> > browsers open for a Very Long Time, so the automatic check in browser.js
> > doesn't help much there (this was mentioned somewhere in bug 1009816).
> 
> Forgot to mention - the background update should only go ahead if the
> on-startup check hasn't happened in the past day.
> 
> And you *DO* need to check the state of applyBackgroundUpdates when doing
> background update checks. AddonManager.shouldAutoUpdate() is a useful helper
> function here. AddonManager.jsm normally does this for you, but only if you
> use onUpdateAvailable and implement AddonInstall.

That is not the scope of this bug though, i'll file a follow-up.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 9 • 10 years ago

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

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 10 • 10 years ago

I think the easiest testing steps here are setting media.gmp-gmpopenh264.lastUpdate to something <1 day ago on a new profile before the automatic update/install happens.
It's an int pref, seconds since unix epoch - e.g. just set it to the result of |Date.now()/1000 - 60|.

Then trigger updates manually from either:
* the context menu on openh264 in the list view of the Addons Manager plugin category
* the detail view of openh264 - you need to turn automatic updates off to get the update link

	Ed Morley [:emorley]
	Comment 11 • 10 years ago

Backed out for exceptions in browser_openH264.js:
https://tbpl.mozilla.org/php/getParsedLog.php?id=44438505&tree=Fx-Team

remote:   https://hg.mozilla.org/integration/fx-team/rev/162e125d88e4

	Maire Reavy [:mreavy]
	Comment 12 • 10 years ago

Hi Georg -- Do we still need to land this or did this get landed in another bug (or was it overtaken by events)?

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 13 • 10 years ago

Thanks for the reminder Maire, this dropped off my radar over the recent busy schedule.

Fixed test and landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/3c052ee8d76a

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/3c052ee8d76a

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 15 • 10 years ago

Comment on attachment 8460326 [details] [diff] [review]
Triggering updates, test coverage

Approval Request Comment
[Feature/regressing bug #]: OpenH264 integration.
[User impact if declined]: Non-working manual triggering of update checks for the plugin.
[Describe test coverage new/current, TBPL]: Automated testing, manual spot-checking, up on m-c.
[Risks and why]: Low-risk, contained to the "find updates" actions in the addon manager UX.
[String/UUID change made/needed]: None.

	Randell Jesup [:jesup] (needinfo me)
	Comment 16 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/9dde3907962c

	Ada [:adalucinet]
	Comment 17 • 10 years ago

Verified as fixed with latest Aurora 33.0a2 and Nightly 34.0a1 builds on Windows 7 x64, Mac OS X 10.9.4 and Ubuntu 13.04 64bit (e.g.: https://pastebin.mozilla.org/5804582 - logs after openh264 is installed and Find Updates option is selected).