Fix overflow bug in histogram sample data structures

Fix overflow bug in histogram sample data structures

BUG=580696
BUG=internal b/26683692
TEST=Modified tests pass. End to end testing verified on problem scenario.

Committed: https://crrev.com/3334b50ea196494f220e432d417517c4e469fe81
Cr-Commit-Position: refs/heads/master@{#371118}

[bcf, 2016-01-22 22:26:52]

Description was changed from

==========
Fix overflow bug in histogram sample data structures

BUG= internal b/26683692
TEST= Modified tests pass. End to end testing verified on problem scenario.
==========

to

==========
Fix overflow bug in histogram sample data structures

BUG= internal b/26683692
TEST= Modified tests pass. End to end testing verified on problem scenario.
==========

[bcf, 2016-01-22 22:26:52]

bcf@chromium.org changed reviewers:
+ asvitkine@chromium.org, wzhong@chromium.org

[Alexei Svitkine (slow), 2016-01-22 22:30:11]

LGTM, thanks!

Please create a crbug and associate with that via BUG=, in case we find that we
need to merge this back or there's some problem.

Otherwise, thanks for the fix and all the tests - woot!

[bcf, 2016-01-22 22:30:17]

The added unit tests are just duplicates of existing ones, but with large
values. Would it be more preferable to just add the test cases to the existing
tests?

[Alexei Svitkine (slow), 2016-01-22 22:33:03]

I think separate tests are fine given they're testing this specific case. Have a
suggestion about naming below.

Still LGTM.

https://codereview.chromium.org/1618423004/diff/1/base/metrics/histogram_unit...
File base/metrics/histogram_unittest.cc (right):

https://codereview.chromium.org/1618423004/diff/1/base/metrics/histogram_unit...
base/metrics/histogram_unittest.cc:263: TEST_F(HistogramTest, AddCountLargeTest)
{
Nit: Maybe AddCount_LargeValuesDontOverflow

And same for the others. (No need to have the word "Test" in the name of the
unit test - even though I know the previous tests do this. Also, no need for the
comment above it.

[bcf, 2016-01-22 22:46:01]

Description was changed from

==========
Fix overflow bug in histogram sample data structures

BUG= internal b/26683692
TEST= Modified tests pass. End to end testing verified on problem scenario.
==========

to

==========
Fix overflow bug in histogram sample data structures

BUG=580696
BUG=internal b/26683692
TEST=Modified tests pass. End to end testing verified on problem scenario.
==========

[bcf, 2016-01-22 22:46:14]

https://codereview.chromium.org/1618423004/diff/1/base/metrics/histogram_unit...
File base/metrics/histogram_unittest.cc (right):

https://codereview.chromium.org/1618423004/diff/1/base/metrics/histogram_unit...
base/metrics/histogram_unittest.cc:263: TEST_F(HistogramTest, AddCountLargeTest)
{
On 2016/01/22 22:33:03, Alexei Svitkine (very slow) wrote:
> Nit: Maybe AddCount_LargeValuesDontOverflow
> 
> And same for the others. (No need to have the word "Test" in the name of the
> unit test - even though I know the previous tests do this. Also, no need for
the
> comment above it.

Done.

[bcf, 2016-01-22 22:46:36]

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

[commit-bot: I haz the power, 2016-01-22 22:48:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1618423004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1618423004/20001

[commit-bot: I haz the power, 2016-01-23 00:22:37]

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

[commit-bot: I haz the power, 2016-01-23 00:22:38]

Dry run: This issue passed the CQ dry run.

[bcf, 2016-01-23 00:35:31]

The CQ bit was checked by bcf@chromium.org

[bcf, 2016-01-23 00:35:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1618423004/#ps20001
(title: "Rename tests, remove redundant comments.")

[commit-bot: I haz the power, 2016-01-23 00:36:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1618423004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1618423004/20001

[commit-bot: I haz the power, 2016-01-23 00:44:15]

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

[commit-bot: I haz the power, 2016-01-23 00:44:16]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, no build URL)

[tandrii(chromium), 2016-01-23 01:28:53]

The CQ bit was checked by tandrii@chromium.org

[commit-bot: I haz the power, 2016-01-23 01:30:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1618423004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1618423004/20001

[commit-bot: I haz the power, 2016-01-23 01:40:06]

Description was changed from

==========
Fix overflow bug in histogram sample data structures

BUG=580696
BUG=internal b/26683692
TEST=Modified tests pass. End to end testing verified on problem scenario.
==========

to

==========
Fix overflow bug in histogram sample data structures

BUG=580696
BUG=internal b/26683692
TEST=Modified tests pass. End to end testing verified on problem scenario.
==========

[commit-bot: I haz the power, 2016-01-23 01:40:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-23 01:40:53]

Description was changed from

==========
Fix overflow bug in histogram sample data structures

BUG=580696
BUG=internal b/26683692
TEST=Modified tests pass. End to end testing verified on problem scenario.
==========

to

==========
Fix overflow bug in histogram sample data structures

BUG=580696
BUG=internal b/26683692
TEST=Modified tests pass. End to end testing verified on problem scenario.

Committed: https://crrev.com/3334b50ea196494f220e432d417517c4e469fe81
Cr-Commit-Position: refs/heads/master@{#371118}
==========

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

Patchset 2 (id:??) landed as
https://crrev.com/3334b50ea196494f220e432d417517c4e469fe81
Cr-Commit-Position: refs/heads/master@{#371118}