Switch to the new CustomHistogram::ArrayToCustomRanges() utility

Switch to the new CustomHistogram::ArrayToCustomRanges() utility
function where appropriate. Add a DCHECK which may be useful to
detect incorrect usage.

BUG=none
TEST=net_unittests, unit_tests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86822

[Jói, 2011-05-24 16:11:58]

Jim: Here's the switch to using the new utility function, as promised. Please
review the whole change.

John: Please review for content/OWNERS. Note that Jim had suggested renaming the
histogram when switching to this new utility function in a separate review
thread, as the bucket definitions will change (the function adds guard buckets
which the previous version did not have).

Cheers,
Jói

[jar (doing other things), 2011-05-24 18:10:07]

LGTM

Possible change below.

http://codereview.chromium.org/6990058/diff/1/base/metrics/histogram.cc
File base/metrics/histogram.cc (right):

http://codereview.chromium.org/6990058/diff/1/base/metrics/histogram.cc#newco...
base/metrics/histogram.cc:949: DCHECK_GE(value, 0);  // You must use positive
sample values.
This is a good idea (I love DCHECKS).  It would be better to push this down into
the constructor, and assert that we never get fed a negative number.  That would
catch more problems (since it is at a lower level).  I'd even be ok with making
this a CHECK() (release time assertion), as I'm not sure how well the code would
handle negative values during searches. I'm pretty sure the first (least) value
in the range MUST be 0, which would break if some negative values were sorted
into the mix

http://codereview.chromium.org/6990058/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/6990058/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/resource_dispatcher_host.cc:1611:
"Net.ErrorCodesForMainFrame2",
Be sure to submit a related addition to histograms.xml

[Jói, 2011-05-24 18:27:31]

Thanks Jim.

http://codereview.chromium.org/6990058/diff/1/base/metrics/histogram.cc
File base/metrics/histogram.cc (right):

http://codereview.chromium.org/6990058/diff/1/base/metrics/histogram.cc#newco...
base/metrics/histogram.cc:949: DCHECK_GE(value, 0);  // You must use positive
sample values.
On 2011/05/24 18:10:07, jar wrote:
> This is a good idea (I love DCHECKS).  It would be better to push this down
into
> the constructor, and assert that we never get fed a negative number.  That
would
> catch more problems (since it is at a lower level).  I'd even be ok with
making
> this a CHECK() (release time assertion), as I'm not sure how well the code
would
> handle negative values during searches. I'm pretty sure the first (least)
value
> in the range MUST be 0, which would break if some negative values were sorted
> into the mix

I moved it to Histogram::SetBucketRange. Let me know if that is wrong.

I left it as a DCHECK to match the other checks that are in place in the class.

http://codereview.chromium.org/6990058/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/6990058/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/resource_dispatcher_host.cc:1611:
"Net.ErrorCodesForMainFrame2",
On 2011/05/24 18:10:07, jar wrote:
> Be sure to submit a related addition to histograms.xml

Thanks for the reminder; preparing that now.

[jar (doing other things), 2011-05-24 21:56:06]

LGTM

[jam, 2011-05-25 00:45:48]

content lgtm

[Jói, 2011-05-26 14:20:58]

Adding cbentzel@ for net/OWNERS approval.

[cbentzel, 2011-05-26 14:24:52]

LGTM