Closed Bug 1557790 Opened 5 years ago Closed 5 years ago

Remote settings blocklist might not initialize extensions/plugin blocklist

Categories

(Toolkit :: Blocklist Implementation, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox67 --- unaffected
firefox67.0.1 --- unaffected
firefox68 --- fixed
firefox69 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/toolkit/mozapps/extensions/Blocklist.jsm#2531-2537 triggers on startup (on idle).

However, that doesn't start the add-on/plugin blocklists.

I think I figured that that would happen either when the addon manager asks us about any add-ons (which is true, and it does -- iff the AM actually does that at any point).

In practice, however, on an ordinary startup I don't think the AM would ask the blocklist anything.

In principle, this is good - we don't do any work that isn't necessary. However, we do want the remote settings client to eventually update, and we also want any blocklist updates that start blocking add-ons to actually work, for existing clients that already have the add-on installed.

Mathieu, what's the expected way in which the RS client syncs and updates? Kris, am I right in thinking that the AM will never call into the blocklist unless the build updates and/or any add-ons/plugins get installed/updated for which it needs to do a check?

I expect that the RS blocklist code needs to at a minimum create the RS client objects to ensure they sync (which will trip update notifications to the AM which will then re-run checks on already-installed add-ons), but perhaps I'm wrong and RS itself already syncs without us instantiating clients, and this is a non-issue in practice?

Flags: needinfo?(mathieu)
Flags: needinfo?(kmaglione+bmo)

(In reply to :Gijs (he/him) from comment #0)

Kris, am I right in thinking that the AM will never call into the blocklist unless the build updates and/or any add-ons/plugins get installed/updated for which it needs to do a check?

Yes. That, or the blocklist is updated. Which I suppose isn't relevant here if the blocklist won't update if we don't touch it first.

Flags: needinfo?(kmaglione+bmo)
Priority: -- → P1

Mathieu, what's the expected way in which the RS client syncs and updates?

The sync can be triggered either from the update timer, or when a push notification is received. In both cases, we fetch the list of collections that were changed since last update, and then pull data for those which are known/enabled (see below).

[...] but perhaps I'm wrong and RS itself already syncs without us instantiating clients, and this is a non-issue in practice?

A collection is pulled from server in the following situations:

  • a client is instantiated (ref)
  • the bucket is main and there is either some local data or a JSON initial dump is packaged (ref)

In the case of addons/plugins blocklists, the bucket is not main (it's blocklists ref), so the clients must be instantiated for the collections to be taken into account.

Flags: needinfo?(mathieu)

In order for the remote settings blocklist to sync, we need to ensure that
the corresponding remote settings clients are created (see also
https://bugzilla.mozilla.org/show_bug.cgi?id=1557790#c2 ). This is
necessary because the blocklist clients are not in the main bucket.

This would otherwise happen as soon as any consumer asked the blocklist
for any block data, but that's not going to happen unless the list of
add-ons or plugins changes. Even if there are no changes to the local
lists of installed things, we do need blocklist updates because
otherwise already-installed items would never get blocked even if/when
they are added to the blocklist.

The client initialization should have no other side effects (in terms of
performance/cost) beyond ensuring they get included in things we ask for
when the update-timer fires.

Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3f2c369a3273
fix initialization of blocklist clients, r=aswan
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Comment on attachment 9071330 [details]
Bug 1557790 - fix initialization of blocklist clients, r?aswan!

Beta/Release Uplift Approval Request

  • User impact if declined: RemoteSettings implementation of addon/plugin (but not gfx) Blocklist fails to sync unless talked to by other bits of Firefox (so post-install blocks of plugins/add-ons might not take effect).
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: n/a
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a small JS-only change in a bit of well-tested code that affects when we initialize certain internal components. We're going to need this change on 68 if we want to eventually switch the implementation (after a lot of tire-kicking etc.) for 68esr. For now, all of this is turned off anyway, so there's almost no risk from that perspective either.

(I've not requested manual testing specific to this bug because there's a separate pending QE request for testing of the overall implementation, and I noticed this issue when I was writing testcases for that request. It doesn't seem useful to do the work twice.)

  • String changes made/needed: n/a
Attachment #9071330 - Flags: approval-mozilla-beta?

OK, really, this is disabled, but apparently to radar the approval request I should mark this affected/fixed, so I'm doing that.

Comment on attachment 9071330 [details]
Bug 1557790 - fix initialization of blocklist clients, r?aswan!

approved for 68.0b10

Attachment #9071330 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: