1577040 - Eliminate the usage of nsIHttpChannel::IsTrackingResource()

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, task)

Description

[(no longer active)]

Right now we have a bunch of usages of nsIHttpChannel::IsTrackingResource() combined with separate third-party checks. They're at best misleading and at worst introducing subtle bugs by doing the third-party checks slightly differently. Let's port all of them to IsThirdPartyTrackingResource().

Comment 1

[(no longer active)]

Attachment: Bug 1577040 - Part 1: Remove the usages of nsIHttpChannel::IsTrackingResource() in the cookie service;

Comment 2

[(no longer active)]

Attachment: Bug 1577040 - Part 2: Remove nsContentUtils::IsTrackingResourceWindow() and replace its calls with IsThirdPartyTrackingResourceWindow();

Comment 3

[(no longer active)]

Attachment: Bug 1577040 - Part 3: Replace nsIHttpChannel.isTrackingResource() with isThirdPartyTrackingResource() in the url-classifier tests;

Comment 4

[(no longer active)]

There is a test failure on caused by part 2 of these patch series, see this try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e54b5f0163fc2b8ceefe3ad1007b27bddd606d5b (the bc3/bc6 failures)

The failure is caused by this change. It is coming from this part of the anti-tracking tests which were added for bug 1504194.

The scenario is that our top-level page is example.net and the third-party page is another-tracking.example.net (both from the same eTLD+1). The third-party page is on the TP list so we classify it as a tracker but because the third-party checks determine that it is first-party wrt to the top-level page, we classify it as a first-party tracker. Hence the above change modifies the meaning of the code.

It seems to me that the best way to deal with this for now is to convert the nsContentUtils::IsTrackingResourceWindow() call to a function which checks the classification flags directly ignoring the first/third-party-ness.

Does that sound good?

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:ehsan, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[(no longer active)]

baku: ping???

Comment 7

[Andrea Marchesini [:baku]]

It seems to me that the best way to deal with this for now is to convert the nsContentUtils::IsTrackingResourceWindow() call to a function which checks the classification flags directly ignoring the first/third-party-ness.

It took me a while to answer. Sorry. Yes, what you propose should work correctly. Do you want me to continue these patches?

Comment 8

[(no longer active)]

(In reply to Andrea Marchesini [:baku] from comment #7)

It seems to me that the best way to deal with this for now is to convert the nsContentUtils::IsTrackingResourceWindow() call to a function which checks the classification flags directly ignoring the first/third-party-ness.

It took me a while to answer. Sorry. Yes, what you propose should work correctly. Do you want me to continue these patches?

That would be great, if you don't mind. I kept rebasing these patches for a few months but then I gave up and deleted them from my queue... :-(

Comment 9

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 0: nsIClassifiedChannel.isThirdPartySocialTrackingResource(), r?ehsan

Comment 10

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 1: Remove the usages of nsIHttpChannel::IsTrackingResource() in the cookie service, r?ehsan

Depends on D63298

Comment 11

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 2: Remove nsContentUtils::IsTrackingResourceWindow() and replace its calls with IsThirdPartyTrackingResourceWindow(), r?ehsan

Depends on D63299

Comment 12

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 3: Replace nsIHttpChannel.isTrackingResource() with isThirdPartyTrackingResource() in the url-classifier tests, r?ehsan

Depends on D63300

Comment 13

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 4: Get rid of nsIClassifiedChannel::IsSocialTrackingResource(), r?ehsan

Depends on D63301

Comment 14

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 5: Remove nsContentUtils::IsTrackingResource() from anti-tracking, r?ehsan

Depends on D63302

Comment 15

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 6: Fix a couple of tests, r?ehsan

Depends on D63303

Comment 16

[Andrea Marchesini [:baku]]

Attachment: Bug 1577040 - Part 7: Get rid of nsIClassifiedChanel::IsTrackingResource(), r?ehsan

Depends on D63304

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c19bec23d39d
Part 0: nsIClassifiedChannel.isThirdPartySocialTrackingResource(), r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/e9bf54da2df4
Part 1: Remove the usages of nsIHttpChannel::IsTrackingResource() in the cookie service, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/273ce1bb372a
Part 2: Remove nsContentUtils::IsTrackingResourceWindow() and replace its calls with IsThirdPartyTrackingResourceWindow(), r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/cb55df34c744
Part 3: Replace nsIHttpChannel.isTrackingResource() with isThirdPartyTrackingResource() in the url-classifier tests, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/a4970ef2e4e2
Part 4: Get rid of nsIClassifiedChannel::IsSocialTrackingResource(), r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/d4997121ca63
Part 5: Remove nsContentUtils::IsTrackingResource() from anti-tracking, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/669f02d2e979
Part 6: Fix a couple of tests, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/31c9eed2f159
Part 7: Get rid of nsIClassifiedChanel::IsTrackingResource(), r=Ehsan

Comment 18

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/c19bec23d39d
https://hg.mozilla.org/mozilla-central/rev/e9bf54da2df4
https://hg.mozilla.org/mozilla-central/rev/273ce1bb372a
https://hg.mozilla.org/mozilla-central/rev/cb55df34c744
https://hg.mozilla.org/mozilla-central/rev/a4970ef2e4e2
https://hg.mozilla.org/mozilla-central/rev/d4997121ca63
https://hg.mozilla.org/mozilla-central/rev/669f02d2e979
https://hg.mozilla.org/mozilla-central/rev/31c9eed2f159