Once bug 1476715 has landed, all of the users of tracking annotations will be responsible for checking whether loads are first-party or third-party. We should simplify the logic in the channel classifier by removing all of the checks for StaticPrefs::privacy_restrict3rdpartystorage_enabled() or AntiTrackingCommon::ShouldActivate(). This will mean that the 3rd-party storage restrictions, like network.http.tailing.enabled and privacy.trackingprotection.lower_network_priority will be gated by privacy.trackingprotection.annotate_channels.

Can you please explain more about what the existing checks need to be replaced with? Simply removing the checks doesn't work, because of the logic in nsChannelClassifier::ShouldEnableTrackingProtectionInternal(). That logic currently ensures that we never annotate first-party channels under any circumstances, which is the reason why we need those pref checks.

(In reply to :Ehsan Akhgari from comment #1) > Can you please explain more about what the existing checks need to be > replaced with? > > Simply removing the checks doesn't work, because of the logic in > nsChannelClassifier::ShouldEnableTrackingProtectionInternal(). That logic > currently ensures that we never annotate first-party channels under any > circumstances, which is the reason why we need those pref checks. Now that bug 1476715 has landed, we can remove the centralized third-party check from ShouldEnableTrackingProtection() when aAnnotationsOnly==true since that third-party check has been distributed across all of the current users. This means that the default cookie restrictions annotations are no longer special.

Comment on attachment 8999245 [details] [diff] [review] Part 1: Remove the centralized third-party checks from nsChannelClassifier::ShouldEnableTrackingProtectionInternal Review of attachment 8999245 [details] [diff] [review]: ----------------------------------------------------------------- Bug 1476715 only added third-party checks to the consumers of tracking annotations. So if we take the check out entirely, we break tracking _protection_. Something else I just realized about https://hg.mozilla.org/mozilla-central/rev/cb8a7a9f4a73: we're only checking for third-party _channels_, not third-party windows. Therefore we've probably re-introduced bug 1108017. Sadly we never landed a test to go with Monica's patch so that's probably why we didn't notice.

(In reply to François Marier [:francois] from comment #5) > Comment on attachment 8999245 [details] [diff] [review] > Part 1: Remove the centralized third-party checks from > nsChannelClassifier::ShouldEnableTrackingProtectionInternal > > Review of attachment 8999245 [details] [diff] [review]: > ----------------------------------------------------------------- > > Bug 1476715 only added third-party checks to the consumers of tracking > annotations. So if we take the check out entirely, we break tracking > _protection_. I see. > Something else I just realized about > https://hg.mozilla.org/mozilla-central/rev/cb8a7a9f4a73: we're only checking > for third-party _channels_, not third-party windows. Therefore we've > probably re-introduced bug 1108017. Sadly we never landed a test to go with > Monica's patch so that's probably why we didn't notice. Can you please file a separate regression bug for that? That should probably be tracked for the 63 release as a regression...

(In reply to :Ehsan Akhgari from comment #6) > Can you please file a separate regression bug for that? That should > probably be tracked for the 63 release as a regression... I just filed bug 1482950 for it.

Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ecdc01374747 Part 1: Remove the centralized third-party checks from nsChannelClassifier::ShouldEnableTrackingProtectionInternal in annotation-only mode; r=francois https://hg.mozilla.org/integration/mozilla-inbound/rev/174bb3e3138c Part 2: Remove the checks for the restrict3rdpartystorage pref from the channel classifier and only rely on the annotate_channels pref; r=francois

https://hg.mozilla.org/mozilla-central/rev/ecdc01374747 https://hg.mozilla.org/mozilla-central/rev/174bb3e3138c