Monitor Add for negative results.

Monitor Add for negative results.

The incoming "count" should not be negative.  If so, record that.  Take
special note if it causes the result to become negative (and clamp it).

BUG=682680
Review-Url: https://codereview.chromium.org/2841143002
Cr-Commit-Position: refs/heads/master@{#467531}
Committed: https://chromium.googlesource.com/chromium/src/+/9c2f79bed5d1eb92360a6c2f88d4970e63438f8f

[bcwhite, 2017-04-26 19:09:15]

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

[commit-bot: I haz the power, 2017-04-26 19:10:12]

Dry run: CQ is trying da patch.

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

[bcwhite, 2017-04-26 19:27:15]

bcwhite@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2017-04-26 19:37:58]

https://codereview.chromium.org/2841143002/diff/1/base/metrics/persistent_sam...
File base/metrics/persistent_sample_map.cc (right):

https://codereview.chromium.org/2841143002/diff/1/base/metrics/persistent_sam...
base/metrics/persistent_sample_map.cc:205: Count* bucket =
GetOrCreateSampleCountStorage(min);
Nit: call this bucket_value as |bucket| is often used to refer to bucket index.

https://codereview.chromium.org/2841143002/diff/1/base/metrics/persistent_sam...
base/metrics/persistent_sample_map.cc:213: *bucket += count;
Couldn't this result in a negative too? i.e. due to overflow.

Let's check for that too.

[commit-bot: I haz the power, 2017-04-26 20:27:40]

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

[commit-bot: I haz the power, 2017-04-26 20:27:41]

Dry run: This issue passed the CQ dry run.

[bcwhite, 2017-04-26 20:46:13]

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

[commit-bot: I haz the power, 2017-04-26 20:46:53]

Dry run: CQ is trying da patch.

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

[bcwhite, 2017-04-26 20:47:14]

https://codereview.chromium.org/2841143002/diff/1/base/metrics/persistent_sam...
File base/metrics/persistent_sample_map.cc (right):

https://codereview.chromium.org/2841143002/diff/1/base/metrics/persistent_sam...
base/metrics/persistent_sample_map.cc:205: Count* bucket =
GetOrCreateSampleCountStorage(min);
On 2017/04/26 19:37:58, Alexei Svitkine (slow) wrote:
> Nit: call this bucket_value as |bucket| is often used to refer to bucket
index.

Went with |local_count_ptr| to match with |count| value retrieved from iterator.

https://codereview.chromium.org/2841143002/diff/1/base/metrics/persistent_sam...
base/metrics/persistent_sample_map.cc:213: *bucket += count;
On 2017/04/26 19:37:58, Alexei Svitkine (slow) wrote:
> Couldn't this result in a negative too? i.e. due to overflow.
> 
> Let's check for that too.

Done.

[Alexei Svitkine (slow), 2017-04-26 20:48:52]

lgtm

[bcwhite, 2017-04-26 21:03:33]

The CQ bit was unchecked by bcwhite@chromium.org

[bcwhite, 2017-04-26 21:03:39]

The CQ bit was checked by bcwhite@chromium.org

[commit-bot: I haz the power, 2017-04-26 21:03:55]

CQ is trying da patch.

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

[bcwhite, 2017-04-26 23:27:26]

The CQ bit was unchecked by bcwhite@chromium.org

[bcwhite, 2017-04-26 23:28:57]

The CQ bit was checked by bcwhite@chromium.org

[commit-bot: I haz the power, 2017-04-26 23:30:31]

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-04-27 00:40:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493249337421400,
"parent_rev": "81773235f9e789fd6e5aacefcad70f1401b8c36e", "commit_rev":
"9c2f79bed5d1eb92360a6c2f88d4970e63438f8f"}

[commit-bot: I haz the power, 2017-04-27 00:41:06]

Description was changed from

==========
Monitor Add for negative results.

The incoming "count" should not be negative.  If so, record that.  Take
special note if it causes the result to become negative (and clamp it).

BUG=682680
==========

to

==========
Monitor Add for negative results.

The incoming "count" should not be negative.  If so, record that.  Take
special note if it causes the result to become negative (and clamp it).

BUG=682680

Review-Url: https://codereview.chromium.org/2841143002
Cr-Commit-Position: refs/heads/master@{#467531}
Committed:
https://chromium.googlesource.com/chromium/src/+/9c2f79bed5d1eb92360a6c2f88d4...
==========

[commit-bot: I haz the power, 2017-04-27 00:41:07]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/9c2f79bed5d1eb92360a6c2f88d4...