Store and return urls instead of hosts for the malware IP matching.

Store and return urls instead of hosts for the malware IP matching.

BUG=176647
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215379

[kewang, 2013-07-30 18:31:35]

One question: should we canonicalize the urls matching malicious IP before
sending them back in the pings?

[noelutz, 2013-07-31 00:38:08]

lgtm

[mattm, 2013-07-31 01:21:41]

GURL.spec() is canonicalized already

Looks like we don't have any histograms for the CSD ping results or ping size,
should probably add some (like https://codereview.chromium.org/19599012 and
https://codereview.chromium.org/21168003 )
But that probably should be a separate CL.

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
File chrome/browser/safe_browsing/browser_feature_extractor.cc (right):

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
chrome/browser/safe_browsing/browser_feature_extractor.cc:241: int matched_badip
= 0;
matched_bad_ips

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
File chrome/browser/safe_browsing/client_side_detection_host.cc (right):

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
chrome/browser/safe_browsing/client_side_detection_host.cc:522:
!MalwareKillSwitchIsOn()) {
I'm not sure if the url is guaranteed to be valid (It would seem weird if it's
not, though), may need to check url.is_valid() here.

[kewang, 2013-07-31 20:40:32]

I will do the histogram in next CL. Also added some unittest case.

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
File chrome/browser/safe_browsing/browser_feature_extractor.cc (right):

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
chrome/browser/safe_browsing/browser_feature_extractor.cc:241: int matched_badip
= 0;
On 2013/07/31 01:21:41, mattm wrote:
> matched_bad_ips

Done.

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
File chrome/browser/safe_browsing/client_side_detection_host.cc (right):

https://chromiumcodereview.appspot.com/21170004/diff/1/chrome/browser/safe_br...
chrome/browser/safe_browsing/client_side_detection_host.cc:522:
!MalwareKillSwitchIsOn()) {
On 2013/07/31 01:21:41, mattm wrote:
> I'm not sure if the url is guaranteed to be valid (It would seem weird if it's
> not, though), may need to check url.is_valid() here.

Done.

[mattm, 2013-07-31 23:56:33]

lgtm with nits

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
File chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc (left):

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc:484:
hosts.insert("test.com");
should be urls now?

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
File chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc (right):

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc:553:
good_urls.insert("ok.com");
http://

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc:587: for (int
i = 0; i < 7; ++i){  // Add 7 ips
space before {

[kewang, 2013-08-01 17:55:15]

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
File chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc (left):

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc:484:
hosts.insert("test.com");
On 2013/07/31 23:56:33, mattm wrote:
> should be urls now?

Done.

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
File chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc (right):

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc:553:
good_urls.insert("ok.com");
On 2013/07/31 23:56:33, mattm wrote:
> http://

Done.

https://codereview.chromium.org/21170004/diff/7001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc:587: for (int
i = 0; i < 7; ++i){  // Add 7 ips
On 2013/07/31 23:56:33, mattm wrote:
> space before {

Done.

[mattm, 2013-08-02 01:11:20]

lgtm

[commit-bot: I haz the power, 2013-08-02 16:18:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kewang@google.com/21170004/27001

[commit-bot: I haz the power, 2013-08-02 16:51:44]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-02 20:38:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kewang@google.com/21170004/27001

[commit-bot: I haz the power, 2013-08-02 22:01:08]

Change committed as 215379