Add UMA stats for the Website Setting UI

Add UMA stats for the Website Setting UI

TBR=rsesek@chromium.org
BUG=113688
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=155688

[markusheintz_, 2012-09-05 15:15:25]

Ben, Finnur could you please review this CL? Thanks a lot

[Finnur, 2012-09-06 10:10:03]

LGTM, with nits (and questions)

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
File chrome/browser/ui/website_settings/website_settings.cc (right):

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:104: // created. So this
counter counts how ofter the Website Settings UI is
s/counter counts/counts/

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:105: // opend.
You can probably move this up a line if you follow my comment two lines up (oh,
and fix the typo).

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions.txt
File chrome/tools/chromeactions.txt (right):

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:13:
0x088990bbea59a185	AboutFlags_disable-website-settings
I saw this added in a changelist 6 days ago...
https://chromiumcodereview.appspot.com/10918021/diff/2003/chrome/tools/chrome...
... but I don't see this in the latest version??
Why does this get added and removed all the time?

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1175: 0xac722301695b9315	PageLoad
and this?

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1365: 0xd3879291d67e1389	ViewAboutNaCl
and this?

[Ben Goodger (Google), 2012-09-06 21:22:35]

lgtm

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
File chrome/browser/ui/website_settings/website_settings.cc (right):

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:103: // Everytime the
Website Settings UI is opend a |WebsiteSettings| object is
Every time

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:103: // Everytime the
Website Settings UI is opend a |WebsiteSettings| object is
opened

[markusheintz_, 2012-09-07 13:31:35]

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
File chrome/browser/ui/website_settings/website_settings.cc (right):

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:103: // Everytime the
Website Settings UI is opend a |WebsiteSettings| object is
On 2012/09/06 21:22:35, Ben Goodger (Google) wrote:
> opened

Done.

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:103: // Everytime the
Website Settings UI is opend a |WebsiteSettings| object is
On 2012/09/06 21:22:35, Ben Goodger (Google) wrote:
> Every time

Done.

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:104: // created. So this
counter counts how ofter the Website Settings UI is
On 2012/09/06 10:10:04, Finnur wrote:
> s/counter counts/counts/

Done.

http://codereview.chromium.org/10908092/diff/1006/chrome/browser/ui/website_s...
chrome/browser/ui/website_settings/website_settings.cc:105: // opend.
On 2012/09/06 10:10:04, Finnur wrote:
> You can probably move this up a line if you follow my comment two lines up
(oh,
> and fix the typo).

Done.

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions.txt
File chrome/tools/chromeactions.txt (right):

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:13:
0x088990bbea59a185	AboutFlags_disable-website-settings
On 2012/09/06 10:10:04, Finnur wrote:
> I saw this added in a changelist 6 days ago...
> diff/2003/chrome/tools/chromeactions.txt
> ... but I don't see this in the latest version??
> Why does this get added and removed all the time?

