Collect stability data from global and activity scopes

Collect stability data from global and activity scopes

BUG=620813
Review-Url: https://codereview.chromium.org/2583223002
Cr-Commit-Position: refs/heads/master@{#442647}
Committed: https://chromium.googlesource.com/chromium/src/+/f380647990c63b5a258efba319f2aa1368b309f4

[manzagop (departed), 2016-12-19 16:41:07]

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

[manzagop (departed), 2016-12-19 16:41:07]

This adds collection for the key value stores!
Pierre

[manzagop (departed), 2016-12-19 16:41:20]

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

[commit-bot: I haz the power, 2016-12-19 16:41:29]

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, 2016-12-19 18:02:52]

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

[commit-bot: I haz the power, 2016-12-19 18:02:54]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[manzagop (departed), 2016-12-19 18:27:18]

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

[commit-bot: I haz the power, 2016-12-19 18:28:04]

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, 2016-12-19 20:13:01]

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

[commit-bot: I haz the power, 2016-12-19 20:13:02]

Dry run: This issue passed the CQ dry run.

[bcwhite, 2016-12-22 17:21:58]

https://codereview.chromium.org/2583223002/diff/20001/base/debug/activity_tra...
File base/debug/activity_tracker.h (right):

https://codereview.chromium.org/2583223002/diff/20001/base/debug/activity_tra...
base/debug/activity_tracker.h:310: ValueType get_type() const { return type; }
Just type().
Can't believe I forgot that one.  :-O

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
File components/browser_watcher/postmortem_report_collector.cc (right):

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/postmortem_report_collector.cc:58: break;
I believe style is

} break;

At least, "git cl format" has done that for me in the past.

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/postmortem_report_collector.cc:88:
(*collected_map)[name_and_value.first] = collected_value;
This will create an default value and then assign to it.  Using

->insert(make_pair(a, std::move(b)))

might be more efficient.  At the very least:

std::move(collected_value)

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
File components/browser_watcher/stability_report.proto (right):

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/stability_report.proto:71: int64 unsigned_value = 8;
Unsigned?

[manzagop (departed), 2017-01-10 14:11:02]

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

[commit-bot: I haz the power, 2017-01-10 14:11:18]

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

[manzagop (departed), 2017-01-10 14:11:22]

Thanks! Another look?

https://codereview.chromium.org/2583223002/diff/20001/base/debug/activity_tra...
File base/debug/activity_tracker.h (right):

https://codereview.chromium.org/2583223002/diff/20001/base/debug/activity_tra...
base/debug/activity_tracker.h:310: ValueType get_type() const { return type; }
On 2016/12/22 17:21:58, bcwhite wrote:
> Just type().
> Can't believe I forgot that one.  :-O

Done. I switched to type() and renamed the variable: type -> type_. Also
suffixed the other class variables with an underscore.

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
File components/browser_watcher/postmortem_report_collector.cc (right):

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/postmortem_report_collector.cc:58: break;
On 2016/12/22 17:21:58, bcwhite wrote:
> I believe style is
> 
> } break;
> 
> At least, "git cl format" has done that for me in the past.

The example from the style guide has it inside. Maybe both are ok?
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/postmortem_report_collector.cc:88:
(*collected_map)[name_and_value.first] = collected_value;
On 2016/12/22 17:21:58, bcwhite wrote:
> This will create an default value and then assign to it.  Using
> 
> ->insert(make_pair(a, std::move(b)))
> 
> might be more efficient.  At the very least:
> 
> std::move(collected_value)

I don't think protos can be moved. I've switched to a swap to avoid the copy
though.

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
File components/browser_watcher/stability_report.proto (right):

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/stability_report.proto:71: int64 unsigned_value = 8;
On 2016/12/22 17:21:58, bcwhite wrote:
> Unsigned?

Done.

[commit-bot: I haz the power, 2017-01-10 15:04:41]

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

[commit-bot: I haz the power, 2017-01-10 15:04:42]

Dry run: This issue passed the CQ dry run.

[manzagop (departed), 2017-01-10 15:11:46]

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

[commit-bot: I haz the power, 2017-01-10 15:12:04]

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-10 16:15:29]

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

[commit-bot: I haz the power, 2017-01-10 16:15:30]

Dry run: This issue passed the CQ dry run.

[bcwhite, 2017-01-10 18:00:38]

lgtm

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
File components/browser_watcher/postmortem_report_collector.cc (right):

https://codereview.chromium.org/2583223002/diff/20001/components/browser_watc...
components/browser_watcher/postmortem_report_collector.cc:58: break;
On 2017/01/10 14:11:22, manzagop wrote:
> On 2016/12/22 17:21:58, bcwhite wrote:
> > I believe style is
> > 
> > } break;
> > 
> > At least, "git cl format" has done that for me in the past.
> 
> The example from the style guide has it inside. Maybe both are ok?
> https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

Acknowledged.

https://codereview.chromium.org/2583223002/diff/60001/base/debug/activity_tra...
File base/debug/activity_tracker.h (right):

https://codereview.chromium.org/2583223002/diff/60001/base/debug/activity_tra...
base/debug/activity_tracker.h:334: ValueType type_;
Thanks.  I think I started this as a struct is why the underscores were missing.

[manzagop (departed), 2017-01-10 18:55:48]

Thanks for the review!

https://codereview.chromium.org/2583223002/diff/60001/base/debug/activity_tra...
File base/debug/activity_tracker.h (right):

https://codereview.chromium.org/2583223002/diff/60001/base/debug/activity_tra...
base/debug/activity_tracker.h:334: ValueType type_;
On 2017/01/10 18:00:38, bcwhite wrote:
> Thanks.  I think I started this as a struct is why the underscores were
missing.

Acknowledged.

[manzagop (departed), 2017-01-10 18:58:50]

The CQ bit was checked by manzagop@chromium.org

[commit-bot: I haz the power, 2017-01-10 18:59:24]

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-10 19:06:59]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484074730061370,
"parent_rev": "aa8e4c9b15d480285490e8343a419379a4b57889", "commit_rev":
"f380647990c63b5a258efba319f2aa1368b309f4"}

[commit-bot: I haz the power, 2017-01-10 19:07:30]

Description was changed from

==========
Collect stability data from global and activity scopes

BUG=620813
==========

to

==========
Collect stability data from global and activity scopes

BUG=620813

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

[commit-bot: I haz the power, 2017-01-10 19:07:31]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/f380647990c63b5a258efba319f2...