Commit 7153bc0f authored by Monica Basta's avatar Monica Basta Committed by Commit Bot

[Signin]: Recompute local default profile names.

On startup,check if two or more profiles has the same default profile
name, e.g: "Person 1", for these profiles recompute the local profile
name in a way that "Person n" is unique.

This can happen because the profile name was a syncable pref so having
two profiles with the same local profile name "Person n" was possible.

Bug: 1012182
Change-Id: Ic19352c4fdd15717b7d88c53e3e769eb3dfaa8bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864781
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706838}
parent 7147e129
...@@ -106,6 +106,8 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs, ...@@ -106,6 +106,8 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
// profile names. // profile names.
if (!disable_avatar_download_for_testing_) if (!disable_avatar_download_for_testing_)
MigrateLegacyProfileNamesAndDownloadAvatars(); MigrateLegacyProfileNamesAndDownloadAvatars();
RecomputeProfileNamesIfNeeded();
} }
ProfileInfoCache::~ProfileInfoCache() { ProfileInfoCache::~ProfileInfoCache() {
...@@ -789,6 +791,28 @@ const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex( ...@@ -789,6 +791,28 @@ const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
image_path); image_path);
} }
void ProfileInfoCache::RecomputeProfileNamesIfNeeded() {
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
std::vector<ProfileAttributesEntry*> entries = GetAllProfilesAttributes();
if (entries.size() < 2)
return;
for (size_t i = 0; i < entries.size() - 1; i++) {
base::string16 name = entries[i]->GetLocalProfileName();
if (!IsDefaultProfileName(name))
continue;
for (size_t j = i + 1; j < entries.size(); j++) {
if (name == entries[j]->GetLocalProfileName()) {
entries[j]->SetLocalProfileName(
ChooseNameForNewProfile(entries[j]->GetAvatarIconIndex()));
UpdateSortForProfileIndex(entries[j]->profile_index());
}
}
}
#endif
}
void ProfileInfoCache::MigrateLegacyProfileNamesAndDownloadAvatars() { void ProfileInfoCache::MigrateLegacyProfileNamesAndDownloadAvatars() {
// Only do this on desktop platforms. // Only do this on desktop platforms.
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
......
...@@ -192,6 +192,11 @@ class ProfileInfoCache : public ProfileInfoInterface, ...@@ -192,6 +192,11 @@ class ProfileInfoCache : public ProfileInfoInterface,
// used by the profiles. // used by the profiles.
void MigrateLegacyProfileNamesAndDownloadAvatars(); void MigrateLegacyProfileNamesAndDownloadAvatars();
// 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
// recomputed to "Person 1" and "Person 2".
void RecomputeProfileNamesIfNeeded();
std::vector<std::string> sorted_keys_; std::vector<std::string> sorted_keys_;
const base::FilePath user_data_dir_; const base::FilePath user_data_dir_;
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <stddef.h> #include <stddef.h>
#include <stdint.h> #include <stdint.h>
#include <algorithm>
#include <vector> #include <vector>
#include "base/bind.h" #include "base/bind.h"
...@@ -110,6 +111,11 @@ void ProfileInfoCacheTest::SetUp() { ...@@ -110,6 +111,11 @@ void ProfileInfoCacheTest::SetUp() {
testing_profile_manager_.profile_info_cache()->AddObserver(&name_observer_); testing_profile_manager_.profile_info_cache()->AddObserver(&name_observer_);
} }
void ProfileInfoCacheTest::RemoveObserver() {
testing_profile_manager_.profile_info_cache()->RemoveObserver(
&name_observer_);
}
void ProfileInfoCacheTest::TearDown() { void ProfileInfoCacheTest::TearDown() {
// Drain remaining tasks to make sure all tasks are completed. This prevents // Drain remaining tasks to make sure all tasks are completed. This prevents
// memory leaks. // memory leaks.
...@@ -798,6 +804,58 @@ TEST_F(ProfileInfoCacheTest, EntriesInAttributesStorage) { ...@@ -798,6 +804,58 @@ TEST_F(ProfileInfoCacheTest, EntriesInAttributesStorage) {
} }
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
TEST_F(ProfileInfoCacheTest, RecomputeProfileNamesIfNeeded) {
// Duplicate profile names causes the observer to crash.
RemoveObserver();
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, MigrateLegacyProfileNamesWithNewAvatarMenu) {
EXPECT_EQ(0U, GetCache()->GetNumberOfProfiles()); EXPECT_EQ(0U, GetCache()->GetNumberOfProfiles());
......
...@@ -54,6 +54,7 @@ class ProfileInfoCacheTest : public testing::Test { ...@@ -54,6 +54,7 @@ class ProfileInfoCacheTest : public testing::Test {
ProfileInfoCache* GetCache(); ProfileInfoCache* GetCache();
base::FilePath GetProfilePath(const std::string& base_name); base::FilePath GetProfilePath(const std::string& base_name);
void ResetCache(); void ResetCache();
void RemoveObserver();
private: private:
// BrowserTaskEnvironment needs to be up through the destruction of the // BrowserTaskEnvironment needs to be up through the destruction of the
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment