Performance monitor stats gathering.

Performance monitor stats gathering.
Re-landing https://chromiumcodereview.appspot.com/10656052 after it was reverted.
See https://chromiumcodereview.appspot.com/10827151/

TBR=brettw@chromium.org

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

[Yoyo Zhou, 2012-08-07 19:35:56]

LGTM, assuming the unit_tests are the only change since the original CL.

You might want to put in the CL description that this is relanding [original
codereview].

[commit-bot: I haz the power, 2012-08-07 19:38:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mitchellwrosen@chromium.org/10831196/1

[Devlin, 2012-08-07 19:39:39]

https://chromiumcodereview.appspot.com/10831196/diff/1/chrome/browser/perform...
File chrome/browser/performance_monitor/performance_monitor_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/10831196/diff/1/chrome/browser/perform...
chrome/browser/performance_monitor/performance_monitor_browsertest.cc:525:
ASSERT_EQ(1u, stats.size());
I think a check that it actually recorded something other than zero is important
here (since we've had problems in the past with it not gathering correctly). If
we are consistently recording 0 in the release, then it becomes worse than a
useless stat, since it's incredibly misleading. The for-loop tactic didn't work,
but have you tried just having chrome do some "typical" behavior? For instance,
visit a url (or 10), refresh a page, open a new tab, download something, etc.
This would be a more realistic test, anyway.

[commit-bot: I haz the power, 2012-08-07 21:21:51]

List of reviewers changed. rdevlin.cronin@chromium.org did a drive-by without
LGTM'ing!

[mitchellwrosen, 2012-08-07 21:30:09]

On 2012/08/07 19:39:39, D Cronin wrote:
>
https://chromiumcodereview.appspot.com/10831196/diff/1/chrome/browser/perform...
> File chrome/browser/performance_monitor/performance_monitor_browsertest.cc
> (right):
> 
>
https://chromiumcodereview.appspot.com/10831196/diff/1/chrome/browser/perform...
> chrome/browser/performance_monitor/performance_monitor_browsertest.cc:525:
> ASSERT_EQ(1u, stats.size());
> I think a check that it actually recorded something other than zero is
important
> here (since we've had problems in the past with it not gathering correctly).
If
> we are consistently recording 0 in the release, then it becomes worse than a
> useless stat, since it's incredibly misleading. The for-loop tactic didn't
work,
> but have you tried just having chrome do some "typical" behavior? For
instance,
> visit a url (or 10), refresh a page, open a new tab, download something, etc.
> This would be a more realistic test, anyway.

Talked about in IRC, but I'll summarize -- was going to investigate ChromeOS
failure after this landed, per Devlin's concern, but since the commit-bot
hiccuped anyways, I'll push another patch and wait for an LGTM from Devlin.

[Devlin, 2012-08-07 23:07:56]

LGTM for the time being (since more testing are being developed).

[commit-bot: I haz the power, 2012-08-07 23:31:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mitchellwrosen@chromium.org/10831196/9001

[commit-bot: I haz the power, 2012-08-08 00:56:20]

Change committed as 150475