So we can use it to block tracking domains.

https://tbpl.mozilla.org/?tree=Try&rev=0b72534bfceb

Hey Ryan, Can you specify which paths you want for the gethash and update URLs? https://tracking.services.mozilla.com/{gethash,update,whatever you want} The existing gethash and update urls are here: http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#927 The SAFEBROWSING_ID stuff is rewritten by the url formatter, which we can just skip if you don't think we'll need it: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/urlformatter/nsURLFormatter.js#126 Thanks, Monica

Comment on attachment 8448235 [details] [diff] [review] Register tracking protection list and hook it up in nsChannelClassifier ( Review of attachment 8448235 [details] [diff] [review]: ----------------------------------------------------------------- Ryan, please review URLs in firefox.js. gcp, to dogfood please change browser.tracking_protection.enabled = true and point updateUrl to "http://54.184.154.124/downloads?updateURL&client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2&key=%GOOGLE_API_KEY%". I filed bug 1032414 to fix the stream updater from calling downloadsuccess for NXDOMAINs, and bug 1032393 to enable gethash requests for this list when there's a server for it.

Comment on attachment 8448235 [details] [diff] [review] Register tracking protection list and hook it up in nsChannelClassifier ( Review of attachment 8448235 [details] [diff] [review]: ----------------------------------------------------------------- "There are only two hard things in Computer Science: cache invalidation and naming things." Don't you need changes to the error pages to deal with the third category? ::: browser/app/profile/firefox.js @@ +935,5 @@ > > pref("browser.safebrowsing.malware.reportURL", "https://safebrowsing.google.com/safebrowsing/diagnostic?client=%NAME%&hl=%LOCALE%&site="); > pref("browser.safebrowsing.appRepURL", "https://sb-ssl.google.com/safebrowsing/clientreport/download?key=%GOOGLE_API_KEY%"); > > +pref("browser.tracking_protection.updateURL", "https://tracking.services.mozilla.com/update?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2&key=%GOOGLE_API_KEY%"); GOOGLE_API_KEY for our own service? Boo! ::: modules/libpref/src/init/all.js @@ +819,5 @@ > // 0 = tracking is acceptable > // 1 = tracking is unacceptable > pref("privacy.donottrackheader.value", 1); > +// Enforce tracking protection > +pref("privacy.trackingprotection.enabled", false); tracking_protection earlier vs trackingprotection here @@ +4186,5 @@ > pref("urlclassifier.malware_table", "goog-malware-shavar,test-malware-simple"); > pref("urlclassifier.phish_table", "goog-phish-shavar,test-phish-simple"); > pref("urlclassifier.downloadBlockTable", ""); > pref("urlclassifier.downloadAllowTable", ""); > +pref("urlclassifier.tracking_list", "mozpub-track-digest256"); malware_table, BlockTable and now tracking_list. Consistent naming please. ::: toolkit/components/url-classifier/content/listmanager.js @@ +379,5 @@ > // As long as the delay is something sane (5 minutes or more), update > // our delay time for requesting updates. Setting the delay requires a > // repeating timer, so always use one. > if (delay >= (5 * 60) && this.updateCheckers_[updateUrl]) { > + log("Waiting for " + delay + " seconds"); nit: unrelated change

(In reply to Gian-Carlo Pascutto [:gcp] from comment #10) > Comment on attachment 8448235 [details] [diff] [review] > Register tracking protection list and hook it up in nsChannelClassifier ( > > Review of attachment 8448235 [details] [diff] [review]: > ----------------------------------------------------------------- > > "There are only two hard things in Computer Science: cache invalidation and > naming things." > > Don't you need changes to the error pages to deal with the third category? This list is not intended to include domains that people actually want to visit as a top-level URL. I filed bug 1032480 in case someone actually does want to visit a site on the list, but I don't think it should block this one, especially since it is prefed off and undiscoverable right now. > GOOGLE_API_KEY for our own service? Boo! OK :) > @@ +4186,5 @@ > > pref("urlclassifier.malware_table", "goog-malware-shavar,test-malware-simple"); > > pref("urlclassifier.phish_table", "goog-phish-shavar,test-phish-simple"); > > pref("urlclassifier.downloadBlockTable", ""); > > pref("urlclassifier.downloadAllowTable", ""); > > +pref("urlclassifier.tracking_list", "mozpub-track-digest256"); > > malware_table, BlockTable and now tracking_list. Consistent naming please. Yeah, this is due to bug 985720 :( I will rename everything to camel case. > ::: toolkit/components/url-classifier/content/listmanager.js > @@ +379,5 @@ > > // As long as the delay is something sane (5 minutes or more), update > > // our delay time for requesting updates. Setting the delay requires a > > // repeating timer, so always use one. > > if (delay >= (5 * 60) && this.updateCheckers_[updateUrl]) { > > + log("Waiting for " + delay + " seconds"); > > nit: unrelated change

Comment on attachment 8448950 [details] [diff] [review] Register tracking protection list and hook it up in nsChannelClassifier ( Review of attachment 8448950 [details] [diff] [review]: ----------------------------------------------------------------- Green try (doesn't include observer changes): https://tbpl.mozilla.org/?tree=Try&rev=ad4d23be4bed

Comment on attachment 8448950 [details] [diff] [review] Register tracking protection list and hook it up in nsChannelClassifier ( Review of attachment 8448950 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but no reason not to add the URLs for all platforms. ::: browser/app/profile/firefox.js @@ +936,5 @@ > pref("browser.safebrowsing.malware.reportURL", "https://safebrowsing.google.com/safebrowsing/diagnostic?client=%NAME%&hl=%LOCALE%&site="); > pref("browser.safebrowsing.appRepURL", "https://sb-ssl.google.com/safebrowsing/clientreport/download?key=%GOOGLE_API_KEY%"); > > +pref("browser.trackingprotection.updateURL", "https://tracking.services.mozilla.com/update?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2"); > +pref("browser.trackingprotection.gethashURL", "https://tracking.services.mozilla.com/gethash?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2"); Don't you need to update Android with the same links? Alternatively, why aren't these in the generic all.js?

Hmm, interesting -- the regular safebrowsing update prefs are in firefox.js and mobile.js only, I missed the mobile.js one. I'll try moving to all.js: https://tbpl.mozilla.org/?tree=Try&rev=270d8479f2f5

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e35172b6476