Looks like in CL https://chromiumcodereview.appspot.com/10873020 (that added
this counter) I forgot to run
chrome/tools/extract_actions.py with the --hash flag :(

But since then this counter was not removed or re-added anymore. At least not by
me.

I assume this showed up in the CL you mentioned because it was picked up by the
chrome/tools/extract_actions.py script that generates this file.

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1175: 0xac722301695b9315	PageLoad
On 2012/09/06 10:10:04, Finnur wrote:
> and this?

I don't know what's up with PageLoad and ViewAboutNaCL below.

"PageLoad" is collected in chrome/browser/metrics/metrics_service.cc:1597

and "ViewAboutNaCl" is collected in 
chrome/browser/ui/webui/nacl_ui.cc:273

I think I should not remove these lines since this file is auto generated.

But I make sure to reach out to the folks that added these counters.

http://codereview.chromium.org/10908092/diff/1006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1365: 0xd3879291d67e1389	ViewAboutNaCl
On 2012/09/06 10:10:04, Finnur wrote:
> and this?
same as above.

http://codereview.chromium.org/10908092/diff/7006/chrome/tools/chromeactions.txt
File chrome/tools/chromeactions.txt (right):

http://codereview.chromium.org/10908092/diff/7006/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1175: 0xac722301695b9315	PageLoad
I don't know what's up with PageLoad and ViewAboutNaCL below.

Page load is used in chrome/browser/metrics/metrics_service.cc:1597

and
chrome/browser/ui/webui/nacl_ui.cc:273

I think I should not remove these lines since this file is auto generated.

But I make sure to reach out to the folks that added these counters.

[Finnur, 2012-09-07 14:05:43]

OK, still LGTM.

[markusheintz_, 2012-09-07 14:34:25]

Adding Mark and Jan because of the additional UMA hashes

@mpearson, jvoung: Please take a look at my comments in
chrome/tools/chromeactions.txt.

Maybe you forgot to run chrome/tools/extract_actions.py --hash when you added
UMA counter.

(see content/public/browser/user_metrics.h for more infos)

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions.txt
File chrome/tools/chromeactions.txt (right):

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:13:
0x088990bbea59a185	AboutFlags_disable-website-settings
@finnur: FYI
Sorry Finnur I didn't notice this earlier, but it looks like the CL you were
referring earlier was not committed yet.

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1175: 0xac722301695b9315	PageLoad
@mpearson: Hey Mark in CL 10882022
(https://chromiumcodereview.appspot.com/10882022) you added the "PageLoad" UMA
counter. 

Apparently there was no hash added for this counter.
Maybe you forgot to run chrome/tools/extract_actions.py --hash

Are you ok with me checking in this line for you?

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1365: 0xd3879291d67e1389	ViewAboutNaCl
@jvoung: Hey Jan in CL
10823214(https://chromiumcodereview.appspot.com/10823214/) you added the UMA
"ViewAboutNaCl" counter.

Apparently there was no hash added for this counter.
Maybe you forgot to run chrome/tools/extract_actions.py --hash

Are you ok with me checking in this line for you?

[jvoung (off chromium), 2012-09-07 15:22:54]

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions.txt
File chrome/tools/chromeactions.txt (right):

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1365: 0xd3879291d67e1389	ViewAboutNaCl
On 2012/09/07 14:34:25, markusheintz_ wrote:
> @jvoung: Hey Jan in CL
> 10823214(https://chromiumcodereview.appspot.com/10823214/) you added the UMA
> "ViewAboutNaCl" counter.
> 
> Apparently there was no hash added for this counter.
> Maybe you forgot to run chrome/tools/extract_actions.py --hash
> 
> Are you ok with me checking in this line for you?

Oops, yes I didn't know about that.  I'm okay with you checking it in.  Thanks!

[Mark P, 2012-09-07 15:49:44]

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions.txt
File chrome/tools/chromeactions.txt (right):

http://codereview.chromium.org/10908092/diff/9002/chrome/tools/chromeactions....
chrome/tools/chromeactions.txt:1175: 0xac722301695b9315	PageLoad
On 2012/09/07 14:34:25, markusheintz_ wrote:
> @mpearson: Hey Mark in CL 10882022
> (https://chromiumcodereview.appspot.com/10882022) you added the "PageLoad" UMA
> counter. 
> 
> Apparently there was no hash added for this counter.
> Maybe you forgot to run chrome/tools/extract_actions.py --hash
> 
> Are you ok with me checking in this line for you?

Yes, I forgot.  Please check in this line!

[commit-bot: I haz the power, 2012-09-07 15:56:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/markusheintz@chromium.org/10908092/9002

[commit-bot: I haz the power, 2012-09-07 17:31:53]

Try job failure for 10908092-9002 (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-09-08 21:09:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/markusheintz@chromium.org/10908092/9002

[commit-bot: I haz the power, 2012-09-08 23:03:31]

Try job failure for 10908092-9002 (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&...