1090450 - Add-on background update always skipped

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Each time the background add-on update timer is reached, my add-ons do not update.

I investigated this by enabling "extensions.logging.enabled", and setting "extensions.update.interval" to a low value, and got the message:

"Skipping background update check"

every update interval.

It appears this was introduced by bug 995108 with a subtly broken refactoring.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Properly check add-on update state during update interval

This resolves the issue for me.

If you have suggestions on where to test this, I am happy to add it.  I haven't looked at Add-on Manager ever before, so I am not really sure where would be good.

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 8512924 [details] [diff] [review]
Properly check add-on update state during update interval

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

test_backgroundupdate.js should be verifying this but it now bypasses all the checks to force the update check to happen. Instead of calling AddonManagerPrivate.backgroundUpdateCheck directly we could have it call the addonManager.js component as an nsITimerCallback.

Bonus points for extending it to check the various different cases for having updates enabled but I'll take the simple test fix for now.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Properly check add-on update state during update interval (v2, Mossop: r+)

Added the simple test fix to use the timer callback.

I verified that the updated test would fail without the bug fix.

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=46d68ea32c4e

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

jryans is on pto today. Assuming this is good on m-c, what do you think about uplifting to 34 and 35, which are both marked as affected?

Comment 7

[Paul Rouget [:paul]]

Comment on attachment 8513206 [details] [diff] [review]
Properly check add-on update state during update interval (v2, Mossop: r+)

Yes. Let's uplift this bug.

Approval Request Comment
[Feature/regressing bug #]: regression: bug 995108 
[User impact if declined]: Addons are not auto-updated
[Describe test coverage new/current, TBPL]: in m-c
[Risks and why]: Low. Small patch.
[String/UUID change made/needed]: none

Comment 8

[Dave Townsend [:mossop]]

Yes, absolutely

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8513206 [details] [diff] [review]
Properly check add-on update state during update interval (v2, Mossop: r+)

Beta+
Aurora+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/491cde606eaa
https://hg.mozilla.org/releases/mozilla-beta/rev/06d2090db817