[Signin]: Remove sorting profile keys in the ProfileInfoCache.

Remove unecessary sorting for |sorted_keys_| as there is no need for
the list of keys to be sorted. We use
|ProfileAttributesStorage::GetAllProfilesAttributesSortedByName| to get
the profiles sorted by name.

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

chrome/browser/profiles/gaia_info_update_service_unittest.cc

@@ -294,48 +294,6 @@ TEST_F(GAIAInfoUpdateServiceTest, ProfileLockEnabledForWhitelist) {
       GetString(prefs::kGoogleServicesHostedDomain));
 }
 
-// TODO(anthonyvd) : remove or update test once the refactoring of the internals
-// of ProfileInfoCache is complete.
-TEST_F(GAIAInfoUpdateServiceTest, HandlesProfileReordering) {
-  size_t index = GetCache()->GetIndexOfProfileWithPath(profile()->GetPath());
-  GetCache()->SetLocalProfileNameOfProfileAtIndex(index, FullName16("B"));
-  GetCache()->SetProfileIsUsingDefaultNameAtIndex(index, true);
-
-  CreateProfile(FullName("A"));
-  CreateProfile(FullName("C"));
-  CreateProfile(FullName("E"));
-
-  size_t index_before =
-      GetCache()->GetIndexOfProfileWithPath(profile()->GetPath());
-
-  // Rename our profile.
-  RenameProfile(FullName16("D"), GivenName16("D"));
-  // Profiles should have been reordered in the cache.
-  EXPECT_NE(index_before,
-            GetCache()->GetIndexOfProfileWithPath(profile()->GetPath()));
-  // Rename the profile back to the original name, it should go back to its
-  // original position.
-  RenameProfile(FullName16("B"), GivenName16("B"));
-  EXPECT_EQ(index_before,
-            GetCache()->GetIndexOfProfileWithPath(profile()->GetPath()));
-
-  // Rename only the given name of our profile.
-  RenameProfile(FullName16("B"), GivenName16("D"));
-  // Rename the profile back to the original name, it should go back to its
-  // original position.
-  RenameProfile(FullName16("B"), GivenName16("B"));
-  EXPECT_EQ(index_before,
-            GetCache()->GetIndexOfProfileWithPath(profile()->GetPath()));
-
-  // Rename only the full name of our profile.
-  RenameProfile(FullName16("D"), GivenName16("B"));
-  // Rename the profile back to the original name, it should go back to its
-  // original position.
-  RenameProfile(FullName16("B"), GivenName16("B"));
-  EXPECT_EQ(index_before,
-            GetCache()->GetIndexOfProfileWithPath(profile()->GetPath()));
-}
-
 TEST_F(GAIAInfoUpdateServiceTest, ShouldUseGAIAProfileInfo) {
 #if defined(OS_CHROMEOS)
   // This feature should never be enabled on ChromeOS.

chrome/browser/profiles/profile_info_cache.cc

@@ -83,7 +83,7 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
 #endif
     base::string16 name;
     info->GetString(kNameKey, &name);
-    sorted_keys_.insert(FindPositionForProfile(it.key(), name), it.key());
+    keys_.push_back(it.key());
     profile_attributes_entries_[user_data_dir_.AppendASCII(it.key()).value()] =
         std::unique_ptr<ProfileAttributesEntry>(nullptr);
 
@@ -167,7 +167,7 @@ void ProfileInfoCache::AddProfileToCache(const base::FilePath& profile_path,
     old_single_profile_name = entry->GetName();
   }
 
-  sorted_keys_.insert(FindPositionForProfile(key, name), key);
+  keys_.push_back(key);
   profile_attributes_entries_[user_data_dir_.AppendASCII(key).value()] =
       std::unique_ptr<ProfileAttributesEntry>();
 
@@ -219,7 +219,7 @@ void ProfileInfoCache::DeleteProfileFromCache(
   base::DictionaryValue* cache = update.Get();
   std::string key = CacheKeyFromProfilePath(profile_path);
   cache->Remove(key, NULL);
-  sorted_keys_.erase(std::find(sorted_keys_.begin(), sorted_keys_.end(), key));
+  keys_.erase(std::find(keys_.begin(), keys_.end(), key));
   profile_attributes_entries_.erase(profile_path.value());
 
   bool single_profile_name_changed =
@@ -236,7 +236,7 @@ void ProfileInfoCache::DeleteProfileFromCache(
 }
 
 size_t ProfileInfoCache::GetNumberOfProfiles() const {
-  return sorted_keys_.size();
+  return keys_.size();
 }
 
 size_t ProfileInfoCache::GetIndexOfProfileWithPath(
@@ -244,8 +244,8 @@ size_t ProfileInfoCache::GetIndexOfProfileWithPath(
   if (profile_path.DirName() != user_data_dir_)
     return std::string::npos;
   std::string search_key = CacheKeyFromProfilePath(profile_path);
-  for (size_t i = 0; i < sorted_keys_.size(); ++i) {
-    if (sorted_keys_[i] == search_key)
+  for (size_t i = 0; i < keys_.size(); ++i) {
+    if (keys_[i] == search_key)
       return i;
   }
   return std::string::npos;
@@ -279,7 +279,7 @@ base::string16 ProfileInfoCache::GetNameOfProfileAtIndex(size_t index) const {
 }
 
 base::FilePath ProfileInfoCache::GetPathOfProfileAtIndex(size_t index) const {
-  return user_data_dir_.AppendASCII(sorted_keys_[index]);
+  return user_data_dir_.AppendASCII(keys_[index]);
 }
 
 base::string16 ProfileInfoCache::GetUserNameOfProfileAtIndex(
@@ -455,7 +455,6 @@ void ProfileInfoCache::SetLocalProfileNameOfProfileAtIndex(
 
   base::string16 new_display_name = GetNameToDisplayOfProfileAtIndex(index);
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
-  UpdateSortForProfileIndex(index);
 
   if (old_display_name != new_display_name) {
     for (auto& observer : observer_list_)
@@ -572,7 +571,6 @@ void ProfileInfoCache::SetGAIANameOfProfileAtIndex(size_t index,
   SetInfoForProfileAtIndex(index, std::move(info));
   base::string16 new_display_name = GetNameToDisplayOfProfileAtIndex(index);
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
-  UpdateSortForProfileIndex(index);
 
   if (old_display_name != new_display_name) {
     for (auto& observer : observer_list_)
@@ -593,7 +591,6 @@ void ProfileInfoCache::SetGAIAGivenNameOfProfileAtIndex(
   SetInfoForProfileAtIndex(index, std::move(info));
   base::string16 new_display_name = GetNameToDisplayOfProfileAtIndex(index);
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
-  UpdateSortForProfileIndex(index);
 
   if (old_display_name != new_display_name) {
     for (auto& observer : observer_list_)
@@ -723,7 +720,7 @@ const base::DictionaryValue* ProfileInfoCache::GetInfoForProfileAtIndex(
   const base::DictionaryValue* cache =
       prefs_->GetDictionary(prefs::kProfileInfoCache);
   const base::DictionaryValue* info = NULL;
-  cache->GetDictionaryWithoutPathExpansion(sorted_keys_[index], &info);
+  cache->GetDictionaryWithoutPathExpansion(keys_[index], &info);
   return info;
 }
 
@@ -732,7 +729,7 @@ void ProfileInfoCache::SetInfoForProfileAtIndex(
     std::unique_ptr<base::DictionaryValue> info) {
   DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache);
   base::DictionaryValue* cache = update.Get();
-  cache->SetWithoutPathExpansion(sorted_keys_[index], std::move(info));
+  cache->SetWithoutPathExpansion(keys_[index], std::move(info));
 }
 
 std::string ProfileInfoCache::CacheKeyFromProfilePath(
@@ -742,36 +739,6 @@ std::string ProfileInfoCache::CacheKeyFromProfilePath(
   return base_name.MaybeAsASCII();
 }
 
-std::vector<std::string>::iterator ProfileInfoCache::FindPositionForProfile(
-    const std::string& search_key,
-    const base::string16& search_name) {
-  base::string16 search_name_l = base::i18n::ToLower(search_name);
-  for (size_t i = 0; i < GetNumberOfProfiles(); ++i) {
-    base::string16 name_l =
-        base::i18n::ToLower(GetNameToDisplayOfProfileAtIndex(i));
-    int name_compare = search_name_l.compare(name_l);
-    if (name_compare < 0)
-      return sorted_keys_.begin() + i;
-    if (name_compare == 0) {
-      int key_compare = search_key.compare(sorted_keys_[i]);
-      if (key_compare < 0)
-        return sorted_keys_.begin() + i;
-    }
-  }
-  return sorted_keys_.end();
-}
-
-void ProfileInfoCache::UpdateSortForProfileIndex(size_t index) {
-  base::string16 name = GetNameToDisplayOfProfileAtIndex(index);
-
-  // Remove and reinsert key in |sorted_keys_| to alphasort.
-  std::string key = CacheKeyFromProfilePath(GetPathOfProfileAtIndex(index));
-  auto key_it = std::find(sorted_keys_.begin(), sorted_keys_.end(), key);
-  DCHECK(key_it != sorted_keys_.end());
-  sorted_keys_.erase(key_it);
-  sorted_keys_.insert(FindPositionForProfile(key, name), key);
-}
-
 const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
     size_t index) const {
   const size_t avatar_index = GetAvatarIconIndexOfProfileAtIndex(index);
@@ -806,7 +773,6 @@ void ProfileInfoCache::RecomputeProfileNamesIfNeeded() {
       if (name == entries[j]->GetLocalProfileName()) {
         entries[j]->SetLocalProfileName(
             ChooseNameForNewProfile(entries[j]->GetAvatarIconIndex()));
-        UpdateSortForProfileIndex(entries[j]->profile_index());
       }
     }
   }

chrome/browser/profiles/profile_info_cache.h

@@ -197,7 +197,7 @@ class ProfileInfoCache : public ProfileInfoInterface,
   // recomputed to "Person 1" and "Person 2".
   void RecomputeProfileNamesIfNeeded();
 
-  std::vector<std::string> sorted_keys_;
+  std::vector<std::string> keys_;
   const base::FilePath user_data_dir_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileInfoCache);

chrome/browser/profiles/profile_info_cache_unittest.cc

@@ -281,12 +281,9 @@ TEST_P(ProfileInfoCacheTestWithParam, GAIAName) {
   EXPECT_TRUE(GetCache()->GetGAIANameOfProfileAtIndex(index1).empty());
   EXPECT_TRUE(GetCache()->GetGAIANameOfProfileAtIndex(index2).empty());
 
-  // Set GAIA name. This re-sorts the cache.
+  // Set GAIA name.
   base::string16 gaia_name(ASCIIToUTF16("Pat Smith"));
   GetCache()->SetGAIANameOfProfileAtIndex(index2, gaia_name);
-  index1 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_1"));
-  index2 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_2"));
-
   // Since there is a GAIA name, we use that as a display name.
   EXPECT_TRUE(GetCache()->GetGAIANameOfProfileAtIndex(index1).empty());
   EXPECT_EQ(gaia_name, GetCache()->GetGAIANameOfProfileAtIndex(index2));
@@ -294,14 +291,10 @@ TEST_P(ProfileInfoCacheTestWithParam, GAIAName) {
                                      concatenate_enabled_, false),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
 
-  // This re-sorts the cache.
   base::string16 custom_name(ASCIIToUTF16("Custom name"));
   GetCache()->SetLocalProfileNameOfProfileAtIndex(index2, custom_name);
   GetCache()->SetProfileIsUsingDefaultNameAtIndex(index2, false);
 
-  index1 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_1"));
-  index2 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_2"));
-
   EXPECT_EQ(GetExpectedNameToDisplay(gaia_name, custom_name, false,
                                      concatenate_enabled_, false),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
@@ -331,7 +324,6 @@ TEST_F(ProfileInfoCacheTest, ConcatenateGaiaNameAndProfileName) {
       GetProfilePath("path_2"), ASCIIToUTF16("Person 2"), std::string(),
       base::string16(), false, 0, std::string(), EmptyAccountId());
 
-  index1 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_1"));
   EXPECT_EQ(ASCIIToUTF16("Patt (Person 1)"),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index1));
 
@@ -340,16 +332,13 @@ TEST_F(ProfileInfoCacheTest, ConcatenateGaiaNameAndProfileName) {
             GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
   // Set Gaia name.
   GetCache()->SetGAIANameOfProfileAtIndex(index2, ASCIIToUTF16("Patti Smith"));
-  index2 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_2"));
   // Profile name is a substring of Gaia name.
   GetCache()->SetLocalProfileNameOfProfileAtIndex(index2,
                                                   ASCIIToUTF16("patti"));
-  index2 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_2"));
   EXPECT_EQ(ASCIIToUTF16("Patti Smith"),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
   // Profile name equals Gaia given name.
   GetCache()->SetGAIAGivenNameOfProfileAtIndex(index2, ASCIIToUTF16("Patti"));
-  index2 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_2"));
   EXPECT_EQ(ASCIIToUTF16("Patti"),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index2));
   GetCache()->SetLocalProfileNameOfProfileAtIndex(index2, ASCIIToUTF16("Work"));
@@ -362,12 +351,10 @@ TEST_F(ProfileInfoCacheTest, ConcatenateGaiaNameAndProfileName) {
                                 std::string(), EmptyAccountId());
   int index3 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_3"));
   GetCache()->SetGAIAGivenNameOfProfileAtIndex(index3, ASCIIToUTF16("Pat"));
-  index3 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_3"));
   EXPECT_EQ(ASCIIToUTF16("Pat"),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index3));
   GetCache()->SetGAIANameOfProfileAtIndex(index3, ASCIIToUTF16("Pat Smith"));
   GetCache()->SetGAIAGivenNameOfProfileAtIndex(index3, ASCIIToUTF16(""));
-  index3 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_3"));
   EXPECT_EQ(ASCIIToUTF16("Pat Smith"),
             GetCache()->GetNameToDisplayOfProfileAtIndex(index3));
 
@@ -438,49 +425,6 @@ TEST_F(ProfileInfoCacheTest, MutateProfile) {
 #endif
 }
 
-TEST_F(ProfileInfoCacheTest, Sort) {
-  base::string16 name_a = ASCIIToUTF16("apple");
-  GetCache()->AddProfileToCache(GetProfilePath("path_a"), name_a, std::string(),
-                                base::string16(), false, 0, std::string(),
-                                EmptyAccountId());
-
-  base::string16 name_c = ASCIIToUTF16("cat");
-  GetCache()->AddProfileToCache(GetProfilePath("path_c"), name_c, std::string(),
-                                base::string16(), false, 0, std::string(),
-                                EmptyAccountId());
-
-  // Sanity check the initial order.
-  EXPECT_EQ(name_a, GetCache()->GetNameToDisplayOfProfileAtIndex(0));
-  EXPECT_EQ(name_c, GetCache()->GetNameToDisplayOfProfileAtIndex(1));
-
-  // Add a new profile (start with a capital to test case insensitive sorting.
-  base::string16 name_b = ASCIIToUTF16("Banana");
-  GetCache()->AddProfileToCache(GetProfilePath("path_b"), name_b, std::string(),
-                                base::string16(), false, 0, std::string(),
-                                EmptyAccountId());
-
-  // Verify the new order.
-  EXPECT_EQ(name_a, GetCache()->GetNameToDisplayOfProfileAtIndex(0));
-  EXPECT_EQ(name_b, GetCache()->GetNameToDisplayOfProfileAtIndex(1));
-  EXPECT_EQ(name_c, GetCache()->GetNameToDisplayOfProfileAtIndex(2));
-
-  // Change the name of an existing profile.
-  name_a = UTF8ToUTF16("dog");
-  GetCache()->SetLocalProfileNameOfProfileAtIndex(0, name_a);
-
-  // Verify the new order.
-  EXPECT_EQ(name_b, GetCache()->GetNameToDisplayOfProfileAtIndex(0));
-  EXPECT_EQ(name_c, GetCache()->GetNameToDisplayOfProfileAtIndex(1));
-  EXPECT_EQ(name_a, GetCache()->GetNameToDisplayOfProfileAtIndex(2));
-
-  // Delete a profile.
-  GetCache()->DeleteProfileFromCache(GetProfilePath("path_c"));
-
-  // Verify the new order.
-  EXPECT_EQ(name_b, GetCache()->GetNameToDisplayOfProfileAtIndex(0));
-  EXPECT_EQ(name_a, GetCache()->GetNameToDisplayOfProfileAtIndex(1));
-}
-
 // Will be removed SOON with ProfileInfoCache tests.
 TEST_F(ProfileInfoCacheTest, BackgroundModeStatus) {
   GetCache()->AddProfileToCache(