Include all server redirects in referrer chain

In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Add two histograms to count # of NavigationEvents and
# of ResolvedIPAddress every 2 minutes. These help us estimate
the memory usage.

Also, fix a bug that causes duplicating IP addresses.

BUG=680238
Review-Url: https://codereview.chromium.org/2644133005
Cr-Commit-Position: refs/heads/master@{#447382}
Committed: https://chromium.googlesource.com/chromium/src/+/de82ac84d9c24e3910cfc4c20d81d94bfca48943

[Jialiu Lin, 2017-01-20 00:01:14]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 00:02:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2017-01-20 01:07:17]

Description was changed from

==========
add the compelte server redirects
BUG=680238
==========

to

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url and
the final url (after all server-redirects). Per Bineval team's
request, this CL adds all the server redirect urls (including
intermediate ones) to download pings. This is because these
intermediate server redirects might be used as signal to
identify certain malware/UwS campaigns.

Also, fix a bug that causes recording duplicated IP addresses.
BUG=680238
==========

[Jialiu Lin, 2017-01-20 01:08:12]

Description was changed from

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url and
the final url (after all server-redirects). Per Bineval team's
request, this CL adds all the server redirect urls (including
intermediate ones) to download pings. This is because these
intermediate server redirects might be used as signal to
identify certain malware/UwS campaigns.

Also, fix a bug that causes recording duplicated IP addresses.
BUG=680238
==========

to

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Also, fix a bug that causes recording duplicated IP
addresses.

BUG=680238
==========

[commit-bot: I haz the power, 2017-01-20 01:08:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-20 01:08:30]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-01-20 01:33:49]

Description was changed from

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Also, fix a bug that causes recording duplicated IP
addresses.

BUG=680238
==========

to

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Also, fix a bug that causes duplicating IP addresses.

BUG=680238
==========

[Jialiu Lin, 2017-01-20 01:36:27]

Patchset #2 (id:20001) has been deleted

[Jialiu Lin, 2017-01-20 01:39:03]

jialiul@chromium.org changed reviewers:
+ nparker@chromium.org

[Jialiu Lin, 2017-01-20 01:39:04]

nparker@, could you take a look?

[Nathan Parker, 2017-01-20 23:36:20]

lgtm

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer.h (right):

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer.h:48: //
including the final destination
Maybe say in the comments where in the list the final URL is (first? last?)

