UMA: Add PageLoad user action

Adds a PageLoad user that gets recorded on every page load, exactly at the same time as the pageload count histogram is updated.

For background: this is what most people think the LoadURL user action does.  It does not.  LoadURL only gets recorded only on omnibox-mediated events.  LoadURL is in fact useless because there are already omnibox event logs which cover the  same space.

BUG=

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

[Mark P, 2012-08-24 00:38:10]

Assigning to Ilya for preliminary comments on the code while I'm still awaiting
feedback whether this is the right thing to do.

--mark

[Ilya Sherman, 2012-08-24 06:18:49]

LGTM, thanks :)

[Mark P, 2012-08-24 20:47:32]

Note the new description and effect.

I'm still holding off on checking this in until I get more comments.

--mark

[Mark P, 2012-08-27 18:25:19]

Requesting OWNERS review:

sky - chrome/browser/ui/ review
jar - chrome/browser/metrics/ review

thanks,
mark

[sky, 2012-08-27 20:46:57]

LGTM

[jar (doing other things), 2012-08-27 21:40:59]

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
File chrome/browser/ui/browser_commands.cc (left):

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
chrome/browser/ui/browser_commands.cc:344:
content::RecordAction(UserMetricsAction("LoadURL"));
Do you have any interest in seeing how these numbers compare to the new metric? 
By leaving this deprecated (perhaps with a comment, and TODO() ...remove) you
get to see what the difference is "during real usage."

WDYT?

Perf wise, this would not IMO have any impact (re: cost of the extra metric).

[Mark P, 2012-08-27 21:58:46]

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
File chrome/browser/ui/browser_commands.cc (left):

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
chrome/browser/ui/browser_commands.cc:344:
content::RecordAction(UserMetricsAction("LoadURL"));
On 2012/08/27 21:40:59, jar wrote:
> Do you have any interest in seeing how these numbers compare to the new
metric? 

Good question.  I don't see a good reason to compare the next numbers with the
old.  I know the new will generally be bigger, but how much so will vary
substantially by country.  At the moment, I don't see anything useful that could
come of this.  Push back if you want.  (I don't feel that strongly about
removing LoadURL, so if you feel strongly that I should keep it (in
intended-to-be-ignored form) then tell me.)

> By leaving this deprecated (perhaps with a comment, and TODO() ...remove) you
> get to see what the difference is "during real usage."
> 
> WDYT?
>
> Perf wise, this would not IMO have any impact (re: cost of the extra metric).

[jar (doing other things), 2012-08-27 22:11:34]

+cc Ian F.

IMO, you should leave both intact for now.  Ian may comment... and if his
opinion differs, I'd go with his throughts.

...and then...

LGTM.

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
File chrome/browser/ui/browser_commands.cc (left):

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
chrome/browser/ui/browser_commands.cc:344:
content::RecordAction(UserMetricsAction("LoadURL"));
I'm wary that it would instantly "break" the current "counts" dashboard that was
historically resurrected (and maintained) by Ian F.  

IMO, unless you have a concurrent patch to fix those dashboards, you should
leave 'em both in... then migrate the dashboards (maybe use both, and have two
columns???), and then remove the old.

On 2012/08/27 21:58:46, Mark P wrote:
> On 2012/08/27 21:40:59, jar wrote:
> > Do you have any interest in seeing how these numbers compare to the new
> metric? 
> 
> Good question.  I don't see a good reason to compare the next numbers with the
> old.  I know the new will generally be bigger, but how much so will vary
> substantially by country.  At the moment, I don't see anything useful that
could
> come of this.  Push back if you want.  (I don't feel that strongly about
> removing LoadURL, so if you feel strongly that I should keep it (in
> intended-to-be-ignored form) then tell me.)
> 
> > By leaving this deprecated (perhaps with a comment, and TODO() ...remove)
you
> > get to see what the difference is "during real usage."
> > 
> > WDYT?
> >
> > Perf wise, this would not IMO have any impact (re: cost of the extra
metric).

[Mark P, 2012-08-29 00:17:40]

ptal

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
File chrome/browser/ui/browser_commands.cc (left):

http://codereview.chromium.org/10882022/diff/6001/chrome/browser/ui/browser_c...
chrome/browser/ui/browser_commands.cc:344:
content::RecordAction(UserMetricsAction("LoadURL"));
On 2012/08/27 22:11:34, jar wrote:
> I'm wary that it would instantly "break" the current "counts" dashboard that
was
> historically resurrected (and maintained) by Ian F.  
> 
> IMO, unless you have a concurrent patch to fix those dashboards, you should
> leave 'em both in... then migrate the dashboards (maybe use both, and have two
> columns???), and then remove the old.
> 
> On 2012/08/27 21:58:46, Mark P wrote:
> > On 2012/08/27 21:40:59, jar wrote:
> > > Do you have any interest in seeing how these numbers compare to the new
> > metric? 
> > 
> > Good question.  I don't see a good reason to compare the next numbers with
the
> > old.  I know the new will generally be bigger, but how much so will vary
> > substantially by country.  At the moment, I don't see anything useful that
> could
> > come of this.  Push back if you want.  (I don't feel that strongly about
> > removing LoadURL, so if you feel strongly that I should keep it (in
> > intended-to-be-ignored form) then tell me.)
> > 
> > > By leaving this deprecated (perhaps with a comment, and TODO() ...remove)
> you
> > > get to see what the difference is "during real usage."
> > > 
> > > WDYT?
> > >
> > > Perf wise, this would not IMO have any impact (re: cost of the extra
> metric).
> 

Okay.  Restored LoadURL recording.  I'll remove in way down the line after the
dashboard migrates.

[jar (doing other things), 2012-08-29 01:09:15]

lgtm

[commit-bot: I haz the power, 2012-08-29 01:37:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/10882022/11001

[commit-bot: I haz the power, 2012-08-29 02:53:53]

Try job failure for 10882022-11001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-08-29 03:46:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/10882022/11001

[commit-bot: I haz the power, 2012-08-29 05:41:43]

Try job failure for 10882022-11001 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-29 05:42:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/10882022/11001

[commit-bot: I haz the power, 2012-08-29 06:50:25]

Try job failure for 10882022-11001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-08-29 06:51:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/10882022/11001

[commit-bot: I haz the power, 2012-08-29 09:00:54]

Try job failure for 10882022-11001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-08-29 15:23:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/10882022/15006

[commit-bot: I haz the power, 2012-08-29 17:23:36]

Try job failure for 10882022-15006 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-29 21:12:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/10882022/15006

[commit-bot: I haz the power, 2012-08-30 05:16:44]

Change committed as 154077