1624822 - Use only one RemoteSetting instance for all URL Classifier features

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, task)

Description

[Dimi Lee [:dimi]]

We should be able to use the same RemoteSetting instance for all the URL Classifier features. Besides that, we should also avoid syncing remote setting data for every features (See Bug 1617971 Comment 4)

Comment 1

[Dimi Lee [:dimi]]

Attachment: Bug 1624822 - Use only one RemoteSetting instance for all URL Classifier features

Comment 2

[Dimi Lee [:dimi]]

Attachment: Bug 1624822 - Do not request remote setting data for each URL classifier feature r=johannh,dimi

Comment 3

[:Gijs (out for now; he/him)]

Attachment: Bug 1624822 follow-up to fix eslint issues

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cffc27728f9e
Do not request remote setting data for each URL classifier feature r=Gijs

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0dc7fbadfe15
follow-up to fix eslint issues. CLOSED TREE

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/cffc27728f9e
https://hg.mozilla.org/mozilla-central/rev/0dc7fbadfe15

Comment 7

[:Gijs (out for now; he/him)]

This is getting backed out for causing bug 1627377. This is because I missed the fact that not all features get created immediately when the service initializes, and when features get created later, they get no initial data and so never properly initialize. I'll put up a combined patch that addresses this and request review.

Comment 8

[Cosmin Sabou [:CosminS]]

Backed out for causing Bug 1627377.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=296207604&group_state=expanded&searchStr=windows%2C10%2Cx64%2Cshippable%2Copt%2Cmochitests%2Ctest-windows10-64-shippable%2Fopt-mochitest-browser-chrome-e10s-7%2Cm%28bc7%29&tochange=cffc27728f9edce1393f3a0c1ec06e18d82b5ace&fromchange=449fd8a4035ec5f19127f2d042c6ee6d7b6bf740

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=296192718&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/890c9bc949ba110737e6d15804fb282705972f8e

[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - TEST-PASS | toolkit/components/antitracking/test/browser/browser_siteSpecificWorkArounds.js | callbackAfterRemoval must be passed when accessRemoval is non-null -
[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - Waiting for the skip list service to initialize...
[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - Buffered messages finished
[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/antitracking/test/browser/browser_siteSpecificWorkArounds.js | Uncaught exception - Skip list service initialized - timed out after 50 tries.
[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - Leaving test bound
[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - Entering test bound
[task 2020-04-04T11:43:38.176Z] 11:43:38 INFO - Cleaning up.
[task 2020-04-04T11:43:38.317Z] 11:43:38 INFO - Leaving test bound
[task 2020-04-04T11:43:38.318Z] 11:43:38 INFO - Entering test bound
[task 2020-04-04T11:43:38.319Z] 11:43:38 INFO - Starting non-blocking cookieBehavior (0) without allow list test localStorage with a tracker that is whitelisted via a pref running in a normal window with iframe sandbox set to null and access removal set to null
[task 2020-04-04T11:43:38.319Z] 11:43:38 INFO - TEST-PASS | toolkit/components/antitracking/test/browser/browser_siteSpecificWorkArounds.js | callbackAfterRemoval must be passed when accessRemoval is non-null -
[task 2020-04-04T11:43:38.359Z] 11:43:38 INFO - Waiting for the skip list service to initialize...
[task 2020-04-04T11:43:43.479Z] 11:43:43 INFO - Not taking screenshot here: see the one that was previously logged

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/890c9bc949ba

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/465fa77baabe
Do not request remote setting data for each URL classifier feature r=johannh

Comment 11

[Dimi Lee [:dimi]]

Attachment: Bug 1624822 - Do not request remote setting data for each URL classifier feature r=gijs

Comment 12

[Dimi Lee [:dimi]]

hi gijs,
Thank you for your help and sorry for not thinking and testing carefully while working on this.
I didn't realize that you already have a patch reviewed while working on my fix yesterday.
I wrote testcases anyway, maybe we can still use that part.

Comment 13

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/465fa77baabe