1513535 - Support the ability to separate feature toggle and list update in URL classifier

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, enhancement, P2)

Description

[Dimi Lee [:dimi]]

A requirement from GeckoView is that we should be able to separate the feature(tracking protection, safebrowsing...etc) toggle and list update so each session can decide if a feature is enabled or not and the runtime can define the lists it downloads.

Right now feature toggle is controlled by preference and URL classifier also uses the same preference to determine which list to download.

Comment 1

[Dimi Lee [:dimi]]

Hi Eugen,
As we talked at the All Hands, I created this bug to support separate feature enable and list update.

Would you mind checking if my understanding(in description) is correct and is there anything I missed?
It would be also great if you can describe more about how you gonna use it to make sure the implementation
will fit GeckoView's need, thanks!

Comment 2

[Eugen Sawin [:esawin]]

Currently, SafeBrowsing.jsm enables the features and list updates based on the same pref ("privacy.trackingprotection.enabled").

GeckoView doesn't set "privacy.trackingprotection.enabled" to control tracking protection, instead we set nsIDocShell.useTrackingProtection (see bug 1322576) via GeckoSessionSettings.
Additionally, we expose a blocklist selection setting in GeckoRuntimeSettings via "urlclassifier.trackingTable".

That means, for GeckoView, list selection and updates are handled in the GeckoRuntime while toggling TP on/off is handled per  GeckoSession which is subject to its own lifecycle independent from the GeckoRuntime.

We would like to remove the current workaround seen in [1], which overrides "privacy.trackingprotection.enabled" with an unrelated GeckoView-only pref which is set by default.
The new URLClassifier either needs a proper API to enable/disable features and list updates or at least separated prefs to control list updates and feature activity.

[1] https://searchfox.org/mozilla-central/source/toolkit/components/url-classifier/SafeBrowsing.jsm#237

Comment 3

[Dimi Lee [:dimi]]

Attachment: WIP.patch

Hi Eugen,
I have a patch that supports "force" update individual feature.
For tracking protection, it will be "browser.safebrowsing.features.trackingprotection.update"

So,

1. If is not set, it then depends on the feature's on/off preference to enable/disable update(just like what we have now)
2. If it is set, then we enable/disable update accordingly.

Does that work for you?

Comment 4

[Dimi Lee [:dimi]]

Attachment: WIP.patch

Comment 5

[Eugen Sawin [:esawin]]

That sounds good. Would it be safe to toggle those prefs during a running session, would a disable->enable switch force an update immediately?

I assume we want to enable all the relevant prefs in geckoview-prefs.js for now.

Comment 6

[Dimi Lee [:dimi]]

(In reply to Eugen Sawin [:esawin] from comment #5)

That sounds good. Would it be safe to toggle those prefs during a running session, would a disable->enable switch force an update immediately?

Yes, it should be safe to toggle those prefs during a session.
A disable-enable won't trigger an update immediately, the "nextupdatetime" should be still the same.

Comment 7

[Dimi Lee [:dimi]]

Attachment: Bug 1513535 - Support the ability to separate feature toggle and list update in URL classifier. r?gcp

Add preferences "browser.safebrowsing.features.[feature name].update".

Normally these preferences won't be set so the SafeBrowsing uses features's
enable/disable preferences to decide if it should update the list or
not.

If an update preference is present, then it has higher priority then the
enable/disable one.

This provides a way for the SafeBrowsing consumer to be able to separate
feature toggle and upodate.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f86cea87bf82
Support the ability to separate feature toggle and list update in URL classifier. r=gcp

Comment 9

[Stefan Hindli [:stefan_hindli]]

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