[Signin]: Migrate Legacy default profile names to 'Person %n' on Desktop.

This CL does the following:
(1) If the user has not customized their local profile name |IsUsingDefaultName()
== true| and their local profile name is not 'Person %n', we change the
local profile name to 'Person %n'.
(2) For newly created profiles, we set the local profile name to 'Person %n'
unless the user specifies something else.
(3) Fix some corner cases for the display profile name.

Bug: 1018719
Change-Id: Ic92bde725d5e33105e251214f01b646a2ff63088
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1878377
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710277}

chrome/browser/profiles/profile_attributes_entry.h

@@ -48,6 +48,9 @@ class ProfileAttributesEntry {
   // Gets the name of the profile to be displayed in the User Menu. The name can
   // be the GAIA name, local profile name or a combination of them.
   base::string16 GetName() const;
+  // Returns |GetGAIAGivenName()| if not empty. Otherwise, returns
+  // |GetGAIAName()|.
+  base::string16 GetGAIANameToDisplay() const;
   // Returns true if the profile name has changed.
   bool HasProfileNameChanged();
 
@@ -173,7 +176,6 @@ class ProfileAttributesEntry {
   //   return false.
   // - Otherwise the concatenation of GAIA name and local profile name.
   base::string16 GetNameToDisplay() const;
-  base::string16 GetGAIANameToDisplay() const;
   base::string16 GetLastNameToDisplay() const;
 
   // Returns true if:

chrome/browser/profiles/profile_attributes_storage.cc

@@ -224,7 +224,23 @@ base::string16 ProfileAttributesStorage::ChooseNameForNewProfile(
 }
 
 bool ProfileAttributesStorage::IsDefaultProfileName(
-    const base::string16& name) const {
+    const base::string16& name,
+    bool include_check_for_legacy_profile_name) const {
+  // Check whether it's one of the "Person %d" style names.
+  std::string default_name_format = l10n_util::GetStringFUTF8(
+      IDS_NEW_NUMBERED_PROFILE_NAME, base::ASCIIToUTF16("%d"));
+  int generic_profile_number;  // Unused. Just a placeholder for sscanf.
+  int assignments =
+      sscanf(base::UTF16ToUTF8(name).c_str(), default_name_format.c_str(),
+             &generic_profile_number);
+  if (assignments == 1)
+    return true;
+
+#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
+  if (!include_check_for_legacy_profile_name)
+    return false;
+#endif
+
   // Check if it's a "First user" old-style name.
   if (name == l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME) ||
       name == l10n_util::GetStringUTF16(IDS_LEGACY_DEFAULT_PROFILE_NAME))
@@ -235,17 +251,7 @@ bool ProfileAttributesStorage::IsDefaultProfileName(
     if (name == l10n_util::GetStringUTF16(kDefaultNames[i]))
       return true;
   }
-
-  // Check whether it's one of the "Person %d" style names.
-  std::string default_name_format = l10n_util::GetStringFUTF8(
-      IDS_NEW_NUMBERED_PROFILE_NAME, base::ASCIIToUTF16("%d"));
-
-  int generic_profile_number;  // Unused. Just a placeholder for sscanf.
-  int assignments = sscanf(base::UTF16ToUTF8(name).c_str(),
-                           default_name_format.c_str(),
-                           &generic_profile_number);
-  // Unless it matched the format, this is a custom name.
-  return assignments == 1;
+  return false;
 }
 
 size_t ProfileAttributesStorage::ChooseAvatarIconIndexForNewProfile() const {

chrome/browser/profiles/profile_attributes_storage.h

@@ -84,7 +84,15 @@ class ProfileAttributesStorage
   base::string16 ChooseNameForNewProfile(size_t icon_index) const;
 
   // Determines whether |name| is one of the default assigned names.
-  bool IsDefaultProfileName(const base::string16& name) const;
+  // On Desktop, if |include_check_for_legacy_profile_name| is false,
+  // |IsDefaultProfileName()| would only return true if the |name| is in the
+  // form of |Person %n| which is the new default local profile name. If
+  // |include_check_for_legacy_profile_name| is true, we will also check if name
+  // is one of the legacy profile names (e.g. Saratoga, Default user, ..).
+  // For other platforms, so far |include_check_for_legacy_profile_name|
+  // is not used.
+  bool IsDefaultProfileName(const base::string16& name,
+                            bool include_check_for_legacy_profile_name) const;
 
   // Returns an avatar icon index that can be assigned to a newly created
   // profile. Note that the icon may not be unique since there are a limited

chrome/browser/profiles/profile_info_cache.cc

@@ -49,6 +49,10 @@ const char kIsOmittedFromProfileListKey[] = "is_omitted_from_profile_list";
 const char kSigninRequiredKey[] = "signin_required";
 const char kSupervisedUserId[] = "managed_user_id";
 const char kAccountIdKey[] = "account_id_key";
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+const char kLegacyProfileNameMigrated[] = "legacy.profile.name.migrated";
+bool migration_enabled_for_testing_ = false;
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
 void DeleteBitmap(const base::FilePath& image_path) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
@@ -91,7 +95,10 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
     if (!info->GetBoolean(kIsUsingDefaultNameKey, &using_default_name)) {
       // If the preference hasn't been set, and the name is default, assume
       // that the user hasn't done this on purpose.
-      using_default_name = IsDefaultProfileName(name);
+      // |include_check_for_legacy_profile_name| is true as this is an old
+      // pre-existing profile and might have a legacy default profile name.
+      using_default_name = IsDefaultProfileName(
+          name, /*include_check_for_legacy_profile_name=*/true);
       info->SetBoolean(kIsUsingDefaultNameKey, using_default_name);
     }
 
@@ -105,9 +112,18 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
   // If needed, start downloading the high-res avatars and migrate any legacy
   // profile names.
   if (!disable_avatar_download_for_testing_)
-    MigrateLegacyProfileNamesAndDownloadAvatars();
+    DownloadAvatars();
 
-  RecomputeProfileNamesIfNeeded();
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  bool migrate_legacy_profile_names =
+      (!prefs_->GetBoolean(kLegacyProfileNameMigrated) &&
+       ProfileAttributesEntry::ShouldConcatenateGaiaAndProfileName()) ||
+      migration_enabled_for_testing_;
+  if (migrate_legacy_profile_names) {
+    MigrateLegacyProfileNamesAndRecomputeIfNeeded();
+    prefs_->SetBoolean(kLegacyProfileNameMigrated, true);
+  }
+#endif  //! defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 }
 
 ProfileInfoCache::~ProfileInfoCache() {
@@ -148,7 +164,12 @@ void ProfileInfoCache::AddProfileToCache(const base::FilePath& profile_path,
   info->SetString(kSupervisedUserId, supervised_user_id);
   info->SetBoolean(kIsOmittedFromProfileListKey, !supervised_user_id.empty());
   info->SetBoolean(ProfileAttributesEntry::kProfileIsEphemeral, false);
-  info->SetBoolean(kIsUsingDefaultNameKey, IsDefaultProfileName(name));
+  // Either the user has provided a name manually on purpose, and in this case
+  // we should not check for legacy profile names or this a new profile but then
+  // it is not a legacy name, so we dont need to check for legacy names.
+  info->SetBoolean(kIsUsingDefaultNameKey,
+                   IsDefaultProfileName(
+                       name, /*include_check_for_legacy_profile_name*/ false));
   // Assume newly created profiles use a default avatar.
   info->SetBoolean(kIsUsingDefaultAvatarKey, true);
   if (account_id.HasAccountIdKey())
@@ -649,6 +670,9 @@ const base::FilePath& ProfileInfoCache::GetUserDataDir() const {
 // static
 void ProfileInfoCache::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kProfileInfoCache);
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  registry->RegisterBooleanPref(kLegacyProfileNameMigrated, false);
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 }
 
 const base::DictionaryValue* ProfileInfoCache::GetInfoForProfileAtIndex(
@@ -695,49 +719,52 @@ const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
                                    image_path);
 }
 
-void ProfileInfoCache::RecomputeProfileNamesIfNeeded() {
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+void ProfileInfoCache::MigrateLegacyProfileNamesAndRecomputeIfNeeded() {
+  DCHECK(ProfileAttributesEntry::ShouldConcatenateGaiaAndProfileName());
   std::vector<ProfileAttributesEntry*> entries = GetAllProfilesAttributes();
-  if (entries.size() < 2)
-    return;
+  for (size_t i = 0; i < entries.size(); i++) {
+    base::string16 profile_name = entries[i]->GetLocalProfileName();
+    if (!entries[i]->IsUsingDefaultName())
+      continue;
+
+    // Migrate any legacy profile names ("First user", "Default Profile",
+    // "Saratoga", ...) to new style default names Person %n ("Person 1").
+    if (!IsDefaultProfileName(
+            profile_name, /*include_check_for_legacy_profile_name=*/false)) {
+      entries[i]->SetLocalProfileName(
+          ChooseNameForNewProfile(entries[i]->GetAvatarIconIndex()));
+      continue;
+    }
 
-  for (size_t i = 0; i < entries.size() - 1; i++) {
-    base::string16 name = entries[i]->GetLocalProfileName();
-    if (!IsDefaultProfileName(name))
+    if (i == (entries.size() - 1))
       continue;
 
+    // Current profile name is Person %n.
+    // Rename duplicate default profile names, e.g.: Person 1, Person 1 to
+    // Person 1, Person 2.
     for (size_t j = i + 1; j < entries.size(); j++) {
-      if (name == entries[j]->GetLocalProfileName()) {
+      if (profile_name == entries[j]->GetLocalProfileName()) {
         entries[j]->SetLocalProfileName(
             ChooseNameForNewProfile(entries[j]->GetAvatarIconIndex()));
       }
     }
   }
-#endif
 }
 
-void ProfileInfoCache::MigrateLegacyProfileNamesAndDownloadAvatars() {
+// static
+void ProfileInfoCache::EnableLegacyProfileMigrationForTesting() {
+  migration_enabled_for_testing_ = true;
+}
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+
+void ProfileInfoCache::DownloadAvatars() {
   // Only do this on desktop platforms.
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
-  // Migrate any legacy default profile names ("First user", "Default Profile")
-  // to new style default names ("Person 1").
-  const base::string16 default_profile_name = base::i18n::ToLower(
-      l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME));
-  const base::string16 default_legacy_profile_name = base::i18n::ToLower(
-      l10n_util::GetStringUTF16(IDS_LEGACY_DEFAULT_PROFILE_NAME));
-
   std::vector<ProfileAttributesEntry*> entries = GetAllProfilesAttributes();
   for (ProfileAttributesEntry* entry : entries) {
     DownloadHighResAvatarIfNeeded(entry->GetAvatarIconIndex(),
                                   entry->GetPath());
-
-    // Rename the necessary profiles.
-    base::string16 name = base::i18n::ToLower(entry->GetLocalProfileName());
-    if (name == default_profile_name || name == default_legacy_profile_name) {
-      entry->SetIsUsingDefaultName(true);
-      entry->SetLocalProfileName(
-          ChooseNameForNewProfile(entry->GetAvatarIconIndex()));
-    }
   }
 #endif
 }

chrome/browser/profiles/profile_info_cache.h

@@ -20,6 +20,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_info_cache_observer.h"
@@ -168,6 +169,8 @@ class ProfileInfoCache : public ProfileInfoInterface,
  private:
   FRIEND_TEST_ALL_PREFIXES(ProfileAttributesStorageTest,
                            DownloadHighResAvatarTest);
+  FRIEND_TEST_ALL_PREFIXES(ProfileInfoCacheTest,
+                           MigrateLegacyProfileNamesAndRecomputeIfNeeded);
 
   const base::DictionaryValue* GetInfoForProfileAtIndex(size_t index) const;
   // Saves the profile info to a cache.
@@ -187,17 +190,18 @@ class ProfileInfoCache : public ProfileInfoInterface,
   // generic profile avatar.
   const gfx::Image* GetHighResAvatarOfProfileAtIndex(size_t index) const;
 
-  // Migrate any legacy profile names ("First user", "Default Profile") to
-  // new style default names ("Person 1"), and download and high-res avatars
-  // used by the profiles.
-  void MigrateLegacyProfileNamesAndDownloadAvatars();
+  // Download and high-res avatars used by the profiles.
+  void DownloadAvatars();
+  void NotifyIfProfileNamesHaveChanged();
 
-  // Recompute profile names to guarantee there are no duplicates of "Person n"
-  // exist, i.e. Two or more profiles with the profile name "Person 1" would be
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  // Migrate any legacy profile names ("First user", "Default Profile") to
+  // new style default names ("Person 1"). Rename any duplicates of "Person n"
+  // i.e. Two or more profiles with the profile name "Person 1" would be
   // recomputed to "Person 1" and "Person 2".
-  void RecomputeProfileNamesIfNeeded();
-
-  void NotifyIfProfileNamesHaveChanged();
+  void MigrateLegacyProfileNamesAndRecomputeIfNeeded();
+  static void EnableLegacyProfileMigrationForTesting();
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
   std::vector<std::string> keys_;
   const base::FilePath user_data_dir_;

chrome/browser/profiles/profile_info_cache_unittest.cc

@@ -397,12 +397,12 @@ TEST_F(ProfileInfoCacheTest, ConcatenateGaiaNameAndProfileName) {
             GetCache()->GetNameToDisplayOfProfileAtIndex(index3));
 
   GetCache()->SetLocalProfileNameOfProfileAtIndex(index1, ASCIIToUTF16("Patt"));
-  // EXPECT_EQ(ASCIIToUTF16("Patt (Patt)"),
-  //           GetCache()->GetNameToDisplayOfProfileAtIndex(index1));
-  // EXPECT_EQ(ASCIIToUTF16("Patt (patt)"),
-  //           GetCache()->GetNameToDisplayOfProfileAtIndex(index3));
-  // EXPECT_EQ(ASCIIToUTF16("Olly"),
-  //           GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
+  EXPECT_EQ(ASCIIToUTF16("Patt (Patt)"),
+            GetCache()->GetNameToDisplayOfProfileAtIndex(index1));
+  EXPECT_EQ(ASCIIToUTF16("Patt (patt)"),
+            GetCache()->GetNameToDisplayOfProfileAtIndex(index3));
+  EXPECT_EQ(ASCIIToUTF16("Olly"),
+            GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
 }
 
 TEST_F(ProfileInfoCacheTest, DeleteProfile) {
@@ -787,112 +787,128 @@ TEST_F(ProfileInfoCacheTest, EntriesInAttributesStorage) {
 }
 
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
-TEST_F(ProfileInfoCacheTest, RecomputeProfileNamesIfNeeded) {
-  // Duplicate profile names causes the observer to crash.
-  EXPECT_EQ(0U, GetCache()->GetNumberOfProfiles());
-
-  base::FilePath path_1 = GetProfilePath("path_1");
-  base::string16 name_1 = ASCIIToUTF16("Person 3");
-  GetCache()->AddProfileToCache(path_1, name_1, std::string(), base::string16(),
-                                false, 0, std::string(), EmptyAccountId());
-  base::FilePath path_2 = GetProfilePath("path_2");
-  GetCache()->AddProfileToCache(path_2, ASCIIToUTF16("Person 1"), std::string(),
-                                base::string16(), false, 1, std::string(),
-                                EmptyAccountId());
-  base::FilePath path_3 = GetProfilePath("path_3");
-  GetCache()->AddProfileToCache(path_3, ASCIIToUTF16("Person 2"), std::string(),
-                                base::string16(), false, 2, std::string(),
-                                EmptyAccountId());
-  base::FilePath path_4 = GetProfilePath("path_4");
-  GetCache()->AddProfileToCache(path_4, ASCIIToUTF16("Person 1"), std::string(),
-                                base::string16(), false, 3, std::string(),
-                                EmptyAccountId());
-  base::string16 name_5 = ASCIIToUTF16("Smith");
-  base::FilePath path_5 = GetProfilePath("path_5");
-  GetCache()->AddProfileToCache(path_5, name_5, std::string(), base::string16(),
-                                false, 2, std::string(), EmptyAccountId());
-  base::FilePath path_6 = GetProfilePath("path_6");
-  GetCache()->AddProfileToCache(path_6, ASCIIToUTF16("Person 2"), std::string(),
-                                base::string16(), false, 2, std::string(),
-                                EmptyAccountId());
-
-  EXPECT_EQ(6U, GetCache()->GetNumberOfProfiles());
-
-  ResetCache();
-  // Check non-duplicate profile name is not changed.
-  EXPECT_EQ(name_1, GetCache()->GetNameToDisplayOfProfileAtIndex(
-                        GetCache()->GetIndexOfProfileWithPath(path_1)));
-  // Check nondefault profile name is not changed.
-  EXPECT_EQ(name_5, GetCache()->GetNameToDisplayOfProfileAtIndex(
-                        GetCache()->GetIndexOfProfileWithPath(path_5)));
-
-  // Check there is no duplicate profile name.
-  std::vector<ProfileAttributesEntry*> entries =
-      GetCache()->GetAllProfilesAttributes();
-  for (size_t index = 1; index < entries.size(); index++) {
-    base::string16 name = entries[index - 1]->GetLocalProfileName();
-    EXPECT_TRUE(std::none_of(entries.begin() + index, entries.end(),
-                             [name](ProfileAttributesEntry* entry) {
-                               return entry->GetLocalProfileName() == name;
-                             }));
-  }
-}
-
-TEST_F(ProfileInfoCacheTest, MigrateLegacyProfileNamesWithNewAvatarMenu) {
+TEST_F(ProfileInfoCacheTest, MigrateLegacyProfileNamesAndRecomputeIfNeeded) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kProfileMenuRevamp);
   EXPECT_EQ(0U, GetCache()->GetNumberOfProfiles());
-
+  // Mimick a pre-existing Directory with profiles that has legacy profile
+  // names.
   base::FilePath path_1 = GetProfilePath("path_1");
   GetCache()->AddProfileToCache(path_1, ASCIIToUTF16("Default Profile"),
                                 std::string(), base::string16(), false, 0,
                                 std::string(), EmptyAccountId());
+  int index1 = GetCache()->GetIndexOfProfileWithPath(path_1);
+  GetCache()->SetProfileIsUsingDefaultNameAtIndex(index1, true);
+
   base::FilePath path_2 = GetProfilePath("path_2");
   GetCache()->AddProfileToCache(path_2, ASCIIToUTF16("First user"),
                                 std::string(), base::string16(), false, 1,
                                 std::string(), EmptyAccountId());
+  int index2 = GetCache()->GetIndexOfProfileWithPath(path_2);
+  GetCache()->SetProfileIsUsingDefaultNameAtIndex(index2, true);
+
   base::string16 name_3 = ASCIIToUTF16("Lemonade");
   base::FilePath path_3 = GetProfilePath("path_3");
   GetCache()->AddProfileToCache(path_3, name_3, std::string(), base::string16(),
                                 false, 2, std::string(), EmptyAccountId());
+  int index3 = GetCache()->GetIndexOfProfileWithPath(path_3);
+  EXPECT_FALSE(GetCache()->ProfileIsUsingDefaultNameAtIndex(index3));
+  // Set is using default to true, should migrate "Lemonade" to "Person %n".
+  GetCache()->SetProfileIsUsingDefaultNameAtIndex(index3, true);
+
   base::string16 name_4 = ASCIIToUTF16("Batman");
   base::FilePath path_4 = GetProfilePath("path_4");
   GetCache()->AddProfileToCache(path_4, name_4, std::string(), base::string16(),
                                 false, 3, std::string(), EmptyAccountId());
-  base::string16 name_5 = ASCIIToUTF16("Person 2");
+  int index4 = GetCache()->GetIndexOfProfileWithPath(path_4);
+  GetCache()->SetProfileIsUsingDefaultNameAtIndex(index4, true);
+
+  // Should not be migrated.
+  base::string16 name_5 = ASCIIToUTF16("Batman");
   base::FilePath path_5 = GetProfilePath("path_5");
   GetCache()->AddProfileToCache(path_5, name_5, std::string(), base::string16(),
+                                false, 3, std::string(), EmptyAccountId());
+
+  base::string16 name_6 = ASCIIToUTF16("Person 2");
+  base::FilePath path_6 = GetProfilePath("path_6");
+  GetCache()->AddProfileToCache(path_6, name_6, std::string(), base::string16(),
                                 false, 2, std::string(), EmptyAccountId());
 
-  EXPECT_EQ(5U, GetCache()->GetNumberOfProfiles());
+  base::FilePath path_7 = GetProfilePath("path_7");
+  base::string16 name_7 = ASCIIToUTF16("Person 3");
+  GetCache()->AddProfileToCache(path_7, name_7, std::string(), base::string16(),
+                                false, 0, std::string(), EmptyAccountId());
+
+  // Should be renamed to unique Person %n.
+  base::FilePath path_8 = GetProfilePath("path_8");
+  GetCache()->AddProfileToCache(path_8, ASCIIToUTF16("Person 1"), std::string(),
+                                base::string16(), false, 1, std::string(),
+                                EmptyAccountId());
+  base::FilePath path_9 = GetProfilePath("path_9");
+  GetCache()->AddProfileToCache(path_9, ASCIIToUTF16("Person 2"), std::string(),
+                                base::string16(), false, 2, std::string(),
+                                EmptyAccountId());
+  base::FilePath path_10 = GetProfilePath("path_10");
+  GetCache()->AddProfileToCache(path_10, ASCIIToUTF16("Person 1"),
+                                std::string(), base::string16(), false, 3,
+                                std::string(), EmptyAccountId());
+  // Should not be migrated.
+  base::string16 name_11 = ASCIIToUTF16("Smith");
+  base::FilePath path_11 = GetProfilePath("path_11");
+  GetCache()->AddProfileToCache(path_11, name_11, std::string(),
+                                base::string16(), false, 2, std::string(),
+                                EmptyAccountId());
+  base::FilePath path_12 = GetProfilePath("path_12");
+  GetCache()->AddProfileToCache(path_12, ASCIIToUTF16("Person 2"),
+                                std::string(), base::string16(), false, 2,
+                                std::string(), EmptyAccountId());
 
+  EXPECT_EQ(12U, GetCache()->GetNumberOfProfiles());
 
   ResetCache();
+  ProfileInfoCache::EnableLegacyProfileMigrationForTesting();
+
+  EXPECT_EQ(name_5, GetCache()->GetNameToDisplayOfProfileAtIndex(
+                        GetCache()->GetIndexOfProfileWithPath(path_5)));
+  EXPECT_EQ(name_11, GetCache()->GetNameToDisplayOfProfileAtIndex(
+                         GetCache()->GetIndexOfProfileWithPath(path_11)));
 
   // Legacy profile names like "Default Profile" and "First user" should be
-  // migrated to "Person %n" type names, i.e. any permutation of "Person 1" and
-  // "Person 3".
-  if (ASCIIToUTF16("Person 1") ==
-      GetCache()->GetNameToDisplayOfProfileAtIndex(
-          GetCache()->GetIndexOfProfileWithPath(path_1))) {
-    EXPECT_EQ(ASCIIToUTF16("Person 3"),
-              GetCache()->GetNameToDisplayOfProfileAtIndex(
-                  GetCache()->GetIndexOfProfileWithPath(path_2)));
-  } else {
-    EXPECT_EQ(ASCIIToUTF16("Person 3"),
-              GetCache()->GetNameToDisplayOfProfileAtIndex(
+  // migrated to "Person %n" type names, i.e. any permutation of "Person %n".
+  std::set<base::string16> expected_profile_names;
+  expected_profile_names.insert(ASCIIToUTF16("Person 1"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 2"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 3"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 4"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 5"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 6"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 7"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 8"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 9"));
+  expected_profile_names.insert(ASCIIToUTF16("Person 10"));
+
+  std::set<base::string16> actual_profile_names;
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
       GetCache()->GetIndexOfProfileWithPath(path_1)));
-    EXPECT_EQ(ASCIIToUTF16("Person 1"),
-              GetCache()->GetNameToDisplayOfProfileAtIndex(
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
       GetCache()->GetIndexOfProfileWithPath(path_2)));
-  }
-
-  // Other profile names should not be migrated even if they're the old
-  // default cartoon profile names.
-  EXPECT_EQ(name_3, GetCache()->GetNameToDisplayOfProfileAtIndex(
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
       GetCache()->GetIndexOfProfileWithPath(path_3)));
-  EXPECT_EQ(name_4, GetCache()->GetNameToDisplayOfProfileAtIndex(
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
       GetCache()->GetIndexOfProfileWithPath(path_4)));
-  EXPECT_EQ(name_5, GetCache()->GetNameToDisplayOfProfileAtIndex(
-                        GetCache()->GetIndexOfProfileWithPath(path_5)));
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
+      GetCache()->GetIndexOfProfileWithPath(path_6)));
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
+      GetCache()->GetIndexOfProfileWithPath(path_7)));
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
+      GetCache()->GetIndexOfProfileWithPath(path_8)));
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
+      GetCache()->GetIndexOfProfileWithPath(path_9)));
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
+      GetCache()->GetIndexOfProfileWithPath(path_10)));
+  actual_profile_names.insert(GetCache()->GetNameToDisplayOfProfileAtIndex(
+      GetCache()->GetIndexOfProfileWithPath(path_12)));
+  EXPECT_EQ(actual_profile_names, expected_profile_names);
 }
 
 TEST_F(ProfileInfoCacheTest, GetGaiaImageForAvatarMenu) {

chrome/browser/profiles/profile_manager_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "build/build_config.h"
@@ -31,6 +32,7 @@
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -1523,6 +1525,14 @@ TEST_F(ProfileManagerTest, ProfileDisplayNamePreservesSignedInName) {
 TEST_F(ProfileManagerTest, ProfileDisplayNameIsEmailIfDefaultName) {
   if (!profiles::IsMultipleProfilesEnabled())
     return;
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  // This test is relevant on desktop if |kProfileMenuRevamp| is disabled. If
+  // it is enabled for pre-existing directory with legacy profile name, they
+  // will be migrated to new default profile name |Person %n|. For newly created
+  // profiles, only |Person %n| is considered as a default profile name.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(features::kProfileMenuRevamp);
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileAttributesStorage& storage =
@@ -1553,15 +1563,23 @@ TEST_F(ProfileManagerTest, ProfileDisplayNameIsEmailIfDefaultName) {
   entry->SetGAIAGivenName(base::string16());
   entry->SetGAIAName(base::string16());
 
-  // This may resort the cache, so be extra cautious to use the right profile.
   ASSERT_TRUE(storage.GetProfileAttributesWithPath(profile2->GetPath(),
                                                    &entry));
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  // (Default profile, Batman,..) are legacy profile names on Desktop and are
+  // not considered default profile names for newly created profiles.
+  // We use "Person %n" as the default profile name. Set |SetIsUsingDefaultName|
+  // manually to mimick pre-existing profiles.
+  entry->SetIsUsingDefaultName(true);
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+
   entry->SetAuthInfo("23456", email2, true);
   entry->SetGAIAGivenName(base::string16());
   entry->SetGAIAName(base::string16());
 
   ASSERT_TRUE(storage.GetProfileAttributesWithPath(profile3->GetPath(),
                                                    &entry));
+
   entry->SetAuthInfo("34567", email3, true);
   entry->SetGAIAGivenName(base::string16());
   entry->SetGAIAName(base::string16());
@@ -1576,7 +1594,7 @@ TEST_F(ProfileManagerTest, ProfileDisplayNameIsEmailIfDefaultName) {
 
   // Adding a Gaia name to a profile that previously had a default name should
   // start displaying it.
-  const base::string16 gaia_given_name(ASCIIToUTF16("Robin (Person 1)"));
+  const base::string16 gaia_given_name(ASCIIToUTF16("Robin"));
   ASSERT_TRUE(storage.GetProfileAttributesWithPath(profile1->GetPath(),
                                                    &entry));
   entry->SetGAIAGivenName(gaia_given_name);

chrome/browser/profiles/profile_metrics.cc

@@ -199,7 +199,7 @@ bool ProfileMetrics::CountProfileInformation(ProfileManager* manager,
       counts->unused++;
     } else {
       counts->active++;
-      if (!storage.IsDefaultProfileName(entry->GetLocalProfileName()))
+      if (!entry->IsUsingDefaultName())
         counts->named++;
       if (entry->IsSupervised())
         counts->supervised++;

chrome/browser/profiles/profiles_state.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
@@ -83,11 +84,10 @@ void SetLastUsedProfile(const std::string& profile_dir) {
 
 #if !defined(OS_ANDROID)
 base::string16 GetAvatarNameForProfile(const base::FilePath& profile_path) {
-  base::string16 display_name;
-
   if (profile_path == ProfileManager::GetGuestProfilePath()) {
-    display_name = l10n_util::GetStringUTF16(IDS_GUEST_PROFILE_NAME);
-  } else {
+    return l10n_util::GetStringUTF16(IDS_GUEST_PROFILE_NAME);
+  }
+
   ProfileAttributesStorage& storage =
       g_browser_process->profile_manager()->GetProfileAttributesStorage();
 
@@ -95,23 +95,30 @@ base::string16 GetAvatarNameForProfile(const base::FilePath& profile_path) {
   if (!storage.GetProfileAttributesWithPath(profile_path, &entry))
     return l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME);
 
-    // Using the --new-avatar-menu flag, there's a couple of rules about what
-    // the avatar button displays. If there's a single profile, with a default
-    // name (i.e. of the form Person %d) not manually set, it should display
-    // IDS_SINGLE_PROFILE_DISPLAY_NAME. If the profile is signed in but is using
-    // a default name, use the profiles's email address. Otherwise, it
-    // will return the actual name of the profile.
-    const base::string16 profile_name = entry->GetName();
-    const base::string16 email = entry->GetUserName();
-    bool is_default_name = entry->IsUsingDefaultName() &&
-        storage.IsDefaultProfileName(profile_name);
+  const base::string16 profile_name_to_display = entry->GetName();
+  // If the user has set their local profile name on purpose.
+  bool is_default_name = entry->IsUsingDefaultName();
+  if (!is_default_name)
+    return profile_name_to_display;
+
+  // The profile is signed in and has a GAIA name.
+  const base::string16 gaia_name_to_display = entry->GetGAIANameToDisplay();
+  if (!gaia_name_to_display.empty())
+    return profile_name_to_display;
+
+  // For a single profile that does not have a GAIA name
+  // (most probably not signed in), with a default name
+  // (i.e. of the form Person %d) not manually set, it should display
+  // IDS_SINGLE_PROFILE_DISPLAY_NAME.
+  if (storage.GetNumberOfProfiles() == 1u)
+    return l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME);
 
-    if (storage.GetNumberOfProfiles() == 1u && is_default_name)
-      display_name = l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME);
-    else
-      display_name = (is_default_name && !email.empty()) ? email : profile_name;
-  }
-  return display_name;
+  // If the profile is signed in but does not have a GAIA name nor a custom
+  // local profile name, show the email address if it exists.
+  // Otherwise, show the profile name which is expected to be the local
+  // profile name.
+  const base::string16 email = entry->GetUserName();
+  return email.empty() ? profile_name_to_display : email;
 }
 
 #if !defined(OS_CHROMEOS)

chrome/browser/signin/signin_profile_attributes_updater.cc

@@ -65,6 +65,22 @@ void SigninProfileAttributesUpdater::UpdateProfileAttributes() {
     // Reset prefs. Note: this will also update the |ProfileAttributesEntry|.
     prefs_->ClearPref(prefs::kProfileUsingDefaultAvatar);
     prefs_->ClearPref(prefs::kProfileUsingGAIAAvatar);
+
+    // If the concatenation is not enabled, we either show the GAIA name or
+    // the local profile name based on |prefs::kProfileUsingDefaultName|.
+    // If the profile has been created with a custom name, we need to reset
+    // |prefs::kProfileUsingDefaultName| on sign in/sync events for the display
+    // name to be the GAIA name otherwise it will be the custom local profile
+    // name.
+    if (!clear_profile &&
+        !ProfileAttributesEntry::ShouldConcatenateGaiaAndProfileName()) {
+      prefs_->SetString(
+          prefs::kProfileName,
+          base::UTF16ToUTF8(
+              profile_attributes_storage_->ChooseNameForNewProfile(
+                  entry->GetAvatarIconIndex())));
+      prefs_->ClearPref(prefs::kProfileUsingDefaultName);
+    }
   }
 
   if (clear_profile) {

chrome/browser/signin/signin_profile_attributes_updater_unittest.cc

@@ -7,10 +7,12 @@
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/signin/signin_util.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -27,21 +29,29 @@ namespace {
 const char kEmail[] = "example@email.com";
 
 #if !defined(OS_CHROMEOS)
-void CheckProfilePrefsReset(PrefService* pref_service) {
+void CheckProfilePrefsReset(PrefService* pref_service,
+                            bool expected_using_default_name) {
   EXPECT_TRUE(pref_service->GetBoolean(prefs::kProfileUsingDefaultAvatar));
   EXPECT_FALSE(pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar));
+  EXPECT_EQ(expected_using_default_name,
+            pref_service->GetBoolean(prefs::kProfileUsingDefaultName));
 }
 
-void CheckProfilePrefsSet(PrefService* pref_service) {
+void CheckProfilePrefsSet(PrefService* pref_service,
+                          bool expected_is_using_default_name) {
   EXPECT_FALSE(pref_service->GetBoolean(prefs::kProfileUsingDefaultAvatar));
   EXPECT_TRUE(pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar));
+  EXPECT_EQ(expected_is_using_default_name,
+            pref_service->GetBoolean(prefs::kProfileUsingDefaultName));
 }
 
 // Set the prefs to nondefault values.
 void SetProfilePrefs(PrefService* pref_service) {
   pref_service->SetBoolean(prefs::kProfileUsingDefaultAvatar, false);
   pref_service->SetBoolean(prefs::kProfileUsingGAIAAvatar, true);
-  CheckProfilePrefsSet(pref_service);
+  pref_service->SetBoolean(prefs::kProfileUsingDefaultName, false);
+
+  CheckProfilePrefsSet(pref_service, false);
 }
 #endif  // !defined(OS_CHROMEOS)
 }  // namespace
@@ -139,14 +149,41 @@ TEST_F(SigninProfileAttributesUpdaterTest, AuthError) {
 }
 
 #if !defined(OS_CHROMEOS)
-TEST_F(SigninProfileAttributesUpdaterTest, SigninSignoutResetsProfilePrefs) {
+class SigninProfileAttributesUpdaterTestWithParam
+    : public SigninProfileAttributesUpdaterTest,
+      public ::testing::WithParamInterface<bool> {
+ public:
+  SigninProfileAttributesUpdaterTestWithParam()
+      : SigninProfileAttributesUpdaterTest() {
+    concatenate_enabled_ = GetParam();
+    if (concatenate_enabled_) {
+      scoped_feature_list_.InitAndEnableFeature(features::kProfileMenuRevamp);
+    } else {
+      scoped_feature_list_.InitAndDisableFeature(features::kProfileMenuRevamp);
+    }
+  }
+
+ protected:
+  base::test::ScopedFeatureList scoped_feature_list_;
+  bool concatenate_enabled_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SigninProfileAttributesUpdaterTestWithParam);
+};
+
+INSTANTIATE_TEST_SUITE_P(SigninProfileAttributesUpdaterTest,
+                         SigninProfileAttributesUpdaterTestWithParam,
+                         testing::Bool());
+
+TEST_P(SigninProfileAttributesUpdaterTestWithParam,
+       SigninSignoutResetsProfilePrefs) {
   PrefService* pref_service = profile_->GetPrefs();
   ProfileAttributesEntry* entry;
   ASSERT_TRUE(profile_manager_.profile_attributes_storage()
                   ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
 
   // Set profile prefs.
-  CheckProfilePrefsReset(pref_service);
+  CheckProfilePrefsReset(pref_service, true);
 #if !defined(OS_ANDROID)
   SetProfilePrefs(pref_service);
 
@@ -154,22 +191,24 @@ TEST_F(SigninProfileAttributesUpdaterTest, SigninSignoutResetsProfilePrefs) {
   AccountInfo account_info = identity_test_env_.MakeAccountAvailableWithCookies(
       "email1@example.com", "gaia_id_1");
   EXPECT_FALSE(entry->IsAuthenticated());
-  CheckProfilePrefsReset(pref_service);
+  // If concatenate is disabled, we reset kProfileIsUsingDefault to true on
+  // sign in/ sync. Otherwise, we don't reset kProfileIsUsingDefault.
+  CheckProfilePrefsReset(pref_service, !concatenate_enabled_);
   SetProfilePrefs(pref_service);
   // Signout should reset profile prefs.
   identity_test_env_.SetCookieAccounts({});
-  CheckProfilePrefsReset(pref_service);
+  CheckProfilePrefsReset(pref_service, false);
 #endif  // !defined(OS_ANDROID)
 
   SetProfilePrefs(pref_service);
   // Set primary account should reset profile prefs.
   AccountInfo primary_account =
       identity_test_env_.MakePrimaryAccountAvailable("primary@example.com");
-  CheckProfilePrefsReset(pref_service);
+  CheckProfilePrefsReset(pref_service, !concatenate_enabled_);
   SetProfilePrefs(pref_service);
   // Disabling sync should reset profile prefs.
   identity_test_env_.ClearPrimaryAccount();
-  CheckProfilePrefsReset(pref_service);
+  CheckProfilePrefsReset(pref_service, false);
 }
 
 #if !defined(OS_ANDROID)
@@ -188,12 +227,12 @@ TEST_F(SigninProfileAttributesUpdaterTest,
   // Given it is the same account, profile prefs should keep the same state.
   identity_test_env_.SetPrimaryAccount(account_info.email);
   EXPECT_TRUE(entry->IsAuthenticated());
-  CheckProfilePrefsSet(pref_service);
+  CheckProfilePrefsSet(pref_service, false);
   identity_test_env_.ClearPrimaryAccount();
-  CheckProfilePrefsReset(pref_service);
+  CheckProfilePrefsReset(pref_service, false);
 }
 
-TEST_F(SigninProfileAttributesUpdaterTest,
+TEST_P(SigninProfileAttributesUpdaterTestWithParam,
        EnablingSyncWithDifferentAccountThanUPAResetsProfilePrefs) {
   PrefService* pref_service = profile_->GetPrefs();
   ProfileAttributesEntry* entry;
@@ -207,7 +246,7 @@ TEST_F(SigninProfileAttributesUpdaterTest,
   AccountInfo primary_account =
       identity_test_env_.MakePrimaryAccountAvailable("primary@example.com");
   EXPECT_TRUE(entry->IsAuthenticated());
-  CheckProfilePrefsReset(pref_service);
+  CheckProfilePrefsReset(pref_service, !concatenate_enabled_);
 }
 #endif  // !defined(OS_ANDROID)
 

chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc

@@ -376,11 +376,12 @@ void DiceTurnSyncOnHelper::OnProviderUpdatePropagated(
 void DiceTurnSyncOnHelper::CreateNewSignedInProfile() {
   // Create a new profile and have it call back when done so we can start the
   // signin flow.
-  size_t icon_index = g_browser_process->profile_manager()
-                          ->GetProfileAttributesStorage()
-                          .ChooseAvatarIconIndexForNewProfile();
+  ProfileAttributesStorage& storage =
+      g_browser_process->profile_manager()->GetProfileAttributesStorage();
+  size_t icon_index = storage.ChooseAvatarIconIndexForNewProfile();
+
   ProfileManager::CreateMultiProfileAsync(
-      base::UTF8ToUTF16(account_info_.email),
+      storage.ChooseNameForNewProfile(icon_index),
       profiles::GetDefaultAvatarIconUrl(icon_index),
       base::BindRepeating(&DiceTurnSyncOnHelper::OnNewProfileCreated,
                           weak_pointer_factory_.GetWeakPtr()));

chrome/common/pref_names.h

@@ -495,6 +495,9 @@ extern const char kProfileLastUsed[];
 extern const char kProfilesLastActive[];
 extern const char kProfilesNumCreated[];
 extern const char kProfileInfoCache[];
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+extern const char kLegacyProfileNamesMigrated[];
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 extern const char kProfileCreatedByVersion[];
 extern const char kProfilesDeleted[];