Add metrics to the new ProfileChooser Tutorials

Add metrics to the new ProfileChooser Tutorials

Noms as TBR, the profile_metrics have been well reviewed by others.

BUG=401147
TBD=noms@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289935

[Mike Lerman, 2014-08-13 16:40:25]

Hi reviewers,

PTAL.

noms - profile_metrics
msw - c/b/ui/views (and cocoa)
asvitkine - histograms (and cocoa if you like)
guohui - ProfileChooser View/Controller

Thanks!

[Alexei Svitkine (slow), 2014-08-14 15:53:39]

https://codereview.chromium.org/470643002/diff/60001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/470643002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:24727: +    Histogram for tracking user
interactions with the bubble that appears for
Nit: "Histogram for" is redundant. Just "Tracks user interactions".. - same for
the other ones below.

https://codereview.chromium.org/470643002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:24732: +<histogram
name="Profile.NewAvatarMenuNotYou" enum="ProfileNewAvatarMenuNotYou">
I suggest using two levels of dotted prefixes, so these histograms are grouped
better on the dashboard.

e.g. Profile.NewAvatarMenu.NotYou and same for others. Also FRE is a bit
cryptic, maybe instead FirstRun? (i.e. Profile.NewAvatarMenu.FirstRun)

[Mike Lerman, 2014-08-14 17:54:55]

Thanks Alexei.

https://codereview.chromium.org/470643002/diff/60001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/470643002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:24727: +    Histogram for tracking user
interactions with the bubble that appears for
On 2014/08/14 15:53:39, Alexei Svitkine wrote:
> Nit: "Histogram for" is redundant. Just "Tracks user interactions".. - same
for
> the other ones below.

Done.

https://codereview.chromium.org/470643002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:24732: +<histogram
name="Profile.NewAvatarMenuNotYou" enum="ProfileNewAvatarMenuNotYou">
On 2014/08/14 15:53:39, Alexei Svitkine wrote:
> I suggest using two levels of dotted prefixes, so these histograms are grouped
> better on the dashboard.
> 
> e.g. Profile.NewAvatarMenu.NotYou and same for others. Also FRE is a bit
> cryptic, maybe instead FirstRun? (i.e. Profile.NewAvatarMenu.FirstRun)

Done.

[Alexei Svitkine (slow), 2014-08-14 17:59:46]

lgtm

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_metrics.cc (right):

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
chrome/browser/profiles/profile_metrics.cc:369: DCHECK(metric <
NUM_PROFILE_DESKTOP_SIGNIN_METRICS);
Nit: Can this be DCHECK_LT? Same below.

[guohui, 2014-08-14 18:03:06]

lgtm with nits.

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_metrics.h (right):

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
chrome/browser/profiles/profile_metrics.h:160: // in users in the new avatar
menu on the first run.
The upgrade flow is not for users who signed in through first run. Instead it
appears for any profile that is created before new-avatar-menu is enabled. If
the profile is not signed in when the tutorial shown, then the 'not claire' link
will be hidden.

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
chrome/browser/profiles/profile_metrics.h:174: // can navigate to from the
Upgrade bubble during the first run.
same here, has nothing to do with first run.

[Mike Lerman, 2014-08-14 18:15:30]

Thanks Hui.

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_metrics.cc (right):

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
chrome/browser/profiles/profile_metrics.cc:369: DCHECK(metric <
NUM_PROFILE_DESKTOP_SIGNIN_METRICS);
On 2014/08/14 17:59:46, Alexei Svitkine wrote:
> Nit: Can this be DCHECK_LT? Same below.

Didn't know that was an option. Done, thanks!

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_metrics.h (right):

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
chrome/browser/profiles/profile_metrics.h:160: // in users in the new avatar
menu on the first run.
On 2014/08/14 18:03:06, guohui wrote:
> The upgrade flow is not for users who signed in through first run. Instead it
> appears for any profile that is created before new-avatar-menu is enabled. If
> the profile is not signed in when the tutorial shown, then the 'not claire'
link
> will be hidden.

Done.

https://codereview.chromium.org/470643002/diff/80001/chrome/browser/profiles/...
chrome/browser/profiles/profile_metrics.h:174: // can navigate to from the
Upgrade bubble during the first run.
On 2014/08/14 18:03:06, guohui wrote:
> same here, has nothing to do with first run.

Done.

[msw, 2014-08-14 20:42:03]

