There is a lot here and I'm a bit lost as to what this bug is aiming to do, so here is some info in the hopes of reaching a consensus on what needs to be done here.
(In reply to Eugen Sawin [:esawin] from comment #0)
The current GV
ContentBlocking.onContentBlocked implementation predates Gecko's
onContentBlockingEvent is just a refactoring of the old
onSecurityChange API which used to be used for both dispatching security change notifications and content blocking notifications. Nothing too relevant changed during this refactor, so the above (
onContentBlockingEvent isn't super interesting...).
and depends on the barely supported
nsIClassifiedChannel.matchedList which doesn't give us the full range of today's content blocking categories.
Um, why do you think that?
nsIClassifiedChannel.matchedList is as supported as it ever was before, it gives you the matched safebrowsing table that was used to block an element. It is currently supported for the following content blocking back-ends:
- Tracking protection
- Safe browsing
- Fingerprinting blocking
- Cryptocurrency mining blocking
- Social tracking protection
Note that this is only supported for back-ends which block the loading of resources.
We could in theory provide this data for more backends, e.g. our "channel annotation" backends (for example the one used for cookie blocking/ETP) where we don't block the loading of resources if that is useful. (GeckoView is the only consumer of this data so this data so far is only available where GV has asked for it or where it has been cargo-cult copied...)
We should refactor the GV implementation to use
nsIWebProgressListener.onContentBlockingEvent which would enable the API to report a wider range of block event types, including cookie-based events as triggered by some cookie behavior settings (ETP).
Well, why do these two APIs need to be tied together? Currently GV manages to use
STATE_BLOCKED_TRACKING_CONTENT together with the
matchedList API without any issues, so why is using this API together with another content blocking code like
STATE_COOKIES_BLOCKED_TRACKER an issue?
For the refactoring, we will have to decouple
BlockEvent.categories from the
Category constants, since those only range from anti-tracking to safe browsing. The refactoring should allow for
BlockEvent to report the complete range of content blocking events, including cookies.
Not sure if I understand this part...
(In reply to Eugen Sawin [:esawin] from comment #1)
The issue with
nsIWebProgressListener.STATE_BLOCKED_TRACKING_CONTENT is that it does not allow us to differentiate between ad, analytic, social and content trackers. We will either have to split up
STATE_BLOCKED_TRACKING_CONTENT or continue mapping the GV categories based on
Is there a problem with the second option?
The first option is in theory doable, however it's not really a preferable choice because:
a) Most code wants to treat a type of blocking in a special way, not blocking of specific categories. As a case in point, currently the entire code in Firefox/Gecko wants to treat types and not categories in a special way. So breaking up the content blocking notification types per category will make it more difficult to write code that handles these notifications, since instead of e.g. handling one case for cookie blocking, you'd need to handle e.g. 4 cases.
b) The impact of changing our categorization would ripple through the entire code base, since the categorization would be hardcoded into content blocking notification codes we use in various parts of the code base. For example with the recent rapid changes we have been making to the social tracking protection categorization, each one of those changes would have turned into big and hard to review/get right patches.
c) It is quite a lot of work to do the refactoring suggested. Searching for onContentBlockingEvent in the code shows that this will be a lot of code changes...
Ehsan, are there any plans to provide more detailed (list-based) flags in
nsIWebProgressListener.onContentBlockingEvent? Otherwise, GV will need to continue doing the mapping itself based the matched lists, which will not work at all for bug 1567611.
See above, I'd really like to avoid expanding the content blocking notifications this way.
If that problem is what we're trying to solve here, why not just provide the
matchedList data there?
Alternatively, we could stop reporting at such detail, but that would the break the symmetry between the settings and the reporting, i.e., you can configure GV to block specific categories of trackers (ad, analytic, social, content) but the block reports would not give you this specific category information.
I think we can have our cake and eat it too. :-)