It would be nice if we could have a mode for TP that would run the classifier as usual, but not block anything. It would just put the status on the channel (or somewhere), and we could use that information to inform the user that tracking is (potentially?) occurring.

Karen - We need some Product guidance for this

This requires a higher level discussion about what we want to do with tracking protection beyond Fx42.

Adding Barbara for tracking and figuring out our plan for beyond Fx42

snorp, what's the different between this and 1170194 (tracking detection)

Bug 1170194 would use this to implement the feature.

Francois, is this issue already on your team's radar somewhere else? Basically we want to be able to know if there are tracking elements loaded on a page, so we can show that data to the user as a reason to potentially want to enable tracking protection.

Steve told me that it did come up in Boston and that someone had found a way to do this today so it sounded like we wouldn't have to do anything on the platform side. I've forgotten the exact details though.

Right now if TP in PBM is disabled for a site, then Control Center shows two different messages if trackers are detected or not. The assumption was that if we can do that, then we can add new UI for Tracking Detection. I am not sure of the API they use here.

Bug 1141814 actually ended up doing most of the work here. I'm planning to make the privacy.trackingprotection.annotate_channels pref only control whether channels are annotated as tracking or non-tracking, and add an API to nsIChannel to query that information. I'm going to create another pref (privacy.trackingprotection.lower_network_priority) to control the behavior in bug 1141814.

Comment on attachment 8819433 [details] [diff] [review] Part 2: Add the nsIHttpChannel::IsTrackingResource() API to query the channel's tracking annotation Review of attachment 8819433 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/base/nsChannelClassifier.cpp @@ +714,5 @@ > + } > + nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(mChannel); > + if (httpChannel) { > + static_cast<HttpBaseChannel*>(httpChannel.get())-> > + SetIsTrackingResource(); can HttpBaseChannel have iid to use queryobject? I think it can. no static_cast on http channels please - a bad thing to do!

Comment on attachment 8819434 [details] [diff] [review] Part 3: Add tests for the e10s case too Review of attachment 8819434 [details] [diff] [review]: ----------------------------------------------------------------- hard to check the tests when there is no single word of explanation. can you provide some please? ::: netwerk/protocol/http/nsHttpChannel.cpp @@ +6147,5 @@ > + nsCOMPtr<nsIParentChannel> parentChannel; > + NS_QueryNotificationCallbacks(this, parentChannel); > + RefPtr<HttpChannelParent> httpParent = do_QueryObject(parentChannel); > + if (httpParent) { > + Unused << httpParent->SendSetPriority(newValue); check ipcclosed ::: netwerk/test/unit/test_trackingProtection_annotateChannels.js @@ +33,5 @@ > var httpServer; > var normalOrigin, trackingOrigin; > var testPriorityMap; > var currentTest; > +var skipNormalPriority, skipLowestPriority; add a comment what these are intended for

(In reply to Honza Bambas (:mayhemer) from comment #13) > Comment on attachment 8819433 [details] [diff] [review] > Part 2: Add the nsIHttpChannel::IsTrackingResource() API to query the > channel's tracking annotation > > Review of attachment 8819433 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: netwerk/base/nsChannelClassifier.cpp > @@ +714,5 @@ > > + } > > + nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(mChannel); > > + if (httpChannel) { > > + static_cast<HttpBaseChannel*>(httpChannel.get())-> > > + SetIsTrackingResource(); > > can HttpBaseChannel have iid to use queryobject? I think it can. Yeah, I don't see why not! New patches coming up.

(In reply to Honza Bambas (:mayhemer) from comment #14) > Comment on attachment 8819434 [details] [diff] [review] > Part 3: Add tests for the e10s case too > > Review of attachment 8819434 [details] [diff] [review]: > ----------------------------------------------------------------- > > hard to check the tests when there is no single word of explanation. can > you provide some please? Sure, I'll add some comments.

Comment on attachment 8819924 [details] [diff] [review] Part 3: Add tests for the e10s case too Honza is away until the next year. Patrick, do you mind taking a second look at these patches please?

Comment on attachment 8819923 [details] [diff] [review] Part 2: Add the nsIHttpChannel::IsTrackingResource() API to query the channel's tracking annotation Review of attachment 8819923 [details] [diff] [review]: ----------------------------------------------------------------- awesome. how does tp* feel about this? iirc there was some notable overhead to TP being skirted by being default off. but maybe that's OBE.

Comment on attachment 8819924 [details] [diff] [review] Part 3: Add tests for the e10s case too Review of attachment 8819924 [details] [diff] [review]: ----------------------------------------------------------------- I guess we can comment that this makes setpriority available in content as well.

(In reply to Patrick McManus [:mcmanus] from comment #20) > how does tp* feel about this? iirc there was some notable overhead to TP > being skirted by being default off. but maybe that's OBE. I'm doing this for bug 1322105 where I win ~3% back on Tpo. :-) (In reply to Patrick McManus [:mcmanus] from comment #21) > I guess we can comment that this makes setpriority available in content as > well. Will do. Thanks for the reviews!

BTW I think you may be thinking of bug 1122691. The thing that causes the regression is disabling speculative connect and DNS prefetching for channels because we don't want to do those things if after classification it becomes clear we want to block the connection. I made sure in bug 1324053 that we don't disable those things for passive TP, so that concern is no longer applicable.

Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d2c65588f41f Part 1: Split out a new pref from privacy.trackingprotection.annotate_channels to explicitly control whether the channel priority is adjusted; r=mayhemer https://hg.mozilla.org/integration/mozilla-inbound/rev/891791aa2301 Part 2: Add the nsIHttpChannel::IsTrackingResource() API to query the channel's tracking annotation; r=mayhemer https://hg.mozilla.org/integration/mozilla-inbound/rev/1ac1af4f133b Part 3: Add tests for the e10s case too; r=mayhemer

Shoot, I accidentally pushed these without amending the commit messages. Sorry about that. :/

https://hg.mozilla.org/mozilla-central/rev/d2c65588f41f https://hg.mozilla.org/mozilla-central/rev/891791aa2301 https://hg.mozilla.org/mozilla-central/rev/1ac1af4f133b