Update UMA histograms as discussed with jar.

Update UMA histograms as discussed with jar.

BUG=128307
TEST=Getting histograms that are useful for us.


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

[pastarmovj, 2012-05-16 11:17:34]

Hi Scott,
can you please review this small CL. It only fixes up the UMA stats setup to be
more usable as a result of the discussion I had with jar about that.
Thanks!

[sky, 2012-05-16 15:47:42]

http://codereview.chromium.org/10406005/diff/5001/chrome/browser/ui/views/net...
File chrome/browser/ui/views/network_profile_bubble.cc (right):

http://codereview.chromium.org/10406005/diff/5001/chrome/browser/ui/views/net...
chrome/browser/ui/views/network_profile_bubble.cc:50: kMetricCheckSuppressed,
Chrome uses FOO_BAR style for enums.

[pastarmovj, 2012-05-16 15:55:56]

Thanks for the quick review. PTAL.

http://codereview.chromium.org/10406005/diff/5001/chrome/browser/ui/views/net...
File chrome/browser/ui/views/network_profile_bubble.cc (right):

http://codereview.chromium.org/10406005/diff/5001/chrome/browser/ui/views/net...
chrome/browser/ui/views/network_profile_bubble.cc:50: kMetricCheckSuppressed,
On 2012/05/16 15:47:42, sky wrote:
> Chrome uses FOO_BAR style for enums.

Sorry about that. I was looking at a bad example in enterprise_metrics.h.
Changed all as requested.

[sky, 2012-05-16 16:12:06]

LGTM. I also added Jim to the reivew.

[jar (doing other things), 2012-05-16 16:57:42]

I'd suggest the tiny refactor indicated below, although it is not critical.  It
would make it more compact, both in terms of source (for readability) and object
code (the macro is non-trivial).  

The code is otherwise LGTM.

http://codereview.chromium.org/10406005/diff/3/chrome/browser/ui/views/networ...
File chrome/browser/ui/views/network_profile_bubble.cc (right):

http://codereview.chromium.org/10406005/diff/3/chrome/browser/ui/views/networ...
chrome/browser/ui/views/network_profile_bubble.cc:121:
UMA_HISTOGRAM_ENUMERATION(kMetricNetworkedProfileCheck,
nit: Not a big deal.... but when you have to repeate a histogram in a bunch of
places, it is probably nicer to factor out a static method that just takes the
enum, and call it. This way you can just put the name (sen on line 46) in the
actual histogram call.... and also, the histogram macro is a tab big, and might
as well be done once-and-for-all in one place.

[pastarmovj, 2012-05-18 09:48:52]

Thanks for the comments. I will commit once the try jobs go green.

http://codereview.chromium.org/10406005/diff/3/chrome/browser/ui/views/networ...
File chrome/browser/ui/views/network_profile_bubble.cc (right):

http://codereview.chromium.org/10406005/diff/3/chrome/browser/ui/views/networ...
chrome/browser/ui/views/network_profile_bubble.cc:121:
UMA_HISTOGRAM_ENUMERATION(kMetricNetworkedProfileCheck,
On 2012/05/16 16:57:43, jar wrote:
> nit: Not a big deal.... but when you have to repeate a histogram in a bunch of
> places, it is probably nicer to factor out a static method that just takes the
> enum, and call it. This way you can just put the name (sen on line 46) in the
> actual histogram call.... and also, the histogram macro is a tab big, and
might
> as well be done once-and-for-all in one place.

Done.

[commit-bot: I haz the power, 2012-05-18 10:04:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/10406005/12002

[commit-bot: I haz the power, 2012-05-18 11:29:05]

Change committed as 137843