Lands http://codereview.chromium.org/8801034/ in hopes of getting a

green tree. TBRing since this is a revert.

R=sail@chromium.org, jwd@chromium.org
BUG=87658
TEST=none

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

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

chrome/browser/profiles/profile_impl.cc

@@ -209,12 +209,6 @@ 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,8 +284,6 @@ 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
@@ -1313,10 +1305,6 @@ 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));
@@ -1619,28 +1607,6 @@ 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

chrome/browser/profiles/profile_impl.h

@@ -191,11 +191,6 @@ 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);

chrome/browser/profiles/profile_info_cache.cc

@@ -651,21 +651,16 @@ size_t ProfileInfoCache::GetDefaultAvatarIconCount() {
 
 // static
 int ProfileInfoCache::GetDefaultAvatarIconResourceIDAtIndex(size_t index) {
-  DCHECK(IsDefaultAvatarIconIndex(index));
+  DCHECK_LT(index, GetDefaultAvatarIconCount());
   return kDefaultAvatarIconResources[index];
 }
 
 // static
 std::string ProfileInfoCache::GetDefaultAvatarIconUrl(size_t index) {
-  DCHECK(IsDefaultAvatarIconIndex(index));
+  DCHECK_LT(index, kDefaultAvatarIconsCount);
   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) {

chrome/browser/profiles/profile_info_cache.h

@@ -105,8 +105,6 @@ 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,

chrome/browser/profiles/profile_manager.cc

@@ -57,18 +57,6 @@ 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:
@@ -481,7 +469,6 @@ 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);
 }
@@ -633,49 +620,16 @@ void ProfileManager::AddProfileToCache(Profile* profile) {
   string16 username = UTF8ToUTF16(profile->GetPrefs()->GetString(
       prefs::kGoogleServicesUsername));
 
-  // 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);
-
+  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(),
-                          profile_name,
+                            cache.ChooseNameForNewProfile(icon_index),
                             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)));
-    }
   }
 }
 

chrome/browser/profiles/profile_manager.h

@@ -264,10 +264,6 @@ 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();
 

chrome/browser/profiles/profile_manager_unittest.cc