lgtm with nits, but aim to be more clear, consistent, and concise in your
comments and strings.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
File chrome/browser/profiles/profile_metrics.h (right):

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:146: // Enum for tracking user
interactions with the signin bubble that appears
nit: remove redundant "signin", as per "upon signin" below. The two sentences
could could also be combined to be less redundant.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:149: enum ProfileDesktopSignin {
nit: Is noting Desktop important? Maybe just note that this is only available
for desktop OSes in the comment and rename this, the histogram and helper
function to something like ProfileNewAvatarSignin.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:163: PROFILE_AVATAR_MENU_UPGRADE_VIEW,
nit: = 0

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:173: // Enum for tracking user
interactions with the Not You? bubble that users
nit: 'Not You?' like above; here and in the entries below.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:177: PROFILE_AVATAR_MENU_NOT_YOU_VIEW,
nit: = 0

https://codereview.chromium.org/470643002/diff/100001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/470643002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24724: +<histogram
name="Profile.NewAvatarMenu.NotYou"
nit: match the enum decl order: Signin, Updgrade NotYou; ditto below.

https://codereview.chromium.org/470643002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24728: +    Tracks user interactions
with the Not You? bubble that users can navigate to
nit: 'Not You?' here and below.

[Mike Lerman, 2014-08-14 20:58:30]

Thanks, Mike!

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
File chrome/browser/profiles/profile_metrics.h (right):

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:146: // Enum for tracking user
interactions with the signin bubble that appears
On 2014/08/14 20:42:03, msw wrote:
> nit: remove redundant "signin", as per "upon signin" below. The two sentences
> could could also be combined to be less redundant.

Done.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:149: enum ProfileDesktopSignin {
On 2014/08/14 20:42:03, msw wrote:
> nit: Is noting Desktop important? Maybe just note that this is only available
> for desktop OSes in the comment and rename this, the histogram and helper
> function to something like ProfileNewAvatarSignin.

Done. I went with ProfileNewAvatarMenuSignin to be consistent with the two
histograms below.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:163: PROFILE_AVATAR_MENU_UPGRADE_VIEW,
On 2014/08/14 20:42:03, msw wrote:
> nit: = 0

Done.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:173: // Enum for tracking user
interactions with the Not You? bubble that users
On 2014/08/14 20:42:03, msw wrote:
> nit: 'Not You?' like above; here and in the entries below.

Done.

https://codereview.chromium.org/470643002/diff/100001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:177: PROFILE_AVATAR_MENU_NOT_YOU_VIEW,
On 2014/08/14 20:42:03, msw wrote:
> nit: = 0

Done.

https://codereview.chromium.org/470643002/diff/100001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/470643002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24724: +<histogram
name="Profile.NewAvatarMenu.NotYou"
On 2014/08/14 20:42:03, msw wrote:
> nit: match the enum decl order: Signin, Updgrade NotYou; ditto below.

I changed the order in the profile_metrics.* files, since this file is forced to
alphabetical order.

https://codereview.chromium.org/470643002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:24728: +    Tracks user interactions
with the Not You? bubble that users can navigate to
On 2014/08/14 20:42:03, msw wrote:
> nit: 'Not You?' here and below.

Done.

[msw, 2014-08-14 21:24:49]

Still LGTM with nits.

https://codereview.chromium.org/470643002/diff/160001/chrome/browser/profiles...
File chrome/browser/profiles/profile_metrics.h (right):

https://codereview.chromium.org/470643002/diff/160001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:155: // User chooses to disconnect
(sign out) from within the Not You? bubble.
nit: 'Not You?' (nix "disconnect", "(sign out)", or "within" for one-liner?)

https://codereview.chromium.org/470643002/diff/160001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/470643002/diff/160001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:47212: +  <int value="0" label="View Not
You? Bubble">
nit: 'Not You?'

https://codereview.chromium.org/470643002/diff/160001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:47222: +    User chooses to disconnect
(sign out) from within the 'Not You?' bubble.
nit: Match any changes to the enum value comment.

[Mike Lerman, 2014-08-15 12:53:35]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-15 12:54:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlerman@chromium.org/470643002/180001

[Mike Lerman, 2014-08-15 12:55:46]

Thanks Mike!

https://codereview.chromium.org/470643002/diff/160001/chrome/browser/profiles...
File chrome/browser/profiles/profile_metrics.h (right):

https://codereview.chromium.org/470643002/diff/160001/chrome/browser/profiles...
chrome/browser/profiles/profile_metrics.h:155: // User chooses to disconnect
(sign out) from within the Not You? bubble.
On 2014/08/14 21:24:48, msw wrote:
> nit: 'Not You?' (nix "disconnect", "(sign out)", or "within" for one-liner?)

I had 3 characters to spare :)

https://codereview.chromium.org/470643002/diff/160001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/470643002/diff/160001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:47212: +  <int value="0" label="View Not
You? Bubble">
On 2014/08/14 21:24:49, msw wrote:
> nit: 'Not You?'

Done.

https://codereview.chromium.org/470643002/diff/160001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:47222: +    User chooses to disconnect
(sign out) from within the 'Not You?' bubble.
On 2014/08/14 21:24:49, msw wrote:
> nit: Match any changes to the enum value comment.

Done.

[noms (inactive), 2014-08-15 14:24:11]

profiles lgtm

[Mike Lerman, 2014-08-15 14:43:55]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-15 14:48:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlerman@chromium.org/470643002/200001

[commit-bot: I haz the power, 2014-08-15 18:00:03]

Committed patchset #11 (200001) as 289935