Consider ensuring we sync and initialize the "other" blocklist immediately if the blocklist implementation is changed while the browser is up (e.g. due to pref flip study) and the other implementation's list hasn't been updated
Categories
(Toolkit :: Blocklist Implementation, defect, P1)
Tracking
()
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
As in summary. Right now, both the XML and remote settings implementation will check for new list items once every 24 hours while the browser is up (ie while the browser is up, every 24h, if the browser is shut down and reopened, and on open the last sync was >24h ago, we'll do another one within a few seconds). Additionally, the push notification mechanism for the remote settings implementation could potentially cause more frequent updates.
When we run pref flip experiments, this presents a risk in that for up to 24h, the list could be more out-of-date than we'd like (because we won't be updating the lists that aren't in use, ie no updates to the XML list when we're using RS, and vice versa).
This bug is about fixing this so we check for updates immediately when the pref is flipped.
Assignee | ||
Comment 1•5 years ago
|
||
It's unclear to me how this is passing on infra. After bug 1548542, as far as I can tell the only
time this test passes is when it somehow finishes before it does any of the filter testing.
The filter functions as-is (which predates that bug) does not allow these items to be in the
blocklist without something to identify what they're blocking (a guid/name for addons, a
matchXXXXX prop for plugins). Also, we no longer have a separate bucket pref for the extension
blocklist.
Before this patch, when this (rarely) passes for me on the local machine, it does so because
we bail out immediately after the initial run_test, as if the add_task functions have somehow
not registered. I do not understand why this would happen. In any case, after these changes,
we definitely run the rest of the test and it passes for me locally.
Assignee | ||
Comment 2•5 years ago
|
||
Depends on D35030
Assignee | ||
Updated•5 years ago
|
Comment 4•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/40860d14200d
https://hg.mozilla.org/mozilla-central/rev/eba0ea8c0a9e
Assignee | ||
Comment 5•5 years ago
|
||
Comment on attachment 9072179 [details]
Bug 1559353 - ensure XML and remote settings lists are updated immediately when switching between them, r?leplatrem,mconley
Beta/Release Uplift Approval Request
- User impact if declined: Potential delay in updating the addon/plugin/gfx blocklist if/when we run an experiment to switch its implementation from XML to the remote settings one. Plus associated muddying of the waters in terms of the telemetry we'd collect (ie it'd be hard to distinguish telemetry from users with one from users with the other because we don't know when the list is up-to-date).
- 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: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This adds code that only runs when flipping the pref that switches between the remote settings and XML blocklists. It shouldn't run at any other time, so we won't hit it except in experiments. The patch is comparatively small and there's an automated test. We're also working to get specific QA testing for this feature as a whole.
Note: unusually, uplift request ONLY for this patch, the other commit is test-only and only applies after bug 1548542 which is not on 68.)
- String changes made/needed: Nope
Comment 6•5 years ago
|
||
Comment on attachment 9072179 [details]
Bug 1559353 - ensure XML and remote settings lists are updated immediately when switching between them, r?leplatrem,mconley
blocklist fix, approved for 68.0b13
Comment 7•5 years ago
|
||
bugherder uplift |
Description
•