Change new-avatar-menu to an enable-disable flag.

Move avatar_menu to disable-enable flag. A future CL will set the default behavior to be new_avatar_menu=enabled.

BUG=391497
TBR=davemoore@chromium.org (changes are very minor and are in unit tests)

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

[Mike Lerman, 2014-07-31 19:57:02]

Hi Roger,

Can you take a look at this CL?

I'm sure there will be more work to add, as likely there are unit and browser
tests to address.

Thanks,
Mike

[Roger Tawa OOO till Jul 10th, 2014-08-01 13:45:52]

lgtm

https://codereview.chromium.org/431083003/diff/1/components/signin/core/commo...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/431083003/diff/1/components/signin/core/commo...
components/signin/core/common/profile_management_switches.cc:156: }
Add DCHECK that disable is not set, similar to the two functions below?

[Mike Lerman, 2014-08-01 14:38:56]

Hi unit_test reviewers,

PTAL.

msw - c/b/ui/views
mukai - c/b/notifications
davemoore - c/b/profiles (noms is away)

Thanks.

[Mike Lerman, 2014-08-01 15:35:25]

Adding Scott to review cocoa.

Thanks!

[Jun Mukai, 2014-08-01 16:26:55]

notification lgtm

[msw, 2014-08-01 16:45:15]

To clarify, this changes the default behavior to using the new avatar menu,
right? Please state that *change* to the default more prominently in the CL
description/title. You may want to actually change the default value in a
separate CL, so it's easy to revert that without reverting the flag changes,
etc., if needed.

https://codereview.chromium.org/431083003/diff/80001/chrome/browser/ui/views/...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(left):

https://codereview.chromium.org/431083003/diff/80001/chrome/browser/ui/views/...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:56:
DCHECK(CommandLine::ForCurrentProcess()->HasSwitch(
Should this instead ASSERT_EQ/EXPECT_EQ switches::IsNewAvatarMenu()?

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:23:
STATE_ACCOUNT_CONSISTENCY
nit: trailing comma

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:54: 
nit: remove extra blank line

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.cc (right):

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.cc:28: // Enable the new profile
chooser UI and User Manager.
nit: Consider "new avatar menu" for consistency with kDisable, and use 'the'
before "User Manager."

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.h (right):

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.h:19: extern const char
kDisableNewAvatarMenu[];
nit: alphabetize this (comes before kDisableNewProfileManagement)

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.h:22: extern const char
kEnableNewAvatarMenu[];
nit: alphabetize this (comes before kEnableNewProfileManagement)

[Mike Lerman, 2014-08-01 19:15:00]

This CL no longer includes making new-avatar-menu on by default.

Good suggestion, Mike, and thanks for the other comments too!

https://codereview.chromium.org/431083003/diff/80001/chrome/browser/ui/views/...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(left):

https://codereview.chromium.org/431083003/diff/80001/chrome/browser/ui/views/...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:56:
DCHECK(CommandLine::ForCurrentProcess()->HasSwitch(
On 2014/08/01 16:45:14, msw wrote:
> Should this instead ASSERT_EQ/EXPECT_EQ switches::IsNewAvatarMenu()?

Done.

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:23:
STATE_ACCOUNT_CONSISTENCY
On 2014/08/01 16:45:14, msw wrote:
> nit: trailing comma

Done.

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:54: 
On 2014/08/01 16:45:15, msw wrote:
> nit: remove extra blank line

Done.

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.cc (right):

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.cc:28: // Enable the new profile
chooser UI and User Manager.
On 2014/08/01 16:45:15, msw wrote:
> nit: Consider "new avatar menu" for consistency with kDisable, and use 'the'
> before "User Manager."

Done.

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.h (right):

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.h:19: extern const char
kDisableNewAvatarMenu[];
On 2014/08/01 16:45:15, msw wrote:
> nit: alphabetize this (comes before kDisableNewProfileManagement)

Done.

https://codereview.chromium.org/431083003/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.h:22: extern const char
kEnableNewAvatarMenu[];
On 2014/08/01 16:45:15, msw wrote:
> nit: alphabetize this (comes before kEnableNewProfileManagement)

Done.

[msw, 2014-08-01 19:34:30]

LGTM

[msw, 2014-08-01 19:35:06]

On 2014/08/01 19:34:30, msw wrote:
> LGTM

Oh, you should update the cl title and description.

[Scott Hess - ex-Googler, 2014-08-01 22:02:56]

lgtm for cocoa

[Mike Lerman, 2014-08-04 15:54:39]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-04 15:55:29]

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

[commit-bot: I haz the power, 2014-08-04 19:05:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-04 19:05:48]

Failed to apply patch for components/signin/core/common/signin_switches.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file components/signin/core/common/signin_switches.cc
  Hunk #1 FAILED at 16.
  1 out of 2 hunks FAILED -- saving rejects to file
components/signin/core/common/signin_switches.cc.rej

Patch:       components/signin/core/common/signin_switches.cc
Index: components/signin/core/common/signin_switches.cc
diff --git a/components/signin/core/common/signin_switches.cc
b/components/signin/core/common/signin_switches.cc
index
f8be40cf98fd05c02421c52564d19048956203b3..bb54e077a6e5de4e09083bad3edfb982115946c5
100644
--- a/components/signin/core/common/signin_switches.cc
+++ b/components/signin/core/common/signin_switches.cc
@@ -16,12 +16,18 @@ const char kDisableAccountConsistency[] =
"disable-account-consistency";
 // Disables new profile management system, including new profile chooser UI.
 const char kDisableNewProfileManagement[] = "disable-new-profile-management";
 
+// Disables the new avatar menu, forcing the top-corner avatar button.
+const char kDisableNewAvatarMenu[] = "disable-new-avatar-menu";
+
 // Enables consistent identity features.
 const char kEnableAccountConsistency[] = "enable-account-consistency";
 
-// Enables new profile management system, including new profile chooser UI.
+// Enables new profile management system, including lock mode.
 const char kEnableNewProfileManagement[] = "new-profile-management";
 
+// Enable the new avatar menu and the User Manager.
+const char kEnableNewAvatarMenu[] = "enable-new-avatar-menu";
+
 // Enable sending signin scopde device id to LSO with refresh token request.
 const char kEnableSigninScopedDeviceId[] = "enable-signin-scoped-device-id";
 
@@ -39,10 +45,4 @@ const char kFastUserSwitching[] = "fast-user-switching";
 // Enables using GAIA information to populate profile name and icon.
 const char kGoogleProfileInfo[] = "google-profile-info";
 
-// Use new avatar menu. When combined with new-profile-management, it simply
-// shows the new profile management avatar menu. Otherwise it shows a
redesigned
-// avatar menu with the same functionality as the old one, plus a tutorial card
-// at the top prompting the user to try out the new profile management UI.
-const char kNewAvatarMenu[] = "new-avatar-menu";
-
 }  // namespace switches

[Mike Lerman, 2014-08-05 15:01:15]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-05 15:02:17]

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

[Mike Lerman, 2014-08-05 18:32:57]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-05 18:33:29]

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

[commit-bot: I haz the power, 2014-08-05 23:05:20]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)

[commit-bot: I haz the power, 2014-08-06 05:37:49]

Change committed as 287684