Move avatar_menu to disable-enable flag. A future CL will set the default...

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)

Review URL: https://codereview.chromium.org/431083003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287684 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/about_flags.cc

@@ -1456,7 +1456,8 @@ const Experiment kExperiments[] = {
     IDS_FLAGS_ENABLE_NEW_AVATAR_MENU_NAME,
     IDS_FLAGS_ENABLE_NEW_AVATAR_MENU_DESCRIPTION,
     kOsMac | kOsWin | kOsLinux,
-    SINGLE_VALUE_TYPE(switches::kNewAvatarMenu)
+    ENABLE_DISABLE_VALUE_TYPE(switches::kEnableNewAvatarMenu,
+                              switches::kDisableNewAvatarMenu)
   },
   {
     "enable-web-based-signin",

chrome/browser/notifications/message_center_settings_controller_unittest.cc

@@ -53,6 +53,7 @@ class MessageCenterSettingsControllerTest : public testing::Test {
  private:
   TestingProfileManager testing_profile_manager_;
   scoped_ptr<MessageCenterSettingsController> controller_;
+  content::TestBrowserThreadBundle thread_bundle_;
 
   DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsControllerTest);
 };

chrome/browser/profiles/profile_info_cache_unittest.cc

@@ -531,7 +531,7 @@ TEST_F(ProfileInfoCacheTest, AddStubProfile) {
 }
 
 TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(switches::kNewAvatarMenu);
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
 
   EXPECT_EQ(0U, GetCache()->GetNumberOfProfiles());
   base::FilePath path_1 = GetProfilePath("path_1");

chrome/browser/profiles/profile_list_desktop_unittest.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -86,6 +87,7 @@ class ProfileListDesktopTest : public testing::Test {
   TestingProfileManager manager_;
   scoped_ptr<MockObserver> mock_observer_;
   scoped_ptr<AvatarMenu> avatar_menu_;
+  content::TestBrowserThreadBundle thread_bundle_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileListDesktopTest);
 };

chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm

@@ -75,10 +75,6 @@ class ProfileChooserControllerTest : public CocoaProfileTest {
     [controller_ showWindow:nil];
   }
 
-  void EnableNewAvatarMenuOnly() {
-    CommandLine::ForCurrentProcess()->AppendSwitch(switches::kNewAvatarMenu);
-  }
-
   void EnableFastUserSwitching() {
     CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kFastUserSwitching);
