In bug 1476715, we added third-party checks to all of the users of tracking annotations, but we're now only checking for third-party _channels_, not third-party windows. Therefore we've probably re-introduced bug 1108017. This really needs a test otherwise we're likely to reintroduce this bug again in the future.

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb393ff4d294d8e171c29103d1b7b86a9702baa0

This refactoring is now unnecessary since the third-party check is getting removed.

This expands the test to cover the case where annotations are enabled by priority lowering isn't. A new parameter is introduced to makeConnection in order to specify the topWindowURI. This will be useful in future tests, but it also highlights the fact that we do set this in all of the existing tests. Finally, I also added a number of comments and explicit parameter setting in order to make the test more readily understandable. Depends on D3720

Depends on D3721

The mIsTrackingResource flag on nsIHttpChannel was split into two separate flags depending on whether or not the resource is third-party. The correct flag will be set by the channel classifier. Similarly, a new function was introduced, GetIsThirdPartyTrackingResource(), for those consumers (like TP) who only care about third-party trackers. The existing function, GetIsTracking(), will continue to look at both first-party and third-party trackers (the behavior since first party tracking was added to annotations in bug 1476324). The OverrideTrackingResource() function now allows nsHTMLDocument to override both mIsFirstPartyTrackingResource and mIsThirdPartyTrackingResource, but since this function is a little dangerous and only has a single user, I added an assert to make future callers think twice about using it to opt out of tracking annotations. Currently, only the default storage restrictions need to look at first-party trackers so every other consumer has been moved to mIsThirdPartyTrackingResource or GetIsThirdPartyTrackingResource(). This effectively reverts the third-party checks added in bug 1476715 and replaces them with the more complicated check that was added in bug 1108017. It follows the approach that Ehsan initially suggested in bug 1476715. It also reverts the changes in the expected values of the tracking annotation test since these were, in hindsight, a warning about this regression. Depends on D3722

Comment on attachment 9002258 [details] Bug 1482950 - Backed out changeset 32d94a3cc7af. r=xeonchen! r? Gary for the FastBlock parts (straight revert of his previous patch that's no longer needed)

Comment on attachment 9002259 [details] Bug 1482950 - Improve tracking annotation test. r=ehsan! r? Ehsan for improvements to the tracking annotation test you created last year

Comment on attachment 9002260 [details] Bug 1482950 - Fix eslint errors in the tracking annotations test. r=dimi! r? dimi for pretty straighforward fixes to make the JS linter happy (no actual changes)

Comment on attachment 9002261 [details] Bug 1482950 - Use the correct 3rdparty check in tracking annotations. r=dimi!,ehsan! r? Ehsan for the changes relevant to the default storage restrictions r? Dimi for the URL classifier logic r? Honza for the necko changes

Better tests are planned in bug 1484493, but I have manually verified this using this new test page I created: https://itisatracker.org/bug1108017.html

Comment on attachment 9002258 [details] Bug 1482950 - Backed out changeset 32d94a3cc7af. r=xeonchen! Gary Chen [:xeonchen] has approved the revision.

Comment on attachment 9002260 [details] Bug 1482950 - Fix eslint errors in the tracking annotations test. r=dimi! Dimi Lee[:dimi][:dlee] has approved the revision.

Comment on attachment 9002261 [details] Bug 1482950 - Use the correct 3rdparty check in tracking annotations. r=dimi!,ehsan! Dimi Lee[:dimi][:dlee] has approved the revision.

Comment on attachment 9002261 [details] Bug 1482950 - Use the correct 3rdparty check in tracking annotations. r=dimi!,ehsan! Honza Bambas (:mayhemer) has approved the revision.

Comment on attachment 9002259 [details] Bug 1482950 - Improve tracking annotation test. r=ehsan! :Ehsan Akhgari has approved the revision.

Comment on attachment 9002261 [details] Bug 1482950 - Use the correct 3rdparty check in tracking annotations. r=dimi!,ehsan! :Ehsan Akhgari has approved the revision.

Comment on attachment 9002259 [details] Bug 1482950 - Improve tracking annotation test. r=ehsan! Looks like Phabricator flags aren't synched properly... :-(

Pushed by fmarier@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fe91a0b8bb4c Backed out changeset 32d94a3cc7af. r=xeonchen!

Pushed by fmarier@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0e95533b40e1 Improve tracking annotation test. r=Ehsan!

Pushed by fmarier@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bd35c530c6e7 Fix eslint errors in the tracking annotations test. r=dimi!

Pushed by fmarier@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/251360ecbedf Use the correct 3rdparty check in tracking annotations. r=dimi,Ehsan,mayhemer!,ehsan!

https://hg.mozilla.org/mozilla-central/rev/fe91a0b8bb4c https://hg.mozilla.org/mozilla-central/rev/0e95533b40e1 https://hg.mozilla.org/mozilla-central/rev/bd35c530c6e7 https://hg.mozilla.org/mozilla-central/rev/251360ecbedf