Skip to content

Commit

Permalink
[profiles] Fix incorrect last_name_to_display_ on startup
Browse files Browse the repository at this point in the history
last_name_to_display_ is initialized on startup in
ProfileAttributesEntry::Initialize() when the profile info cache is read
from the local state on disk. ProfileInfoCache initializes each profile
entry one by one. However, profile's visible name depends on other
profiles names because a disambiguation might be required.

last_name_to_display_ has to be updated once all profiles are loaded
to cache. This CL achieves that by calling
ProfileAttributesEntry::HasProfileNameChanged() on each profile entry
after all of them were loaded.

(cherry picked from commit 95d596d)

Bug: 1195784
Change-Id: I60bbff69e1cd6776ab8d6c465a00716a40f77ed0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2814923
Reviewed-by: Monica Basta <msalama@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#871427}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824148
Auto-Submit: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Monica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/branch-heads/4472@{#55}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}
  • Loading branch information
Alex Ilin authored and Chromium LUCI CQ committed Apr 14, 2021
1 parent c6bf910 commit 5b113ba
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 3 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/profiles/profile_attributes_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ void ProfileAttributesEntry::Initialize(ProfileInfoCache* cache,
SetIsSigninRequired(false);
#endif
}
}

void ProfileAttributesEntry::InitializeLastNameToDisplay() {
DCHECK(last_name_to_display_.empty());
last_name_to_display_ = GetName();
}
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/profiles/profile_attributes_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,16 @@ class ProfileAttributesEntry {
FRIEND_TEST_ALL_PREFIXES(ProfileAttributesStorageTest,
DownloadHighResAvatarTest);

// Initializes the current entry instance. The callers must subsequently call
// InitializeLastNameToDisplay() for this entry.
void Initialize(ProfileInfoCache* cache,
const base::FilePath& path,
PrefService* prefs);

// Sets the initial name of the profile to be displayed. The name might depend
// on other's profiles names so this must be called only after all profiles
// has been initialized.
void InitializeLastNameToDisplay();
std::u16string GetLastNameToDisplay() const;

// Returns true if:
Expand Down
16 changes: 14 additions & 2 deletions chrome/browser/profiles/profile_info_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_avatar_icon_util.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/common/buildflags.h"
Expand Down Expand Up @@ -94,6 +95,13 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
InitEntryWithKey(it.key());
}

// A profile name can depend on other profile names. Do an additional pass to
// update last used profile names once all profiles are initialized.
for (ProfileAttributesEntry* entry :
GetAllProfilesAttributes(/*include_guest_profile=*/true)) {
entry->InitializeLastNameToDisplay();
}

// If needed, start downloading the high-res avatars and migrate any legacy
// profile names.
if (!disable_avatar_download_for_testing_)
Expand Down Expand Up @@ -160,7 +168,8 @@ void ProfileInfoCache::AddProfileToCache(const base::FilePath& profile_path,
if (account_id.HasAccountIdKey())
info->SetString(kAccountIdKey, account_id.GetAccountIdKey());
cache->SetWithoutPathExpansion(key, std::move(info));
InitEntryWithKey(key);
ProfileAttributesEntry* entry = InitEntryWithKey(key);
entry->InitializeLastNameToDisplay();

// `OnProfileAdded()` must be the first observer method being called right
// after a new profile is added to cache.
Expand Down Expand Up @@ -553,16 +562,19 @@ void ProfileInfoCache::LoadGAIAPictureIfNeeded() {
}
#endif

void ProfileInfoCache::InitEntryWithKey(const std::string& key) {
ProfileAttributesEntry* ProfileInfoCache::InitEntryWithKey(
const std::string& key) {
// TODO(https://crbug.com/1195784): revert CHECKs back to DCHECKs after the
// crash is investigated.
CHECK(!base::Contains(keys_, key));
keys_.push_back(key);
base::FilePath path = user_data_dir_.AppendASCII(key);
CHECK(!base::Contains(profile_attributes_entries_, path.value()));
auto new_entry = std::make_unique<ProfileAttributesEntry>();
auto* new_entry_raw = new_entry.get();
new_entry->Initialize(this, path, prefs_);
profile_attributes_entries_[path.value()] = std::move(new_entry);
return new_entry_raw;
}

#if !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/profiles/profile_info_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class ProfileInfoCache : public ProfileInfoInterface,
void LoadGAIAPictureIfNeeded();
#endif

void InitEntryWithKey(const std::string& key);
ProfileAttributesEntry* InitEntryWithKey(const std::string& key);

#if !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
// Migrate any legacy profile names ("First user", "Default Profile") to
Expand Down
45 changes: 45 additions & 0 deletions chrome/browser/profiles/profile_info_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <algorithm>
#include <set>
#include <string>
#include <vector>

#include "base/bind.h"
Expand Down Expand Up @@ -1007,3 +1008,47 @@ TEST_F(ProfileInfoCacheTest, ModifiyEntryWhileInitializing) {
EXPECT_FALSE(entry->IsSigninRequired());
}
#endif

TEST_F(ProfileInfoCacheTest, ProfileNamesOnInit) {
// Set up the cache with two profiles having the same GAIA given name.
// The second profile also has a profile name matching the GAIA given name.
std::u16string kDefaultProfileName = u"Person 1";
std::u16string kCommonName = u"Joe";

// Create and initialize the first profile.
base::FilePath path_1 = GetProfilePath("path_1");
GetCache()->AddProfileToCache(path_1, kDefaultProfileName, std::string(),
std::u16string(), false, 0, std::string(),
EmptyAccountId());
ProfileAttributesEntry* entry_1 =
GetCache()->GetProfileAttributesWithPath(path_1);
entry_1->SetGAIAGivenName(kCommonName);
EXPECT_EQ(entry_1->GetName(), kCommonName);

// Create and initialize the second profile.
base::FilePath path_2 = GetProfilePath("path_2");
GetCache()->AddProfileToCache(path_2, kCommonName, std::string(),
std::u16string(), false, 0, std::string(),
EmptyAccountId());
ProfileAttributesEntry* entry_2 =
GetCache()->GetProfileAttributesWithPath(path_2);
entry_2->SetGAIAGivenName(kCommonName);
EXPECT_EQ(entry_2->GetName(), kCommonName);

// The first profile name should be modified.
EXPECT_EQ(entry_1->GetName(),
GetConcatenation(kCommonName, kDefaultProfileName));

// Reset cache to test profile names set on initialization.
ResetCache();
entry_1 = GetCache()->GetProfileAttributesWithPath(path_1);
entry_2 = GetCache()->GetProfileAttributesWithPath(path_2);

// Freshly initialized entries should not report name changes.
EXPECT_FALSE(entry_1->HasProfileNameChanged());
EXPECT_FALSE(entry_2->HasProfileNameChanged());

EXPECT_EQ(entry_1->GetName(),
GetConcatenation(kCommonName, kDefaultProfileName));
EXPECT_EQ(entry_2->GetName(), kCommonName);
}

0 comments on commit 5b113ba

Please sign in to comment.