In bug 1321868, I'm adding a URL classifier lookup for each script we load to determine whether it's tracking or not. bkelly suggested that perhaps we can save this lookup and do one lookup as part of the existing safe-browsing/Necko integration and mark the channel somehow if that lookup determines that the load is from a tracking source. I'm not too familiar with the url-classifier internals, so I'm not sure if doing that would actually save a lookup. Francois, do you have any suggestions here? Thanks!

We run through the URL classifier in these three places: 1. BeginConnect() [1] 2. Open() [2] 3. AsyncOpen() [3] #2 and #3 are for all Safe Browsing lists (including malware and phishing), but the first one is only for tracking protection. If we tagged the channel with TP info earlier, then we could skip potentially skip #1. [1] https://dxr.mozilla.org/mozilla-central/rev/f8086bd3c84fc1a42c3625cf3cc2253f0a5e8cfd/netwerk/protocol/http/nsHttpChannel.cpp#5185 [2] https://dxr.mozilla.org/mozilla-central/rev/f8086bd3c84fc1a42c3625cf3cc2253f0a5e8cfd/netwerk/base/nsBaseChannel.cpp#611 [3] https://dxr.mozilla.org/mozilla-central/rev/f8086bd3c84fc1a42c3625cf3cc2253f0a5e8cfd/netwerk/base/nsBaseChannel.cpp#672

These links are from https://wiki.mozilla.org/Phishing_Protection#Code_walkthrough

Thanks Francois! I think we may be able to achieve the goal here by also doing the classification when we're loading the script and tag the channel but not block the load if TP isn't enabled, sort of along the lines of the approach in bug 1141814. I'll give this a shot next week.

OK, I think I have a plan for how to do this. Bug 1141814 effectively added most of the support for annotating tracking channels, but only used it for lowering the channel priority. So I'm planning to do the following: 1. Fix bug 1324053 because I would like to turn the privacy.trackingprotection.annotate_channels pref on by default. 2. Fix bug 1170190 to provide an API for querying the tracking status of a channel. 3. Redo the way that we mark scripts as tracking to consume that API.

Comment on attachment 8819558 [details] [diff] [review] Part 2: Use the passive tracking protection API to determine whether a script is tracking Review of attachment 8819558 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsScriptLoader.h @@ +138,5 @@ > + return mIsTracking; > + } > + void SetIsTracking() > + { > + mIsTracking = true; Can we MOZ_ASSERT(!mIsTracking)? Or can this legitimately be called twice?

(In reply to Ben Kelly [:bkelly] from comment #7) > ::: dom/base/nsScriptLoader.h > @@ +138,5 @@ > > + return mIsTracking; > > + } > > + void SetIsTracking() > > + { > > + mIsTracking = true; > > Can we MOZ_ASSERT(!mIsTracking)? Or can this legitimately be called twice? I _think_ that should be fine...

Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb169203786 Part 1: Revert the script loader changes from bug 1321868; r=bkelly https://hg.mozilla.org/integration/mozilla-inbound/rev/ea336ca346cd Part 2: Use the passive tracking protection API to determine whether a script is tracking; r=bkelly

This patch showed tp5o perf improvements on perfherder, thanks. For up to date refer: https://treeherder.mozilla.org/perf.html#/alerts?id=4626