Also, a question: Should these two equiv always? If so, is the bool redundant?
  has_server_redirect == !server_redirect_urls.empty()

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer.h:50: GURL
GetDestinationUrl() const {
This could return a const GURL& so it doesn't get copied.  The compiler might do
that anyway, but I'm not sure.

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer.h:53: return
original_request_url;
nit: add an else to make it a bit more readable

https://codereview.chromium.org/2644133005/diff/40001/chrome/common/safe_brow...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2644133005/diff/40001/chrome/common/safe_brow...
chrome/common/safe_browsing/csd.proto:421: // The first entry in
|server_redirect_chain| should be the original request
Looks like the last entry would be redundant, but if auk et.al would prefer it
that way, fine with me.

[Jialiu Lin, 2017-01-21 00:48:27]

Thanks nparker@!

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer.h (right):

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer.h:48: //
including the final destination
On 2017/01/20 23:36:20, Nathan Parker wrote:
> Maybe say in the comments where in the list the final URL is (first? last?)
> 
> Also, a question: Should these two equiv always? If so, is the bool redundant?
>   has_server_redirect == !server_redirect_urls.empty()

You're right, the has_server_redirect boolean is redundant.
Refined comment for server_redirect_urls.

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer.h:50: GURL
GetDestinationUrl() const {
On 2017/01/20 23:36:20, Nathan Parker wrote:
> This could return a const GURL& so it doesn't get copied.  The compiler might
do
> that anyway, but I'm not sure.

Done.

https://codereview.chromium.org/2644133005/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer.h:53: return
original_request_url;
On 2017/01/20 23:36:20, Nathan Parker wrote:
> nit: add an else to make it a bit more readable

Done.

https://codereview.chromium.org/2644133005/diff/40001/chrome/common/safe_brow...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2644133005/diff/40001/chrome/common/safe_brow...
chrome/common/safe_browsing/csd.proto:421: // The first entry in
|server_redirect_chain| should be the original request
On 2017/01/20 23:36:20, Nathan Parker wrote:
> Looks like the last entry would be redundant, but if auk et.al would prefer it
> that way, fine with me.

I think it is less confusing in this way. auk and others didn't object when I
made change in the csd_client.proto.

[Jialiu Lin, 2017-01-26 23:53:09]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 23:54:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2017-01-27 00:27:15]

Patchset #4 (id:80001) has been deleted

[Jialiu Lin, 2017-01-27 00:29:21]

Description was changed from

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Also, fix a bug that causes duplicating IP addresses.

BUG=680238
==========

to

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Add two histograms to count # of NavigationEvents and
# of ResolvedIPAddress every 2 minutes. These help us estimate
the memory usage.

Also, fix a bug that causes duplicating IP addresses.

BUG=680238
==========

[Jialiu Lin, 2017-01-27 00:31:02]

Hi nparker@,
I made some changes to this CL due to my recent discussion with Kathy and Eric
w.r.t ReferrerChainEntry proto. Let me know if this still looks good.

[Jialiu Lin, 2017-01-27 20:07:17]

jialiul@chromium.org changed reviewers:
+ holte@chromium.org

[Jialiu Lin, 2017-01-27 20:07:17]

+holte@ for histograms.xml

Thanks!

[Steven Holte, 2017-01-27 21:01:11]

lgtm

[Nathan Parker, 2017-01-27 22:53:38]

lgtm

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/download_protection_service.cc (right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/download_protection_service.cc:1244: web_contents ?
web_contents->GetLastCommittedURL():GURL()),
nit: add spaces around :

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:244:
if (expected_main_frame_url.is_valid()) {
nit: some of these if()'s start with the populated case, and some start with the
unpopulated case.  It might be easier to read if the first branch was always the
empty/invalid case.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:274:
for (int i = 0; i<actual_entry->server_redirect_chain_size(); i++) {
nit: spaces around <

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:425:
GURL(),                            // main_frame_url
(I don't know how to validate if these are correct)

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:338:
remove_count += (size_before_removal-size_after_removal);
nit: spaces around -
here and below

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:500:
last_nav_event_traced->source_main_frame_url);
What's the different between source_main_frame_url and source_url?  Is it that
the latter might be... an iframe?

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:511:
last_main_frame_url_traced,
Do we no longer need SERVER_REDIRECT?

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc:203:
testing::ElementsAre(base::Bucket(4, 1)));
Fancy!  And nicely done, testing histograms.

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
chrome/common/safe_browsing/csd.proto:400: SERVER_REDIRECT = 5;  // Deprecated
How about renaming to _DEPRECATED_SERVER_REDIRECT, to ensure it isn't used

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
chrome/common/safe_browsing/csd.proto:417: optional URLType type = 2 [default =
CLIENT_REDIRECT];
Why is this the default?

[Jialiu Lin, 2017-01-27 23:25:06]

Thanks!

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/download_protection_service.cc (right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/download_protection_service.cc:1244: web_contents ?
web_contents->GetLastCommittedURL():GURL()),
On 2017/01/27 22:53:37, Nathan Parker wrote:
> nit: add spaces around :

Done.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:244:
if (expected_main_frame_url.is_valid()) {
On 2017/01/27 22:53:37, Nathan Parker wrote:
> nit: some of these if()'s start with the populated case, and some start with
the
> unpopulated case.  It might be easier to read if the first branch was always
the
> empty/invalid case.
You're right. Let me rotate them.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:274:
for (int i = 0; i<actual_entry->server_redirect_chain_size(); i++) {
On 2017/01/27 22:53:38, Nathan Parker wrote:
> nit: spaces around <

Done.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:425:
GURL(),                            // main_frame_url
On 2017/01/27 22:53:37, Nathan Parker wrote:
> (I don't know how to validate if these are correct)
I don't think it is impossible to have a invalid main_frame_url as a parent of a
valid subframe url. If this happens in some weird corner cases, the
main_frame_url in the actual proto should not be set.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:338:
remove_count += (size_before_removal-size_after_removal);
On 2017/01/27 22:53:38, Nathan Parker wrote:
> nit: spaces around -
> here and below

Yes. Did git cl format.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:500:
last_nav_event_traced->source_main_frame_url);
On 2017/01/27 22:53:38, Nathan Parker wrote:
> What's the different between source_main_frame_url and source_url?  Is it that
> the latter might be... an iframe?
Yes, that's right. From the campaign example auk@ sent to me, actually a lot of
bad downloads are triggered by some navigation in subframes.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:511:
last_main_frame_url_traced,
On 2017/01/27 22:53:38, Nathan Parker wrote:
> Do we no longer need SERVER_REDIRECT?
We don't need SERVER_REDIRECT, since this can be deduced by the existence of
server_redirect_chain repeated field.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc:203:
testing::ElementsAre(base::Bucket(4, 1)));
On 2017/01/27 22:53:38, Nathan Parker wrote:
> Fancy!  And nicely done, testing histograms.

Lesson learnt from Iron Woman. :-)

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
chrome/common/safe_browsing/csd.proto:400: SERVER_REDIRECT = 5;  // Deprecated
On 2017/01/27 22:53:38, Nathan Parker wrote:
> How about renaming to _DEPRECATED_SERVER_REDIRECT, to ensure it isn't used

Done.

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
chrome/common/safe_browsing/csd.proto:417: optional URLType type = 2 [default =
CLIENT_REDIRECT];
On 2017/01/27 22:53:38, Nathan Parker wrote:
> Why is this the default?  
If a navigation is not pointing at these key urls (e.g. landing, landing
referrer, download), it is a regular client redirect. That's why we set the
default case to client redirect.

[Jialiu Lin, 2017-01-27 23:25:07]

Thanks!

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/download_protection_service.cc (right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/download_protection_service.cc:1244: web_contents ?
web_contents->GetLastCommittedURL():GURL()),
On 2017/01/27 22:53:37, Nathan Parker wrote:
> nit: add spaces around :

Done.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:244:
if (expected_main_frame_url.is_valid()) {
On 2017/01/27 22:53:37, Nathan Parker wrote:
> nit: some of these if()'s start with the populated case, and some start with
the
> unpopulated case.  It might be easier to read if the first branch was always
the
> empty/invalid case.
You're right. Let me rotate them.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:274:
for (int i = 0; i<actual_entry->server_redirect_chain_size(); i++) {
On 2017/01/27 22:53:38, Nathan Parker wrote:
> nit: spaces around <

Done.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc:425:
GURL(),                            // main_frame_url
On 2017/01/27 22:53:37, Nathan Parker wrote:
> (I don't know how to validate if these are correct)
I don't think it is impossible to have a invalid main_frame_url as a parent of a
valid subframe url. If this happens in some weird corner cases, the
main_frame_url in the actual proto should not be set.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:338:
remove_count += (size_before_removal-size_after_removal);
On 2017/01/27 22:53:38, Nathan Parker wrote:
> nit: spaces around -
> here and below

Yes. Did git cl format.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:500:
last_nav_event_traced->source_main_frame_url);
On 2017/01/27 22:53:38, Nathan Parker wrote:
> What's the different between source_main_frame_url and source_url?  Is it that
> the latter might be... an iframe?
Yes, that's right. From the campaign example auk@ sent to me, actually a lot of
bad downloads are triggered by some navigation in subframes.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:511:
last_main_frame_url_traced,
On 2017/01/27 22:53:38, Nathan Parker wrote:
> Do we no longer need SERVER_REDIRECT?
We don't need SERVER_REDIRECT, since this can be deduced by the existence of
server_redirect_chain repeated field.

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc
(right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc:203:
testing::ElementsAre(base::Bucket(4, 1)));
On 2017/01/27 22:53:38, Nathan Parker wrote:
> Fancy!  And nicely done, testing histograms.

Lesson learnt from Iron Woman. :-)

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
File chrome/common/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
chrome/common/safe_browsing/csd.proto:400: SERVER_REDIRECT = 5;  // Deprecated
On 2017/01/27 22:53:38, Nathan Parker wrote:
> How about renaming to _DEPRECATED_SERVER_REDIRECT, to ensure it isn't used

Done.

https://codereview.chromium.org/2644133005/diff/100001/chrome/common/safe_bro...
chrome/common/safe_browsing/csd.proto:417: optional URLType type = 2 [default =
CLIENT_REDIRECT];
On 2017/01/27 22:53:38, Nathan Parker wrote:
> Why is this the default?  
If a navigation is not pointing at these key urls (e.g. landing, landing
referrer, download), it is a regular client redirect. That's why we set the
default case to client redirect.

[Jialiu Lin, 2017-01-30 17:22:10]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 17:22:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 17:24:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 17:24:29]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Jialiu Lin, 2017-01-31 22:47:42]

The CQ bit was checked by jialiul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 22:48:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2017-01-31 23:33:00]