@@ -97,14 +97,12 @@ class ProfileManagerTest : public testing::Test {
   virtual void SetUp() {
     // Create a new temporary directory, and store the path
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    static_cast<TestingBrowserProcess*>(g_browser_process)->SetProfileManager(
-        new testing::ProfileManager(temp_dir_.path()));
+    profile_manager_.reset(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.
-    ProfileManager* profile_manager = g_browser_process->profile_manager();
-    profile_manager->GetProfileInfoCache();
-    profile_manager->RemoveProfileShortcutManagerForTesting();
+    profile_manager_->GetProfileInfoCache();
+    profile_manager_->RemoveProfileShortcutManagerForTesting();
 #endif
 #if defined(OS_CHROMEOS)
   CommandLine *cl = CommandLine::ForCurrentProcess();
@@ -113,8 +111,7 @@ class ProfileManagerTest : public testing::Test {
   }
 
   virtual void TearDown() {
-    static_cast<TestingBrowserProcess*>(g_browser_process)->SetProfileManager(
-        NULL);
+    profile_manager_.reset();
     message_loop_.RunAllPending();
   }
 
@@ -144,30 +141,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(),
-      g_browser_process->profile_manager()->GetInitialProfileDir().value());
+  EXPECT_EQ(expected_default.value(),
+            profile_manager_->GetInitialProfileDir().value());
 }
 
 #if defined(OS_CHROMEOS)
@@ -180,18 +177,17 @@ 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
@@ -203,15 +199,13 @@ 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.
@@ -227,8 +221,7 @@ TEST_F(ProfileManagerTest, CreateAndUseTwoProfiles) {
   // Make sure any pending tasks run before we destroy the profiles.
   message_loop_.RunAllPending();
 
-  static_cast<TestingBrowserProcess*>(g_browser_process)->SetProfileManager(
-      NULL);
+  profile_manager_.reset();
 
   // Make sure history cleans up correctly.
   message_loop_.RunAllPending();
@@ -248,8 +241,7 @@ TEST_F(ProfileManagerTest, DISABLED_CreateProfileAsync) {
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(1));
 
-  g_browser_process->profile_manager()->CreateProfileAsync(dest_path,
-                                                           &mock_observer);
+  profile_manager_->CreateProfileAsync(dest_path, &mock_observer);
 
   message_loop_.RunAllPending();
 }
@@ -276,11 +268,9 @@ TEST_F(ProfileManagerTest, CreateProfileAsyncMultipleRequests) {
   EXPECT_CALL(mock_observer3, OnProfileCreated(
       SameNotNull(), NotFail())).Times(testing::AtLeast(1));
 
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-
-  profile_manager->CreateProfileAsync(dest_path, &mock_observer1);
-  profile_manager->CreateProfileAsync(dest_path, &mock_observer2);
-  profile_manager->CreateProfileAsync(dest_path, &mock_observer3);
+  profile_manager_->CreateProfileAsync(dest_path, &mock_observer1);
+  profile_manager_->CreateProfileAsync(dest_path, &mock_observer2);
+  profile_manager_->CreateProfileAsync(dest_path, &mock_observer3);
 
   message_loop_.RunAllPending();
 }
@@ -295,17 +285,14 @@ TEST_F(ProfileManagerTest, CreateProfilesAsync) {
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(3));
 
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-
-  profile_manager->CreateProfileAsync(dest_path1, &mock_observer);
-  profile_manager->CreateProfileAsync(dest_path2, &mock_observer);
+  profile_manager_->CreateProfileAsync(dest_path1, &mock_observer);
+  profile_manager_->CreateProfileAsync(dest_path2, &mock_observer);
 
   message_loop_.RunAllPending();
 }
 
 TEST_F(ProfileManagerTest, AutoloadProfilesWithBackgroundApps) {
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  ProfileInfoCache& cache = profile_manager_->GetProfileInfoCache();
 
   EXPECT_EQ(0u, cache.GetNumberOfProfiles());
   cache.AddProfileToCache(cache.GetUserDataDir().AppendASCII("path_1"),
@@ -318,31 +305,7 @@ TEST_F(ProfileManagerTest, AutoloadProfilesWithBackgroundApps) {
   cache.SetBackgroundStatusOfProfileAtIndex(2, true);
   EXPECT_EQ(3u, cache.GetNumberOfProfiles());
 
-  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());
+  profile_manager_->AutoloadProfiles();
 
-  // 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));
+  EXPECT_EQ(2u, profile_manager_->GetLoadedProfiles().size());
 }

chrome/browser/ui/webui/options/manage_profile_handler.cc

@@ -11,16 +11,13 @@
 #include "base/value_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/gaia_info_update_service.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_info_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/ui/webui/web_ui_util.h"
 #include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/pref_names.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/public/browser/notification_service.h"
 #include "grit/generated_resources.h"
@@ -142,15 +139,13 @@ void ManageProfileHandler::SetProfileNameAndIcon(const ListValue* args) {
   ProfileInfoCache& cache =
       g_browser_process->profile_manager()->GetProfileInfoCache();
   size_t profile_index = cache.GetIndexOfProfileWithPath(profile_file_path);
+  if (profile_index == std::string::npos)
+    return;
 
   string16 new_profile_name;
   if (!args->GetString(1, &new_profile_name))
     return;
 
-  Profile* profile =
-      g_browser_process->profile_manager()->GetProfile(profile_file_path);
-  if (!profile)
-    return;
   if (new_profile_name == cache.GetGAIANameOfProfileAtIndex(profile_index)) {
     // Set the profile to use the GAIA name as the profile name. Note, this
     // is a little weird if the user typed their GAIA name manually but
@@ -161,11 +156,7 @@ void ManageProfileHandler::SetProfileNameAndIcon(const ListValue* args) {
     if (profile_index == std::string::npos)
       return;
   } else {
-    PrefService* pref_service = profile->GetPrefs();
-    // Updating the profile preference will cause the cache to be updated for
-    // this preference.
-    pref_service->SetString(prefs::kProfileName, UTF16ToUTF8(new_profile_name));
-
+    cache.SetNameOfProfileAtIndex(profile_index, new_profile_name);
     // Changing the profile name can invalidate the profile index.
     profile_index = cache.GetIndexOfProfileWithPath(profile_file_path);
     if (profile_index == std::string::npos)
@@ -187,11 +178,8 @@ void ManageProfileHandler::SetProfileNameAndIcon(const ListValue* args) {
   if (icon_url == gaia_picture_url_) {
     cache.SetIsUsingGAIAPictureOfProfileAtIndex(profile_index, true);
   } else if (cache.IsDefaultAvatarIconUrl(icon_url, &new_icon_index)) {
-    PrefService* pref_service = profile->GetPrefs();
     ProfileMetrics::LogProfileAvatarSelection(new_icon_index);
-    // Updating the profile preference will cause the cache to be updated for
-    // this preference.
-    pref_service->SetInteger(prefs::kProfileAvatarIndex, new_icon_index);
+    cache.SetAvatarIconOfProfileAtIndex(profile_index, new_icon_index);
     cache.SetIsUsingGAIAPictureOfProfileAtIndex(profile_index, false);
   }
 

chrome/common/pref_names.cc

@@ -881,10 +881,6 @@ const char kPasswordsUseLocalProfileId[] =
     "profile.passwords_use_local_profile_id";
 #endif
 
-// Profile avatar and name
-const char kProfileAvatarIndex[] = "profile.avatar_index";
-const char kProfileName[] = "profile.name";
-
 // *************** LOCAL STATE ***************
 // These are attached to the machine/installation
 

chrome/common/pref_names.h

@@ -313,9 +313,6 @@ extern const char kLocalProfileId[];
 extern const char kPasswordsUseLocalProfileId[];
 #endif
 
-extern const char kProfileAvatarIndex[];
-extern const char kProfileName[];
-
 // Local state prefs. Please add Profile prefs above instead.
 extern const char kCertRevocationCheckingEnabled[];
 extern const char kSSL3Enabled[];