If cookieBehavior=0, and we load some third-party tracking cookies, the cookie blocking category in the protections panel moves to "Allowed" and the subpanel is empty - we should actually be showing the cross-site tracking cookies we're allowing. Locally, I have a patch to hide the cookie category when the subview is empty, so it shows up as "None Detected" which is wrong.

Test page: https://senglehardt.com/test/trackingprotection/test_pages/fingerprinting_and_cryptomining_and_cookies.html
(needs cookieBehavior=0)

I was able to confirm that during the load of this page, no content blocking event is received by the frontend with the STATE_COOKIES_LOADED bit set. The cookies are visible in the inspector.

My first theory as to why this is happening, was that we are returning early at [1] and hence never reach the NotifyBlockingDecision call at [2]. But there are other places where we seem to be calling NotifyBlockingDecision [3][4][5] when a cookie is allowed - but I'm not familiar enough with cookie code to diagnose why those don't seem to be working. [6] also seems relevant in that it is setting the bit on
nsSecureBrowserUIImpl::mEvent, which is consumed in nsGlobalWindowOuter::NotifyContentBlockingEvent [7].

[1] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/components/antitracking/AntiTrackingCommon.cpp#924
[2] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/toolkit/components/antitracking/AntiTrackingCommon.cpp#1043
[3] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/netwerk/cookie/nsCookieService.cpp#3116
[4] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/dom/base/Navigator.cpp#516
[5] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/netwerk/protocol/http/HttpChannelChild.cpp#1954
[6] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/security/manager/ssl/nsSecureBrowserUIImpl.cpp#237
[7] https://searchfox.org/mozilla-central/rev/34cb8d0a2a324043bcfc2c56f37b31abe7fb23a8/dom/base/nsGlobalWindowOuter.cpp#5479

FWIW, I think we should implement a STATE_COOKIES_LOADED_TRACKER to indicate to the frontend specifically that we loaded a third-party cookie that would otherwise have been blocked.

(In reply to Nihanth Subramanya [:nhnt11] from comment #0)

If cookieBehavior=0, and we load some third-party tracking cookies, the cookie blocking category in the protections panel moves to "Allowed" and the subpanel is empty - we should actually be showing the cross-site tracking cookies we're allowing. Locally, I have a patch to hide the cookie category when the subview is empty, so it shows up as "None Detected" which is wrong.

"wrong" is a bit of an oversimplification of what is happening here. :-)

Test page: https://senglehardt.com/test/trackingprotection/test_pages/fingerprinting_and_cryptomining_and_cookies.html
(needs cookieBehavior=0)

I was able to confirm that during the load of this page, no content blocking event is received by the frontend with the STATE_COOKIES_LOADED bit set. The cookies are visible in the inspector.

That is incorrect at least in my Nightly. with network.cookie.cookieBehavior = 0, all other prefs default, in the browser console after loading the above page I run this command and get the result below:

await gBrowser.selectedBrowser.getContentBlockingLog()
"{
 \"https://ads-track-digest256.dummytracker.org\": [[8192, true, 1], [32768, true, 1]],
 \"https://analytics-track-digest256.dummytracker.org\": [[8192, true, 1], [32768, true, 1]],
 \"https://social-track-digest256.dummytracker.org\": [[8192, true, 1], [32768, true, 1]],
 \"https://content-track-digest256.dummytracker.org\": [[8192, true, 1], [32768, true, 1]],
 \"https://base-fingerprinting-track-digest256.dummytracker.org\": [[64, true, 1]],
 \"https://base-cryptomining-track-digest256.dummytracker.org\": [[2048, true, 1]]
}
"

32768 is STATE_COOKIES_LOADED. So STATE_COOKIES_LOADED is definitely there.

Perhaps the problem you're having with STATE_COOKIES_LOADED is its name? It does not mean "cookies were loaded", but it means "the anti-tracking backend made some decision here." In the majority of the cases in practical web pages those decisions are made over cookies or storage but that is not necessarily the case, as you can see based on how the code works (see below).

My first theory as to why this is happening

There is no reason to guess here, FWIW. If you want to know exactly what the anti-tracking backend is doing when cookieBehavior is set to 0, you can run with MOZ_LOG=AntiTracking:5 and look at the anti-tracking log, it's quite useful to reveal what decisions are being made behind the scenes in situations like this. (Understanding the log still needs reading the code of course but at least you can be 100% sure the code took the paths that are indicated by the log output lines.)

, was that we are returning early at [1] and hence never reach the NotifyBlockingDecision call at [2].

That's a very exceptional call site for NotifyBlockingDecision and I don't believe it is important in what you're looking for. For most callers, NotifyBlockingDecision is called from here. The idea is that we have two classes of storage access check APIs, the "high level" APIs which are defined in StorageAccess.h and the "low level" API that is AntiTrackingCommon::IsFirstPartyStorageAccessGrantedFor(). The "high level" APIs always notify about the result of the storage access check.