The CQ bit was unchecked by jialiul@chromium.org

[Jialiu Lin, 2017-01-31 23:33:08]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-01-31 23:33:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2644133005/#ps140001
(title: "rebase update")

[commit-bot: I haz the power, 2017-01-31 23:33:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 00:39:34]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1485905588250560,
"parent_rev": "7993a5787e8b01ff8dd6014380e0d813f73b2a63", "commit_rev":
"de82ac84d9c24e3910cfc4c20d81d94bfca48943"}

[commit-bot: I haz the power, 2017-02-01 00:40:19]

Description was changed from

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Add two histograms to count # of NavigationEvents and
# of ResolvedIPAddress every 2 minutes. These help us estimate
the memory usage.

Also, fix a bug that causes duplicating IP addresses.

BUG=680238
==========

to

==========
In previous implementation, if the server does the more than
one server redirects, we only record the original request url
and the final url (after all server-redirects). Per Bineval
team's request, this CL adds all the server redirect urls 
(including intermediate ones) to download pings. This is
because these intermediate server redirects might be used
as signal to identify certain malware/UwS campaigns.

Add two histograms to count # of NavigationEvents and
# of ResolvedIPAddress every 2 minutes. These help us estimate
the memory usage.

Also, fix a bug that causes duplicating IP addresses.

BUG=680238

Review-Url: https://codereview.chromium.org/2644133005
Cr-Commit-Position: refs/heads/master@{#447382}
Committed:
https://chromium.googlesource.com/chromium/src/+/de82ac84d9c24e3910cfc4c20d81...
==========

[commit-bot: I haz the power, 2017-02-01 00:40:20]

Committed patchset #6 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/de82ac84d9c24e3910cfc4c20d81...