562599 - Add an event to notify that Addon.applyBackgroundUpdates has changed

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Currently, there's no way of listening for when addon.applyBackgroundUpdates changes. Would be great to have an event, either specific to this property, or a generic one that passes a list of what properties changed.

Comment 1

[Dave Townsend [:mossop]]

What do we lose by not having this Blair?

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Without this, we can't guarantee the automatic updates checkbox for each addon is in sync (without constant polling, that is). Mostly important with multiple copies of about:addons open, but other things could potentially change it too (thinking extensions - like the Tor addon disabling automatic updates for itself). Not needed for beta 1, but its something I want to be able to do before final.

We could make this a miscellaneous catch-all event for other properties that don't have events either. addon.blocklistState comes to mind. Something like onPropertyChanged(aAddon, aProperty)

Comment 3

[Dave Townsend [:mossop]]

Ah yeah I hadn't thought about the multiple tabs case much, makes sense.

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Added as a generic property changed event that may be useful for other things in the future.

The event is passed an array of property names that may have changed so we can dispatch a single event for multiple changes. Receivers should not assume that the named properties have actually changed since we may just want to dispatch this after some update method may or may not have made changes.

Comment 5

[Dave Townsend [:mossop]]

http://hg.mozilla.org/mozilla-central/rev/d8c1051908ad

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I would tend to say it should only be an automated test.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed based on check-in and passing tests.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #4)
> Receivers should not assume that
> the named properties have actually changed since we may just want to dispatch
> this after some update method may or may not have made changes.

Just as a for-the-record followup: Discussed this with Dave, and its best if consumers can rely on onPropertyChanged meaning that properties did in fact change. 

So the API *will* guarantee that properties changed, and consumers *can assume* that the properties changed. This was already the case for applyBackgroundUpdates, but any use of onPropertyChanged in the future will also need to guarantee this.