Making profile avatars and names sync. This is done by moving these two properties into profile preferences and making them syncable. The profile observes changes to these prefs and pushes them to the profile info cache.

The initialization of these prefs and of the info cache is done in two steps. First, the profile prefs are setup with values generated by the profile info cache. This causes the profile to try to push the changed values to the info cache, but since the profile isn't in the cache, it fails 


BUG=87658
TEST=ProfileManagerTest.InitProfileUserPrefs, Have two machines with chrome, both with multiple profiles and each with one profile signed in to the same account. Change avatar/name on one, and it will change on the other.

Review URL: http://codereview.chromium.org/8883030

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

7 files changed, 201 insertions, 40 deletions

diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 6574cd7..b51fc03 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -214,6 +214,12 @@ void ProfileImpl::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterBooleanPref(prefs::kClearSiteDataOnExit,
                              false,
                              PrefService::SYNCABLE_PREF);
+  prefs->RegisterIntegerPref(prefs::kProfileAvatarIndex,
+                             -1,
+                             PrefService::SYNCABLE_PREF);
+  prefs->RegisterStringPref(prefs::kProfileName,
+                            "",
+                            PrefService::SYNCABLE_PREF);
 #if !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)
   prefs->RegisterIntegerPref(prefs::kLocalProfileId,
                              kInvalidLocalProfileId,
@@ -290,6 +296,8 @@ void ProfileImpl::DoFinalInit() {
   pref_change_registrar_.Add(prefs::kClearSiteDataOnExit, this);
   pref_change_registrar_.Add(prefs::kGoogleServicesUsername, this);
   pref_change_registrar_.Add(prefs::kDefaultZoomLevel, this);
+  pref_change_registrar_.Add(prefs::kProfileAvatarIndex, this);
+  pref_change_registrar_.Add(prefs::kProfileName, this);
 
   // It would be nice to use PathService for fetching this directory, but
   // the cache directory depends on the profile directory, which isn't available
@@ -1323,6 +1331,10 @@ void ProfileImpl::Observe(int type,
         }
       } else if (*pref_name_in == prefs::kGoogleServicesUsername) {
         UpdateProfileUserNameCache();
+      } else if (*pref_name_in == prefs::kProfileAvatarIndex) {
+        UpdateProfileAvatarCache();
+      } else if (*pref_name_in == prefs::kProfileName) {
+        UpdateProfileNameCache();
       } else if (*pref_name_in == prefs::kDefaultZoomLevel) {
           GetHostZoomMap()->set_default_zoom_level(
               prefs->GetDouble(prefs::kDefaultZoomLevel));
@@ -1627,6 +1639,28 @@ void ProfileImpl::UpdateProfileUserNameCache() {
   }
 }
 
+void ProfileImpl::UpdateProfileNameCache() {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  size_t index = cache.GetIndexOfProfileWithPath(GetPath());
+  if (index != std::string::npos) {
+    std::string profile_name =
+        GetPrefs()->GetString(prefs::kProfileName);
+    cache.SetNameOfProfileAtIndex(index, UTF8ToUTF16(profile_name));
+  }
+}
+
+void ProfileImpl::UpdateProfileAvatarCache() {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  size_t index = cache.GetIndexOfProfileWithPath(GetPath());
+  if (index != std::string::npos) {
+    size_t avatar_index =
+        GetPrefs()->GetInteger(prefs::kProfileAvatarIndex);
+    cache.SetAvatarIconOfProfileAtIndex(index, avatar_index);
+  }
+}
+
 // Gets the cache parameters from the command line. If |is_media_context| is
 // set to true then settings for the media context type is what we need,
 // |cache_path| will be set to the user provided path, or will not be touched if
diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h
index f11ceb5..98edb11 100644
--- a/chrome/browser/profiles/profile_impl.h
+++ b/chrome/browser/profiles/profile_impl.h
@@ -190,6 +190,11 @@ class ProfileImpl : public Profile,
 
   void UpdateProfileUserNameCache();
 
+
+  // Updates the ProfileInfoCache with data from this profile.
+  void UpdateProfileNameCache();
+  void UpdateProfileAvatarCache();
+
   void GetCacheParameters(bool is_media_context,
                           FilePath* cache_path,
                           int* max_size);
diff --git a/chrome/browser/profiles/profile_info_cache.cc b/chrome/browser/profiles/profile_info_cache.cc
index 1816a44..073728a 100644
--- a/chrome/browser/profiles/profile_info_cache.cc
+++ b/chrome/browser/profiles/profile_info_cache.cc
@@ -651,17 +651,22 @@ size_t ProfileInfoCache::GetDefaultAvatarIconCount() {
 
 // static
 int ProfileInfoCache::GetDefaultAvatarIconResourceIDAtIndex(size_t index) {
-  DCHECK_LT(index, GetDefaultAvatarIconCount());
+  DCHECK(IsDefaultAvatarIconIndex(index));
   return kDefaultAvatarIconResources[index];
 }
 
 // static
 std::string ProfileInfoCache::GetDefaultAvatarIconUrl(size_t index) {
-  DCHECK_LT(index, kDefaultAvatarIconsCount);
+  DCHECK(IsDefaultAvatarIconIndex(index));
   return StringPrintf("%s%" PRIuS, kDefaultUrlPrefix, index);
 }
 
 // static
+bool ProfileInfoCache::IsDefaultAvatarIconIndex(size_t index) {
+  return index < kDefaultAvatarIconsCount;
+}
+
+// static
 bool ProfileInfoCache::IsDefaultAvatarIconUrl(const std::string& url,
                                               size_t* icon_index) {
   DCHECK(icon_index);
diff --git a/chrome/browser/profiles/profile_info_cache.h b/chrome/browser/profiles/profile_info_cache.h
index 90fcc5d..f58fbdab 100644
--- a/chrome/browser/profiles/profile_info_cache.h
+++ b/chrome/browser/profiles/profile_info_cache.h
@@ -105,6 +105,8 @@ class ProfileInfoCache : public ProfileInfoInterface,
   static int GetDefaultAvatarIconResourceIDAtIndex(size_t index);
   // Returns a URL for the default avatar icon with specified index.
   static std::string GetDefaultAvatarIconUrl(size_t index);
+  // Checks if |index| is a valid avatar icon index
+  static bool IsDefaultAvatarIconIndex(size_t index);
   // Checks if the given URL points to one of the default avatar icons. If it
   // is, returns true and its index through |icon_index|. If not, returns false.
   static bool IsDefaultAvatarIconUrl(const std::string& icon_url,
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index 509c842..004f323 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -57,6 +57,18 @@ std::vector<FilePath>& ProfilesToDelete() {
   return profiles_to_delete;
 }
 
+// Checks if any user prefs for |profile| have default values.
+bool HasAnyDefaultUserPrefs(Profile* profile) {
+  const PrefService::Preference* avatar_index =
+      profile->GetPrefs()->FindPreference(prefs::kProfileAvatarIndex);
+  DCHECK(avatar_index);
+  const PrefService::Preference* profile_name =
+    profile->GetPrefs()->FindPreference(prefs::kProfileName);
+  DCHECK(profile_name);
+  return avatar_index->IsDefaultValue() ||
+         profile_name->IsDefaultValue();
+}
+
 // Simple task to log the size of the current profile.
 class ProfileSizeTask : public Task {
  public:
@@ -455,6 +467,7 @@ void ProfileManager::OnBrowserSetLastActive(const Browser* browser) {
 
 void ProfileManager::DoFinalInit(Profile* profile, bool go_off_the_record) {
   DoFinalInitForServices(profile, go_off_the_record);
+  InitProfileUserPrefs(profile);
   AddProfileToCache(profile);
   DoFinalInitLogging(profile);
 }
@@ -597,16 +610,49 @@ void ProfileManager::AddProfileToCache(Profile* profile) {
   string16 username = UTF8ToUTF16(profile->GetPrefs()->GetString(
       prefs::kGoogleServicesUsername));
 
-  if (profile->GetPath() == GetDefaultProfileDir(cache.GetUserDataDir())) {
-    cache.AddProfileToCache(
-        profile->GetPath(),
-        l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME), username, 0);
-  } else {
-    size_t icon_index = cache.ChooseAvatarIconIndexForNewProfile();
-    cache.AddProfileToCache(profile->GetPath(),
-                            cache.ChooseNameForNewProfile(icon_index),
-                            username,
-                            icon_index);
+  // Profile name and avatar are set by InitProfileUserPrefs and stored in the
+  // profile. Use those values to setup the cache entry.
+  string16 profile_name = UTF8ToUTF16(profile->GetPrefs()->GetString(
+      prefs::kProfileName));
+
+  size_t icon_index = profile->GetPrefs()->GetInteger(
+      prefs::kProfileAvatarIndex);
+
+  cache.AddProfileToCache(profile->GetPath(),
+                          profile_name,
+                          username,
+                          icon_index);
+}
+
+void ProfileManager::InitProfileUserPrefs(Profile* profile) {
+  ProfileInfoCache& cache = GetProfileInfoCache();
+
+  if (profile->GetPath().DirName() != cache.GetUserDataDir())
+    return;
+
+  // Initialize the user preferences (name and avatar) only if the profile
+  // doesn't have default preferenc values for them.
+  if (HasAnyDefaultUserPrefs(profile)) {
+    size_t profile_cache_index =
+        cache.GetIndexOfProfileWithPath(profile->GetPath());
+    // If the cache has an entry for this profile, use the cache data
+    if (profile_cache_index != std::string::npos) {
+      profile->GetPrefs()->SetInteger(prefs::kProfileAvatarIndex,
+          cache.GetAvatarIconIndexOfProfileAtIndex(profile_cache_index));
+      profile->GetPrefs()->SetString(prefs::kProfileName,
+          UTF16ToUTF8(cache.GetNameOfProfileAtIndex(profile_cache_index)));
+    } else if (profile->GetPath() ==
+               GetDefaultProfileDir(cache.GetUserDataDir())) {
+      profile->GetPrefs()->SetInteger(prefs::kProfileAvatarIndex, 0);
+      profile->GetPrefs()->SetString(prefs::kProfileName,
+          l10n_util::GetStringUTF8(IDS_DEFAULT_PROFILE_NAME));
+    } else {
+      size_t icon_index = cache.ChooseAvatarIconIndexForNewProfile();
+      profile->GetPrefs()->SetInteger(prefs::kProfileAvatarIndex, icon_index);
+      profile->GetPrefs()->SetString(
+          prefs::kProfileName,
+          UTF16ToUTF8(cache.ChooseNameForNewProfile(icon_index)));
+    }
   }
 }
 
@@ -697,8 +743,10 @@ ProfileManagerWithoutInit::ProfileManagerWithoutInit(
 void ProfileManager::RegisterTestingProfile(Profile* profile,
                                             bool add_to_cache) {
   RegisterProfile(profile, true);
-  if (add_to_cache)
+  if (add_to_cache){
+    InitProfileUserPrefs(profile);
     AddProfileToCache(profile);
+  }
 }
 
 #if defined(OS_WIN)
diff --git a/chrome/browser/profiles/profile_manager.h b/chrome/browser/profiles/profile_manager.h
index a63f787e..229d702 100644
--- a/chrome/browser/profiles/profile_manager.h
+++ b/chrome/browser/profiles/profile_manager.h
@@ -241,6 +241,10 @@ class ProfileManager : public base::NonThreadSafe,
   // Adds |profile| to the profile info cache if it hasn't been added yet.
   void AddProfileToCache(Profile* profile);
 
+  // Initializes user prefs of |profile|. This includes profile name and
+  // avatar values
+  void InitProfileUserPrefs(Profile* profile);
+
   // For ChromeOS, determines if profile should be otr.
   bool ShouldGoOffTheRecord();
 
diff --git a/chrome/browser/profiles/profile_manager_unittest.cc b/chrome/browser/profiles/profile_manager_unittest.cc
index 0182d90..5f24011 100644
--- a/chrome/browser/profiles/profile_manager_unittest.cc
+++ b/chrome/browser/profiles/profile_manager_unittest.cc
@@ -97,12 +97,14 @@ class ProfileManagerTest : public testing::Test {
   virtual void SetUp() {
     // Create a new temporary directory, and store the path
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    profile_manager_.reset(new testing::ProfileManager(temp_dir_.path()));
+    static_cast<TestingBrowserProcess*>(g_browser_process)->SetProfileManager(
+        new testing::ProfileManager(temp_dir_.path()));
 #if defined(OS_WIN)
     // Force the ProfileInfoCache to be created immediately, so we can
     // remove the shortcut manager for testing.
-    profile_manager_->GetProfileInfoCache();
-    profile_manager_->RemoveProfileShortcutManagerForTesting();
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    profile_manager->GetProfileInfoCache();
+    profile_manager->RemoveProfileShortcutManagerForTesting();
 #endif
 #if defined(OS_CHROMEOS)
   CommandLine *cl = CommandLine::ForCurrentProcess();
@@ -111,7 +113,8 @@ class ProfileManagerTest : public testing::Test {
   }
 
   virtual void TearDown() {
-    profile_manager_.reset();
+    static_cast<TestingBrowserProcess*>(g_browser_process)->SetProfileManager(
+        NULL);
     message_loop_.RunAllPending();
   }
 
@@ -142,30 +145,30 @@ class ProfileManagerTest : public testing::Test {
   IOThread io_thread_;
 
   scoped_ptr<base::SystemMonitor> system_monitor_dummy_;
-
-  // Also will test profile deletion.
-  scoped_ptr<ProfileManager> profile_manager_;
 };
 
 TEST_F(ProfileManagerTest, GetProfile) {
   FilePath dest_path = temp_dir_.path();
   dest_path = dest_path.Append(FILE_PATH_LITERAL("New Profile"));
 
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
   Profile* profile;
 
   // Successfully create a profile.
-  profile = profile_manager_->GetProfile(dest_path);
+  profile = profile_manager->GetProfile(dest_path);
   EXPECT_TRUE(profile);
 
   // The profile already exists when we call GetProfile. Just load it.
-  EXPECT_EQ(profile, profile_manager_->GetProfile(dest_path));
+  EXPECT_EQ(profile, profile_manager->GetProfile(dest_path));
 }
 
 TEST_F(ProfileManagerTest, DefaultProfileDir) {
   FilePath expected_default =
       FilePath().AppendASCII(chrome::kInitialProfile);
-  EXPECT_EQ(expected_default.value(),
-            profile_manager_->GetInitialProfileDir().value());
+  EXPECT_EQ(
+      expected_default.value(),
+      g_browser_process->profile_manager()->GetInitialProfileDir().value());
 }
 
 #if defined(OS_CHROMEOS)
@@ -178,17 +181,18 @@ TEST_F(ProfileManagerTest, LoggedInProfileDir) {
 
   FilePath expected_default =
       FilePath().AppendASCII(chrome::kInitialProfile);
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
   EXPECT_EQ(expected_default.value(),
-            profile_manager_->GetInitialProfileDir().value());
+            profile_manager->GetInitialProfileDir().value());
 
-  profile_manager_->Observe(chrome::NOTIFICATION_LOGIN_USER_CHANGED,
+  profile_manager->Observe(chrome::NOTIFICATION_LOGIN_USER_CHANGED,
                            content::NotificationService::AllSources(),
                            content::NotificationService::NoDetails());
   FilePath expected_logged_in(profile_dir);
   EXPECT_EQ(expected_logged_in.value(),
-            profile_manager_->GetInitialProfileDir().value());
+            profile_manager->GetInitialProfileDir().value());
   VLOG(1) << temp_dir_.path().Append(
-      profile_manager_->GetInitialProfileDir()).value();
+      profile_manager->GetInitialProfileDir()).value();
 }
 
 #endif
@@ -200,13 +204,15 @@ TEST_F(ProfileManagerTest, CreateAndUseTwoProfiles) {
   FilePath dest_path2 = temp_dir_.path();
   dest_path2 = dest_path2.Append(FILE_PATH_LITERAL("New Profile 2"));
 
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
   // Successfully create the profiles.
   TestingProfile* profile1 =
-      static_cast<TestingProfile*>(profile_manager_->GetProfile(dest_path1));
+      static_cast<TestingProfile*>(profile_manager->GetProfile(dest_path1));
   ASSERT_TRUE(profile1);
 
   TestingProfile* profile2 =
-      static_cast<TestingProfile*>(profile_manager_->GetProfile(dest_path2));
+      static_cast<TestingProfile*>(profile_manager->GetProfile(dest_path2));
   ASSERT_TRUE(profile2);
 
   // Force lazy-init of some profile services to simulate use.
@@ -222,7 +228,8 @@ TEST_F(ProfileManagerTest, CreateAndUseTwoProfiles) {
   // Make sure any pending tasks run before we destroy the profiles.
   message_loop_.RunAllPending();
 
-  profile_manager_.reset();
+  static_cast<TestingBrowserProcess*>(g_browser_process)->SetProfileManager(
+      NULL);
 
   // Make sure history cleans up correctly.
   message_loop_.RunAllPending();
@@ -242,7 +249,7 @@ TEST_F(ProfileManagerTest, DISABLED_CreateProfileAsync) {
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(1));
 
-  profile_manager_->CreateProfileAsync(dest_path,
+  g_browser_process->profile_manager()->CreateProfileAsync(dest_path,
       base::Bind(&MockObserver::OnProfileCreated,
                  base::Unretained(&mock_observer)));
 
@@ -271,13 +278,15 @@ TEST_F(ProfileManagerTest, CreateProfileAsyncMultipleRequests) {
   EXPECT_CALL(mock_observer3, OnProfileCreated(
       SameNotNull(), NotFail())).Times(testing::AtLeast(1));
 
-  profile_manager_->CreateProfileAsync(dest_path,
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
+  profile_manager->CreateProfileAsync(dest_path,
       base::Bind(&MockObserver::OnProfileCreated,
                  base::Unretained(&mock_observer1)));
-  profile_manager_->CreateProfileAsync(dest_path,
+  profile_manager->CreateProfileAsync(dest_path,
       base::Bind(&MockObserver::OnProfileCreated,
                  base::Unretained(&mock_observer2)));
-  profile_manager_->CreateProfileAsync(dest_path,
+  profile_manager->CreateProfileAsync(dest_path,
       base::Bind(&MockObserver::OnProfileCreated,
                  base::Unretained(&mock_observer3)));
 
@@ -294,10 +303,12 @@ TEST_F(ProfileManagerTest, CreateProfilesAsync) {
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(3));
 
-  profile_manager_->CreateProfileAsync(dest_path1,
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
+  profile_manager->CreateProfileAsync(dest_path1,
       base::Bind(&MockObserver::OnProfileCreated,
                  base::Unretained(&mock_observer)));
-  profile_manager_->CreateProfileAsync(dest_path2,
+  profile_manager->CreateProfileAsync(dest_path2,
       base::Bind(&MockObserver::OnProfileCreated,
                  base::Unretained(&mock_observer)));
 
@@ -305,7 +316,8 @@ TEST_F(ProfileManagerTest, CreateProfilesAsync) {
 }
 
 TEST_F(ProfileManagerTest, AutoloadProfilesWithBackgroundApps) {
-  ProfileInfoCache& cache = profile_manager_->GetProfileInfoCache();
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
 
   EXPECT_EQ(0u, cache.GetNumberOfProfiles());
   cache.AddProfileToCache(cache.GetUserDataDir().AppendASCII("path_1"),
@@ -318,7 +330,58 @@ TEST_F(ProfileManagerTest, AutoloadProfilesWithBackgroundApps) {
   cache.SetBackgroundStatusOfProfileAtIndex(2, true);
   EXPECT_EQ(3u, cache.GetNumberOfProfiles());
 
-  profile_manager_->AutoloadProfiles();
+  profile_manager->AutoloadProfiles();
+
+  EXPECT_EQ(2u, profile_manager->GetLoadedProfiles().size());
+}
+
+TEST_F(ProfileManagerTest, InitProfileUserPrefs) {
+  FilePath dest_path = temp_dir_.path();
+  dest_path = dest_path.Append(FILE_PATH_LITERAL("New Profile"));
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
+  Profile* profile;
+
+  // Successfully create the profile
+  profile = profile_manager->GetProfile(dest_path);
+  ASSERT_TRUE(profile);
+
+  // Check that the profile name is non empty
+  std::string profile_name =
+      profile->GetPrefs()->GetString(prefs::kProfileName);
+  EXPECT_FALSE(profile_name.empty());
+
+  // Check that the profile avatar index is valid
+  size_t avatar_index =
+      profile->GetPrefs()->GetInteger(prefs::kProfileAvatarIndex);
+  EXPECT_TRUE(profile_manager->GetProfileInfoCache().IsDefaultAvatarIconIndex(
+      avatar_index));
+}
+
+// Tests that a new profile's entry in the profile info cache is setup with the
+// same values that are in the profile prefs.
+TEST_F(ProfileManagerTest, InitProfileInfoCacheForAProfile) {
+  FilePath dest_path = temp_dir_.path();
+  dest_path = dest_path.Append(FILE_PATH_LITERAL("New Profile"));
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+
+  // Successfully create the profile
+  Profile* profile = profile_manager->GetProfile(dest_path);
+  ASSERT_TRUE(profile);
+
+  std::string profile_name =
+      profile->GetPrefs()->GetString(prefs::kProfileName);
+  size_t avatar_index =
+      profile->GetPrefs()->GetInteger(prefs::kProfileAvatarIndex);
+
+  size_t profile_index = cache.GetIndexOfProfileWithPath(dest_path);
 
-  EXPECT_EQ(2u, profile_manager_->GetLoadedProfiles().size());
+  // Check if the profile prefs are the same as the cache prefs
+  EXPECT_EQ(profile_name,
+            UTF16ToUTF8(cache.GetNameOfProfileAtIndex(profile_index)));
+  EXPECT_EQ(avatar_index,
+            cache.GetAvatarIconIndexOfProfileAtIndex(profile_index));
 }