@@ -97,7 +93,7 @@ class ProfileChooserControllerTest : public CocoaProfileTest {
 };
 
 TEST_F(ProfileChooserControllerTest, InitialLayoutWithNewMenu) {
-  EnableNewAvatarMenuOnly();
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
   StartProfileChooserController();
 
   NSArray* subviews = [[[controller() window] contentView] subviews];
@@ -159,7 +155,7 @@ TEST_F(ProfileChooserControllerTest, InitialLayoutWithNewMenu) {
 }
 
 TEST_F(ProfileChooserControllerTest, InitialLayoutWithFastUserSwitcher) {
-  EnableNewAvatarMenuOnly();
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
   EnableFastUserSwitching();
   StartProfileChooserController();
 
@@ -218,7 +214,7 @@ TEST_F(ProfileChooserControllerTest, InitialLayoutWithFastUserSwitcher) {
 }
 
 TEST_F(ProfileChooserControllerTest, OtherProfilesSortedAlphabetically) {
-  EnableNewAvatarMenuOnly();
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
   EnableFastUserSwitching();
 
   // Add two extra profiles, to make sure sorting is alphabetical and not
@@ -259,7 +255,7 @@ TEST_F(ProfileChooserControllerTest, OtherProfilesSortedAlphabetically) {
 
 TEST_F(ProfileChooserControllerTest,
     LocalProfileActiveCardLinksWithNewMenu) {
-  EnableNewAvatarMenuOnly();
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
   StartProfileChooserController();
   NSArray* subviews = [[[controller() window] contentView] subviews];
   ASSERT_EQ(1U, [subviews count]);
@@ -305,7 +301,7 @@ TEST_F(ProfileChooserControllerTest,
 
 TEST_F(ProfileChooserControllerTest,
     SignedInProfileActiveCardLinksWithNewMenu) {
-  EnableNewAvatarMenuOnly();
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
   // Sign in the first profile.
   ProfileInfoCache* cache = testing_profile_manager()->profile_info_cache();
   cache->SetUserNameOfProfileAtIndex(0, base::ASCIIToUTF16(kEmail));

chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc

@@ -53,13 +53,12 @@ ProfileChooserViewBrowserTest::~ProfileChooserViewBrowserTest() {
 
 void ProfileChooserViewBrowserTest::SetUp() {
   InProcessBrowserTest::SetUp();
-  DCHECK(CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kNewAvatarMenu));
+  DCHECK(switches::IsNewAvatarMenu());
 }
 
 void ProfileChooserViewBrowserTest::SetUpCommandLine(
     CommandLine* command_line) {
-  command_line->AppendSwitch(switches::kNewAvatarMenu);
+  switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess());
 }
 
 void ProfileChooserViewBrowserTest::OpenProfileChooserView() {

components/signin/core/browser/about_signin_internals.cc

@@ -365,14 +365,10 @@ scoped_ptr<base::DictionaryValue> AboutSigninInternals::SigninStatus::ToValue(
   AddSectionEntry(basic_info, "Signin Status", signin_status_string);
   AddSectionEntry(basic_info, "Web Based Signin Enabled?",
       switches::IsEnableWebBasedSignin() == true ? "True" : "False");
-  AddSectionEntry(basic_info, "New Profile Management Enabled?",
-      switches::IsNewProfileManagement() == true ? "True" : "False");
   AddSectionEntry(basic_info, "New Avatar Menu Enabled?",
       switches::IsNewAvatarMenu() == true ? "True" : "False");
-  bool new_avatar_menu_flag =
-      CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewAvatarMenu);
-  AddSectionEntry(basic_info, "New Avatar Menu Flag Set?",
-      new_avatar_menu_flag ? "True" : "False");
+  AddSectionEntry(basic_info, "New Profile Management Enabled?",
+      switches::IsNewProfileManagement() == true ? "True" : "False");
   AddSectionEntry(basic_info, "Account Consistency Enabled?",
       switches::IsEnableAccountConsistency() == true ? "True" : "False");
 

components/signin/core/common/profile_management_switches.cc

@@ -17,32 +17,36 @@ const char kNewProfileManagementFieldTrialName[] = "NewProfileManagement";
 // below assumes the order of the values in this enum.  That is, new profile
 // management is included in consistent identity.
 enum State {
-  STATE_NONE,
+  STATE_OLD_AVATAR_MENU,
+  STATE_NEW_AVATAR_MENU,
   STATE_NEW_PROFILE_MANAGEMENT,
-  STATE_ACCOUNT_CONSISTENCY
+  STATE_ACCOUNT_CONSISTENCY,
 };
 
 State GetProcessState() {
-  // Get the full name of the field trial so that the underlying mechanism
-  // is properly initialized.
-  std::string trial_type =
-      base::FieldTrialList::FindFullName(kNewProfileManagementFieldTrialName);
-
   // Find the state of both command line args.
+  bool is_new_avatar_menu =
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableNewAvatarMenu);
   bool is_new_profile_management =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableNewProfileManagement);
   bool is_consistent_identity =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableAccountConsistency);
+  bool not_new_avatar_menu =
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableNewAvatarMenu);
   bool not_new_profile_management =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableNewProfileManagement);
   bool not_consistent_identity =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableAccountConsistency);
-  int count_args = (is_new_profile_management ? 1 : 0) +
+  int count_args = (is_new_avatar_menu ? 1 : 0) +
+      (is_new_profile_management ? 1 : 0) +
       (is_consistent_identity ? 1 : 0) +
+      (not_new_avatar_menu ? 1 : 0) +
       (not_new_profile_management ? 1 : 0) +
       (not_consistent_identity ? 1 : 0);
   bool invalid_commandline = count_args > 1;
@@ -50,6 +54,7 @@ State GetProcessState() {
   // At most only one of the command line args should be specified, otherwise
   // the finch group assignment is undefined.  If this is the case, disable
   // the field trial so that data is not collected in the wrong group.
+  std::string trial_type;
   if (invalid_commandline) {
     base::FieldTrial* field_trial =
         base::FieldTrialList::Find(kNewProfileManagementFieldTrialName);
@@ -57,24 +62,35 @@ State GetProcessState() {
       field_trial->Disable();
 
     trial_type.clear();
+  } else {
+    // Since the experiment is not being disabled, get the full name of the
+    // field trial which will initialize the underlying mechanism.
+    trial_type =
+        base::FieldTrialList::FindFullName(kNewProfileManagementFieldTrialName);
   }
 
+  // Forcing the old avatar menu takes precedent over other args.
   // Enable command line args take precedent over disable command line args.
   // Consistent identity args take precedent over new profile management args.
-  if (is_consistent_identity) {
+  if (not_new_avatar_menu) {
+    return STATE_OLD_AVATAR_MENU;
+  } else if (is_consistent_identity) {
     return STATE_ACCOUNT_CONSISTENCY;
   } else if (is_new_profile_management) {
     return STATE_NEW_PROFILE_MANAGEMENT;
+  } else if (is_new_avatar_menu) {
+    return STATE_NEW_AVATAR_MENU;
   } else if (not_new_profile_management) {
-    return STATE_NONE;
+    return STATE_NEW_AVATAR_MENU;
   } else if (not_consistent_identity) {
-    return STATE_NEW_PROFILE_MANAGEMENT;
+    return STATE_NEW_AVATAR_MENU;
   }
 
+  // Set the default state
 #if defined(OS_ANDROID)
   State state = STATE_ACCOUNT_CONSISTENCY;
 #else
-  State state = STATE_NONE;
+  State state = STATE_OLD_AVATAR_MENU;
 #endif
 
   if (!trial_type.empty()) {
@@ -83,7 +99,7 @@ State GetProcessState() {
     } else if (trial_type == "AccountConsistency") {
       state = STATE_ACCOUNT_CONSISTENCY;
     } else {
-      state = STATE_NONE;
+      state = STATE_OLD_AVATAR_MENU;
     }
   }
 
@@ -123,13 +139,11 @@ bool IsFastUserSwitching() {
 
 bool IsGoogleProfileInfo() {
   return IsNewAvatarMenu() ||
-      CheckFlag(switches::kGoogleProfileInfo, STATE_NONE);
+      CheckFlag(switches::kGoogleProfileInfo, STATE_OLD_AVATAR_MENU);
 }
 
 bool IsNewAvatarMenu() {
-  bool is_new_avatar_menu =
-      CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewAvatarMenu);
-  return is_new_avatar_menu || IsNewProfileManagement();
+  return GetProcessState() >= STATE_NEW_AVATAR_MENU;
 }
 
 bool IsNewProfileManagement() {
@@ -142,7 +156,8 @@ bool IsNewProfileManagementPreviewEnabled() {
 }
 
 void EnableNewAvatarMenuForTesting(base::CommandLine* command_line) {
-  command_line->AppendSwitch(switches::kNewAvatarMenu);
+  command_line->AppendSwitch(switches::kEnableNewAvatarMenu);
+  DCHECK(!command_line->HasSwitch(switches::kDisableNewAvatarMenu));
 }
 
 void EnableNewProfileManagementForTesting(base::CommandLine* command_line) {

components/signin/core/common/signin_switches.cc

@@ -16,13 +16,19 @@ 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";
+
 // Disable sending signin scoped device id to LSO with refresh token request.
 const char kDisableSigninScopedDeviceId[] = "disable-signin-scoped-device-id";
 
 // Enables consistent identity features.
 const char kEnableAccountConsistency[] = "enable-account-consistency";
 
-// Enables new profile management system, including new profile chooser UI.
+// Enable the new avatar menu and the User Manager.
+const char kEnableNewAvatarMenu[] = "enable-new-avatar-menu";
+
+// Enables new profile management system, including lock mode.
 const char kEnableNewProfileManagement[] = "new-profile-management";
 
 // Enables the pure web-based flow for sign in on first run/NTP/wrench menu/
@@ -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

components/signin/core/common/signin_switches.h

@@ -15,15 +15,16 @@ namespace switches {
 // alongside the definition of their values in the .cc file.
 extern const char kClearTokenService[];
 extern const char kDisableAccountConsistency[];
+extern const char kDisableNewAvatarMenu[];
 extern const char kDisableNewProfileManagement[];
 extern const char kDisableSigninScopedDeviceId[];
 extern const char kEnableAccountConsistency[];
+extern const char kEnableNewAvatarMenu[];
 extern const char kEnableNewProfileManagement[];
 extern const char kEnableWebBasedSignin[];
 extern const char kExtensionsMultiAccount[];
 extern const char kFastUserSwitching[];
 extern const char kGoogleProfileInfo[];
-extern const char kNewAvatarMenu[];
 
 }  // namespace switches