Adding histograms for better Omnibox prerender performance tracking.

Adding histograms for better Omnibox prerender performance tracking.


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

[Peter Kasting, 2011-11-21 22:31:02]

LGTM

[tburkard, 2011-11-21 23:45:39]

http://codereview.chromium.org/8602005/diff/3001/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/8602005/diff/3001/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_manager.cc:654:
UMA_HISTOGRAM_COUNTS("Prerender.OmniboxNavigationsUsedPrerenderCount_" +
Wouldn't this fit nicer into prerender_histograms which already has a way to
construct such histogram names for the different prerender types?

[tburkard, 2011-11-22 18:52:04]

http://codereview.chromium.org/8602005/diff/10001/chrome/browser/prerender/pr...
File chrome/browser/prerender/prerender_histograms.cc (right):

http://codereview.chromium.org/8602005/diff/10001/chrome/browser/prerender/pr...
chrome/browser/prerender/prerender_histograms.cc:156: void
PrerenderHistograms::RecordOmniboxUsedPrerender(Origin origin) const {
You should use the PREFIXED_HISTOGRAM family of macros to do this, or how is
this different from any of the other ones?

[dominich, 2011-11-22 19:00:41]

On 2011/11/22 18:52:04, tburkard wrote:
>
http://codereview.chromium.org/8602005/diff/10001/chrome/browser/prerender/pr...
> File chrome/browser/prerender/prerender_histograms.cc (right):
> 
>
http://codereview.chromium.org/8602005/diff/10001/chrome/browser/prerender/pr...
> chrome/browser/prerender/prerender_histograms.cc:156: void
> PrerenderHistograms::RecordOmniboxUsedPrerender(Origin origin) const {
> You should use the PREFIXED_HISTOGRAM family of macros to do this, or how is
> this different from any of the other ones?

These will never be triggered by any origin other than Omnibox and I don't care
about the experiment id. There's no need to use the extensive prefix support for
something this simple.

[tburkard, 2011-11-22 19:13:55]

LGTM.

There doesnt have to be an experiment ID in order to use it.

If this is temporary and gets removed again at some point, go ahead.

[commit-bot: I haz the power, 2011-11-22 19:19:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominich@chromium.org/8602005/10001

[commit-bot: I haz the power, 2011-11-22 20:48:02]

Change committed as 111207