Add histogram for safebrowsing download ping payload size.

Add histogram for safebrowsing download ping payload size.

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

[noelutz, 2013-07-30 00:19:09]

lgtm

[mattm, 2013-07-30 00:52:26]

+isherman for histograms

[Ilya Sherman, 2013-07-30 01:27:38]

LGTM with a suggestion:

https://codereview.chromium.org/21168003/diff/3001/chrome/browser/safe_browsi...
File chrome/browser/safe_browsing/download_protection_service.cc (right):

https://codereview.chromium.org/21168003/diff/3001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/download_protection_service.cc:672:
client_download_request_data_.size());
Are you sure that a histogram mapping [1, 1,000,000) bytes gives you the dynamic
range that you want?  It seems likely to me that you might not care so much
about distinguishing payload sizes on the order of 1-10 bytes, and possibly care
about distinguishing payload sizes on the order of 1-10 megabytes.  If so, you
might want to switch this histogram to measure KB instead of just bytes.

[mattm, 2013-07-30 01:53:22]

https://codereview.chromium.org/21168003/diff/3001/chrome/browser/safe_browsi...
File chrome/browser/safe_browsing/download_protection_service.cc (right):

https://codereview.chromium.org/21168003/diff/3001/chrome/browser/safe_browsi...
chrome/browser/safe_browsing/download_protection_service.cc:672:
client_download_request_data_.size());
On 2013/07/30 01:27:39, Ilya Sherman wrote:
> Are you sure that a histogram mapping [1, 1,000,000) bytes gives you the
dynamic
> range that you want?  It seems likely to me that you might not care so much
> about distinguishing payload sizes on the order of 1-10 bytes, and possibly
care
> about distinguishing payload sizes on the order of 1-10 megabytes.  If so, you
> might want to switch this histogram to measure KB instead of just bytes.

This is actually the size of the ping we send to the safebrowsing server, not
the size of the file we are checking, so it shouldn't be that big.

[commit-bot: I haz the power, 2013-07-30 08:57:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/21168003/3001

[commit-bot: I haz the power, 2013-07-30 19:46:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/21168003/3001

[commit-bot: I haz the power, 2013-07-31 14:03:13]

Change committed as 214721