The STATE_COOKIES_LOADED content blocking notification is actually a fake update that we send from two places when notifying about status updates for some reason (which is most of the time because some code used the high level storage access check APIs.)

But there are other places where we seem to be calling NotifyBlockingDecision [3][4][5] when a cookie is allowed

Yes, there is. I talked about that in the previous paragraph.

but I'm not familiar enough with cookie code to diagnose why those don't seem to be working. [6] also seems relevant in that it is setting the bit on
nsSecureBrowserUIImpl::mEvent, which is consumed in nsGlobalWindowOuter::NotifyContentBlockingEvent [7].

FWIW the bit you're seeing that is set on the browser secure UI object effectively collapses the STATE_COOKIES_LOADED bit across the top-level document and all of its descendant documents into one bit by ORing them together.

(In reply to Nihanth Subramanya [:nhnt11] from comment #1)

FWIW, I think we should implement a STATE_COOKIES_LOADED_TRACKER to indicate to the frontend specifically that we loaded a third-party cookie that would otherwise have been blocked.

I'm afraid you didn't really explain the higher level problem you're trying to solve in a way that I can understand yet, so I don't know what in the above picture isn't working in the way you expect it to and why such a new content blocking notification would be required, but per the above there were some misunderstandings in your previous assumptions, so please take another look and see if my explanations helped in any way?

If not then I'd appreciate if you could give a clear description of the problem in a way that I can reproduce?

FWIW about the STATE_COOKIES_LOADED_TRACKER suggestion, that is not really something that we can do very easily. What you're asking here requires us to run our anti-tracking checks, but actually not enforce them and only report their status. So instead of having two results for our checks ("allow" and "block") we would need to have additional outcomes such as ("allowbutwouldblock"), and then somewhere some code would be responsible to emit a STATE_COOKIES_LOADED_TRACKER if the outcome was "allowbutwouldblock" and our policy was BEHAVIOR_ACCEPT (aka cookieBehavior=0). This is quite an invasive change to the anti-tracking backend and I don't think is something that I would be willing to adopt since it complicates the code without a great justification. If the only requirement here is some kind of UX spec detail for handling BEHAVIOR_ACCEPT I would much rather ask the UX team to simplify the UX spec instead so that implementing it doesn't require such an invasive change to this part of the code, especially because starting from Firefox 69 BEHAVIOR_ACCEPT is a legacy configuration that the vast majority of our users will not be using, so spending a ton of resources to support it doesn't make sense.

If you disagree I'd be happy to discuss.

(In reply to :ehsan akhgari from comment #2)

Thanks a lot for the detailed response!

First of all, I want to apologize because this entire bug and my minimal investigation of it was based on a boolean expression with bad placement of parentheses so it's all a lie - we do indeed receive STATE_COOKIES_LOADED and all is fine. I made a mistake.

Second of all, thanks for the technical insight - regardless of whether this bug is relevant, I now have a better understanding of the code I was looking at the other day.

Lastly, I hear your concern about it not being worth it to add new flags that require us to run our checks only to report status. There is definitely a sizable cost vs benefit argument here and maybe the utility to most users is indeed not significant enough. Meanwhile, I need to figure out what the possibilities are for the UI given the constraints.

I think we can close this bug since the specific issue it refers to is invalid - if we decide we need more cookie state flags we can do that in another bug.

Ehsan, per our conversation yesterday, to summarize:

With cookieBehavior=4, we are currently using STATE_COOKIES_BLOCKED_TRACKER to determine whether tracking cookies are currently being blocked. This works well unless you add an exception, in which case tracking cookies are present but we only receive STATE_COOKIES_LOADED, so we can not tell whether these were first-party cookies, trackers or regular third-parties. In the old UI this wasn't an issue, but in the new UI it is.

In bug 1574111 we're fixing this by using STATE_LOADED_TRACKING_CONTENT to determine whether we should show the UI for tracking cookies, under the assumption that loaded tracking resources will probably end up setting cookies. Social tracking already employs this workaround as well.

Now obviously this isn't ideal because it's technically incorrect. Hence, we would love to add these flags to the contentBlockingEvent:

STATE_COOKIES_LOADED_TRACKER
STATE_COOKIES_LOADED_SOCIALTRACKER

Which could be triggered instead of STATE_COOKIES_LOADED when the associated channel has been annotated as a social/tracking domain.

Do you think that's doable?

Thanks!

I'm going to put together a patch for this. BTW after more discussion on slack, we decided to send the extra notifications in addition to STATE_COOKIES_LOADED not instead of.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79922efafb5bd93e21b838dc7410f7e644fad176

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