Closed Bug 1029781 Opened 7 years ago Closed 7 years ago
image loaders should set load
Flags | NS _LOAD _CLASSIFY _URI
Since images can hit the malware/phishing/tracking lists, they should go through the channel classifier.
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Comment on attachment 8445535 [details] [diff] [review] Set NS_LOAD_CLASSIFY_URI in image loader Review of attachment 8445535 [details] [diff] [review]: ----------------------------------------------------------------- Hey Seth, This change forces images to go through nsChannelClassifier. This is the correct thing to do for phishing and malware checks, but also we especially need it for tracking protection (which will be prefed off by default), since webbugs are often used for tracking. Generally safebrowsing lookups are very fast (<< .1ms) but it would be great to cross check for performance regressions if such metrics exist. http://telemetry.mozilla.org/#filter=nightly%2F33%2FURLCLASSIFIER_LOOKUP_TIME&aggregates=multiselect-all!Submissions!Mean!5th%20percentile!25th%20percentile!median!75th%20percentile!95th%20percentile&evoOver=Builds&locked=true&sanitize=true&renderhistogram=Graph Monica
Attachment #8445535 - Flags: review?(seth)
Attachment #8445535 - Flags: review?(seth) → review+
I had to back this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5d4dcd761df9 for being a likely cause of Android xpcshell test failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=42467232&tree=Mozilla-Inbound
Thanks Wes. Working on it.
Hey gcp, this seems to be breaking on Classifier::Open: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#745 which is using the profile directory. I'm a little confused, because this breakage is on Android only and I thought that safebrowsing worked on Android. Do I need to add do_get_profile() to these tests, or fix #ifdef MOZ_CLASSIFIER somewhere?
My best bet is that you're causing Classifier to be initialized earlier or be activated in some tests where it wasn't before. So yeah, problems with the profile seem likely. I see no reason why this would trigger on Android only.
Green try with do_get_profile(): https://tbpl.mozilla.org/?tree=Try&rev=439bfe4082df Relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/f3192b2f9195
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.