[Profiles] Fix the usage of custom/default names and avatars

(Hopefully for the last time).
This is based on treib's patch 476703002

The problem that this CL is fixing is that it was a mess to tell whether
a user had a default name/avatar (Lemonade) because we randomly assigned
it at a profile creation time (in old, legacy cases), and then they synced
it because that's what sync does, or because they did that on purpose.
The idea being, of course, that if we randomly called them Lemonade and they
have a Gaia name, we should use the latter, but if they synced "bob", we should
use the sync name.

Ok. Here's how this works now:
- there's a preference for the profile name, and a preference "if it's default"
- if this preference is not set, we assume this is a legacy created profile, so
if it is named First User or Lemonade, the user probably didn't change that.
This sets a "kIsUsingDefaultNameKey" preference in the ProfileInfoCache
- if the ProfileInfoCache has a profile with a kIsUsingDefaultNameKey, then it
uses a Gaia name if it's available.
- the moment the user changes the name of a profile, it stops being default. So
even if I change the profile name from Lemonade to Pickles, even though Pickles
is one of the default profile names, we allow this insanity. 
- a similar dance is done for the default avatar/gaia avatar, only here we maintain
two preferences, because the ProfileInfoCache holds both the gaia and the profile avatar.

Profiles, right? <3

BUG=394586

Review URL: https://codereview.chromium.org/476993002

Cr-Commit-Position: refs/heads/master@{#290101}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290101 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 121 insertions, 49 deletions

diff --git a/chrome/browser/profiles/gaia_info_update_service_unittest.cc b/chrome/browser/profiles/gaia_info_update_service_unittest.cc
index b4359cf..3160852 100644
--- a/chrome/browser/profiles/gaia_info_update_service_unittest.cc
+++ b/chrome/browser/profiles/gaia_info_update_service_unittest.cc
@@ -119,7 +119,7 @@ class GAIAInfoUpdateServiceTest : public ProfileInfoCacheTest {
 
   void RenameProfile(const base::string16& full_name,
                      const base::string16& given_name) {
-    gfx::Image image = gfx::test::CreateImage();
+    gfx::Image image = gfx::test::CreateImage(256,256);
     std::string url("foo.com");
     ProfileDownloadSuccess(full_name, given_name, image, url);
 
@@ -159,7 +159,7 @@ TEST_F(GAIAInfoUpdateServiceTest, DownloadSuccess) {
 
   base::string16 name = base::ASCIIToUTF16("Pat Smith");
   base::string16 given_name = base::ASCIIToUTF16("Pat");
-  gfx::Image image = gfx::test::CreateImage();
+  gfx::Image image = gfx::test::CreateImage(256, 256);
   std::string url("foo.com");
   ProfileDownloadSuccess(name, given_name, image, url);
 
@@ -256,7 +256,7 @@ TEST_F(GAIAInfoUpdateServiceTest, LogOut) {
   signin_manager->SetAuthenticatedUsername("pat@example.com");
   base::string16 gaia_name = base::UTF8ToUTF16("Pat Foo");
   GetCache()->SetGAIANameOfProfileAtIndex(0, gaia_name);
-  gfx::Image gaia_picture = gfx::test::CreateImage();
+  gfx::Image gaia_picture = gfx::test::CreateImage(256,256);
   GetCache()->SetGAIAPictureOfProfileAtIndex(0, &gaia_picture);
 
   // Set a fake picture URL.
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 60f79fe..bf0862d 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -359,6 +359,21 @@ void ProfileImpl::RegisterProfilePrefs(
       prefs::kProfileAvatarIndex,
       -1,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  // Whether a profile is using an avatar without having explicitely chosen it
+  // (i.e. was assigned by default by legacy profile creation).
+  registry->RegisterBooleanPref(
+      prefs::kProfileUsingDefaultAvatar,
+      true,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  registry->RegisterBooleanPref(
+      prefs::kProfileUsingGAIAAvatar,
+      false,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  // Whether a profile is using a default avatar name (eg. Pickles or Person 1).
+  registry->RegisterBooleanPref(
+      prefs::kProfileUsingDefaultName,
+      true,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterStringPref(
       prefs::kSupervisedUserId,
       std::string(),
@@ -541,14 +556,31 @@ void ProfileImpl::DoFinalInit() {
       prefs::kDefaultZoomLevel,
       base::Bind(&ProfileImpl::OnDefaultZoomLevelChanged,
                  base::Unretained(this)));
+
+  // Changes in the profile avatar.
   pref_change_registrar_.Add(
       prefs::kProfileAvatarIndex,
       base::Bind(&ProfileImpl::UpdateProfileAvatarCache,
                  base::Unretained(this)));
   pref_change_registrar_.Add(
+      prefs::kProfileUsingDefaultAvatar,
+      base::Bind(&ProfileImpl::UpdateProfileAvatarCache,
+                 base::Unretained(this)));
+  pref_change_registrar_.Add(
+      prefs::kProfileUsingGAIAAvatar,
+      base::Bind(&ProfileImpl::UpdateProfileAvatarCache,
+                 base::Unretained(this)));
+
+  // Changes in the profile name.
+  pref_change_registrar_.Add(
+      prefs::kProfileUsingDefaultName,
+      base::Bind(&ProfileImpl::UpdateProfileNameCache,
+                 base::Unretained(this)));
+  pref_change_registrar_.Add(
       prefs::kProfileName,
       base::Bind(&ProfileImpl::UpdateProfileNameCache,
                  base::Unretained(this)));
+
   pref_change_registrar_.Add(
       prefs::kForceEphemeralProfiles,
       base::Bind(&ProfileImpl::UpdateProfileIsEphemeralCache,
@@ -1361,6 +1393,9 @@ void ProfileImpl::UpdateProfileNameCache() {
     std::string profile_name =
         GetPrefs()->GetString(prefs::kProfileName);
     cache.SetNameOfProfileAtIndex(index, base::UTF8ToUTF16(profile_name));
+    bool default_name =
+        GetPrefs()->GetBoolean(prefs::kProfileUsingDefaultName);
+    cache.SetProfileIsUsingDefaultNameAtIndex(index, default_name);
   }
 }
 
@@ -1372,6 +1407,12 @@ void ProfileImpl::UpdateProfileAvatarCache() {
     size_t avatar_index =
         GetPrefs()->GetInteger(prefs::kProfileAvatarIndex);
     cache.SetAvatarIconOfProfileAtIndex(index, avatar_index);
+    bool default_avatar =
+        GetPrefs()->GetBoolean(prefs::kProfileUsingDefaultAvatar);
+    cache.SetProfileIsUsingDefaultAvatarAtIndex(index, default_avatar);
+    bool gaia_avatar =
+        GetPrefs()->GetBoolean(prefs::kProfileUsingGAIAAvatar);
+    cache.SetIsUsingGAIAPictureOfProfileAtIndex(index, gaia_avatar);
   }
 }
 
diff --git a/chrome/browser/profiles/profile_info_cache.cc b/chrome/browser/profiles/profile_info_cache.cc
index 10a3259..8aaa047 100644
--- a/chrome/browser/profiles/profile_info_cache.cc
+++ b/chrome/browser/profiles/profile_info_cache.cc
@@ -137,30 +137,6 @@ void DeleteBitmap(const base::FilePath& image_path) {
   base::DeleteFile(image_path, false);
 }
 
-bool IsDefaultName(const base::string16& name) {
-  // Check if it's a "First user" old-style name.
-  if (name == l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME) ||
-      name == l10n_util::GetStringUTF16(IDS_LEGACY_DEFAULT_PROFILE_NAME))
-    return true;
-
-  // Check if it's one of the old-style profile names.
-  for (size_t i = 0; i < arraysize(kDefaultNames); ++i) {
-    if (name == l10n_util::GetStringUTF16(kDefaultNames[i]))
-      return true;
-  }
-
-  // Check whether it's one of the "Person %d" style names.
-  std::string default_name_format = l10n_util::GetStringFUTF8(
-      IDS_NEW_NUMBERED_PROFILE_NAME, base::string16()) + "%d";
-
-  int generic_profile_number;  // Unused. Just a placeholder for sscanf.
-  int assignments = sscanf(base::UTF16ToUTF8(name).c_str(),
-                           default_name_format.c_str(),
-                           &generic_profile_number);
-  // Unless it matched the format, this is a custom name.
-  return assignments == 1;
-}
-
 }  // namespace
 
 ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
@@ -177,8 +153,15 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
     base::string16 name;
     info->GetString(kNameKey, &name);
     sorted_keys_.insert(FindPositionForProfile(it.key(), name), it.key());
-    bool using_default_name = IsDefaultName(name);
-    info->SetBoolean(kIsUsingDefaultNameKey, using_default_name);
+
+    bool using_default_name;
+    if (!info->GetBoolean(kIsUsingDefaultNameKey, &using_default_name)) {
+      // If the preference hasn't been set, and the name is default, assume
+      // that the user hasn't done this on purpose.
+      using_default_name = IsDefaultProfileName(name);
+      info->SetBoolean(kIsUsingDefaultNameKey, using_default_name);
+    }
+
     // For profiles that don't have the "using default avatar" state set yet,
     // assume it's the same as the "using default name" state.
     if (!info->HasKey(kIsUsingDefaultAvatarKey)) {
@@ -223,7 +206,7 @@ void ProfileInfoCache::AddProfileToCache(
   info->SetString(kSupervisedUserId, supervised_user_id);
   info->SetBoolean(kIsOmittedFromProfileListKey, !supervised_user_id.empty());
   info->SetBoolean(kProfileIsEphemeral, false);
-  info->SetBoolean(kIsUsingDefaultNameKey, IsDefaultName(name));
+  info->SetBoolean(kIsUsingDefaultNameKey, IsDefaultProfileName(name));
   // Assume newly created profiles use a default avatar.
   info->SetBoolean(kIsUsingDefaultAvatarKey, true);
   cache->SetWithoutPathExpansion(key, info.release());
@@ -489,10 +472,10 @@ void ProfileInfoCache::SetNameOfProfileAtIndex(size_t index,
 
   base::string16 old_display_name = GetNameOfProfileAtIndex(index);
   info->SetString(kNameKey, name);
-  info->SetBoolean(kIsUsingDefaultNameKey, false);
 
   // This takes ownership of |info|.
   SetInfoForProfileAtIndex(index, info.release());
+
   base::string16 new_display_name = GetNameOfProfileAtIndex(index);
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
   UpdateSortForProfileIndex(index);
@@ -538,8 +521,6 @@ void ProfileInfoCache::SetAvatarIconOfProfileAtIndex(size_t index,
   // This takes ownership of |info|.
   SetInfoForProfileAtIndex(index, info.release());
 
-  SetProfileIsUsingDefaultAvatarAtIndex(index, false);
-
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
 
   // If needed, start downloading the high-res avatar.
@@ -758,6 +739,30 @@ void ProfileInfoCache::SetProfileIsUsingDefaultAvatarAtIndex(
   SetInfoForProfileAtIndex(index, info.release());
 }
 
+bool ProfileInfoCache::IsDefaultProfileName(const base::string16& name) {
+  // Check if it's a "First user" old-style name.
+  if (name == l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME) ||
+      name == l10n_util::GetStringUTF16(IDS_LEGACY_DEFAULT_PROFILE_NAME))
+    return true;
+
+  // Check if it's one of the old-style profile names.
+  for (size_t i = 0; i < arraysize(kDefaultNames); ++i) {
+    if (name == l10n_util::GetStringUTF16(kDefaultNames[i]))
+      return true;
+  }
+
+  // Check whether it's one of the "Person %d" style names.
+  std::string default_name_format = l10n_util::GetStringFUTF8(
+      IDS_NEW_NUMBERED_PROFILE_NAME, base::ASCIIToUTF16("%d"));
+
+  int generic_profile_number;  // Unused. Just a placeholder for sscanf.
+  int assignments = sscanf(base::UTF16ToUTF8(name).c_str(),
+                           default_name_format.c_str(),
+                           &generic_profile_number);
+  // Unless it matched the format, this is a custom name.
+  return assignments == 1;
+}
+
 base::string16 ProfileInfoCache::ChooseNameForNewProfile(
     size_t icon_index) const {
   base::string16 name;
diff --git a/chrome/browser/profiles/profile_info_cache.h b/chrome/browser/profiles/profile_info_cache.h
index 0a35376..f8c4390 100644
--- a/chrome/browser/profiles/profile_info_cache.h
+++ b/chrome/browser/profiles/profile_info_cache.h
@@ -121,6 +121,9 @@ class ProfileInfoCache : public ProfileInfoInterface,
   void SetProfileIsUsingDefaultNameAtIndex(size_t index, bool value);
   void SetProfileIsUsingDefaultAvatarAtIndex(size_t index, bool value);
 
+  // Determines whether |name| is one of the default assigned names.
+  bool IsDefaultProfileName(const base::string16& name);
+
   // Returns unique name that can be assigned to a newly created profile.
   base::string16 ChooseNameForNewProfile(size_t icon_index) const;
 
diff --git a/chrome/browser/profiles/profile_info_cache_unittest.cc b/chrome/browser/profiles/profile_info_cache_unittest.cc
index 82c1f3f..ab25357 100644
--- a/chrome/browser/profiles/profile_info_cache_unittest.cc
+++ b/chrome/browser/profiles/profile_info_cache_unittest.cc
@@ -334,6 +334,8 @@ TEST_F(ProfileInfoCacheTest, GAIAName) {
   // Don't use GAIA name as profile name. This re-sorts the cache.
   base::string16 custom_name(ASCIIToUTF16("Custom name"));
   GetCache()->SetNameOfProfileAtIndex(index2, custom_name);
+  GetCache()->SetProfileIsUsingDefaultNameAtIndex(index2, false);
+
   index1 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_1"));
   index2 = GetCache()->GetIndexOfProfileWithPath(GetProfilePath("path_2"));
 
@@ -382,8 +384,9 @@ TEST_F(ProfileInfoCacheTest, GAIAPicture) {
   EXPECT_TRUE(gfx::test::IsEqual(
       gaia_image, GetCache()->GetAvatarIconOfProfileAtIndex(1)));
 
-  // Set another avatar. This should make it preferred over the GAIA image.
+  // Set a non-default avatar. This should be preferred over the GAIA image.
   GetCache()->SetAvatarIconOfProfileAtIndex(1, kOtherAvatarIndex);
+  GetCache()->SetProfileIsUsingDefaultAvatarAtIndex(1, false);
   EXPECT_FALSE(GetCache()->ProfileIsUsingDefaultAvatarAtIndex(1));
   EXPECT_FALSE(GetCache()->IsUsingGAIAPictureOfProfileAtIndex(1));
   int other_avatar_id =
diff --git a/chrome/browser/profiles/profile_manager_unittest.cc b/chrome/browser/profiles/profile_manager_unittest.cc
index b43a973..1645dc1 100644
--- a/chrome/browser/profiles/profile_manager_unittest.cc
+++ b/chrome/browser/profiles/profile_manager_unittest.cc
@@ -1042,6 +1042,7 @@ TEST_F(ProfileManagerTest, ProfileDisplayNamePreservesCustomName) {
   // We should display custom names for local profiles.
   const base::string16 custom_profile_name = ASCIIToUTF16("Batman");
   cache.SetNameOfProfileAtIndex(0, custom_profile_name);
+  cache.SetProfileIsUsingDefaultNameAtIndex(0, false);
   EXPECT_EQ(custom_profile_name, cache.GetNameOfProfileAtIndex(0));
   EXPECT_EQ(custom_profile_name,
             profiles::GetAvatarNameForProfile(profile1->GetPath()));
diff --git a/chrome/browser/profiles/profiles_state.cc b/chrome/browser/profiles/profiles_state.cc
index c19fb33..8552cdf 100644
--- a/chrome/browser/profiles/profiles_state.cc
+++ b/chrome/browser/profiles/profiles_state.cc
@@ -94,6 +94,15 @@ base::string16 GetAvatarButtonTextForProfile(Profile* profile) {
 void UpdateProfileName(Profile* profile,
                        const base::string16& new_profile_name) {
   PrefService* pref_service = profile->GetPrefs();
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+
+  // This is only called when updating the profile name through the UI,
+  // so we can assume the user has done this on purpose.
+  size_t profile_index = cache.GetIndexOfProfileWithPath(profile->GetPath());
+  if (profile_index != std::string::npos)
+    pref_service->SetBoolean(prefs::kProfileUsingDefaultName, false);
+
   // Updating the profile preference will cause the cache to be updated for
   // this preference.
   pref_service->SetString(prefs::kProfileName,
diff --git a/chrome/browser/profiles/profiles_state.h b/chrome/browser/profiles/profiles_state.h
index 649e1c7..39aae3d 100644
--- a/chrome/browser/profiles/profiles_state.h
+++ b/chrome/browser/profiles/profiles_state.h
@@ -41,6 +41,9 @@ base::string16 GetAvatarButtonTextForProfile(Profile* profile);
 
 // Update the name of |profile| to |new_profile_name|. This updates the
 // profile preferences, which triggers an update in the ProfileInfoCache.
+// This method should be called when the user is explicitely changing
+// the profile name, as it will always set |prefs::kProfileUsingDefaultName|
+// to false.
 void UpdateProfileName(Profile* profile,
                        const base::string16& new_profile_name);
 
diff --git a/chrome/browser/ui/cocoa/profiles/avatar_button_controller_unittest.mm b/chrome/browser/ui/cocoa/profiles/avatar_button_controller_unittest.mm
index 5ce2e9e..a14fbef 100644
--- a/chrome/browser/ui/cocoa/profiles/avatar_button_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/profiles/avatar_button_controller_unittest.mm
@@ -17,8 +17,8 @@
 #import "chrome/browser/ui/cocoa/profiles/profile_chooser_controller.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/signin/core/common/profile_management_switches.h"
-
-const char kDefaultProfileName[] = "default";
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
 
 class AvatarButtonControllerTest : public CocoaProfileTest {
  public:
@@ -50,7 +50,8 @@ class AvatarButtonControllerTest : public CocoaProfileTest {
 
 TEST_F(AvatarButtonControllerTest, ButtonShown) {
   EXPECT_FALSE([view() isHidden]);
-  EXPECT_EQ(kDefaultProfileName, base::SysNSStringToUTF8([button() title]));
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME),
+            base::SysNSStringToUTF16([button() title]));
 }
 
 TEST_F(AvatarButtonControllerTest, DoubleOpen) {
diff --git a/chrome/browser/ui/webui/options/manage_profile_handler.cc b/chrome/browser/ui/webui/options/manage_profile_handler.cc
index e3ad3c0..0979c10 100644
--- a/chrome/browser/ui/webui/options/manage_profile_handler.cc
+++ b/chrome/browser/ui/webui/options/manage_profile_handler.cc
@@ -340,12 +340,6 @@ void ManageProfileHandler::SetProfileIconAndName(const base::ListValue* args) {
   if (!GetProfilePathFromArgs(args, &profile_file_path))
     return;
 
-  ProfileInfoCache& cache =
-      g_browser_process->profile_manager()->GetProfileInfoCache();
-  size_t profile_index = cache.GetIndexOfProfileWithPath(profile_file_path);
-  if (profile_index == std::string::npos)
-    return;
-
   Profile* profile =
       g_browser_process->profile_manager()->GetProfile(profile_file_path);
   if (!profile)
@@ -355,13 +349,17 @@ void ManageProfileHandler::SetProfileIconAndName(const base::ListValue* args) {
   if (!args->GetString(1, &icon_url))
     return;
 
+  PrefService* pref_service = profile->GetPrefs();
+  // Updating the profile preferences will cause the cache to be updated.
+
   // Metrics logging variable.
   bool previously_using_gaia_icon =
-      cache.IsUsingGAIAPictureOfProfileAtIndex(profile_index);
+      pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar);
 
   size_t new_icon_index;
   if (icon_url == gaia_picture_url_) {
-    cache.SetIsUsingGAIAPictureOfProfileAtIndex(profile_index, true);
+    pref_service->SetBoolean(prefs::kProfileUsingDefaultAvatar, false);
+    pref_service->SetBoolean(prefs::kProfileUsingGAIAAvatar, true);
     if (!previously_using_gaia_icon) {
       // Only log if they changed to the GAIA photo.
       // Selection of GAIA photo as avatar is logged as part of the function
@@ -370,11 +368,9 @@ void ManageProfileHandler::SetProfileIconAndName(const base::ListValue* args) {
     }
   } else if (profiles::IsDefaultAvatarIconUrl(icon_url, &new_icon_index)) {
     ProfileMetrics::LogProfileAvatarSelection(new_icon_index);
-    PrefService* pref_service = profile->GetPrefs();
-    // Updating the profile preference will cause the cache to be updated for
-    // this preference.
     pref_service->SetInteger(prefs::kProfileAvatarIndex, new_icon_index);
-    cache.SetIsUsingGAIAPictureOfProfileAtIndex(profile_index, false);
+    pref_service->SetBoolean(prefs::kProfileUsingDefaultAvatar, false);
+    pref_service->SetBoolean(prefs::kProfileUsingGAIAAvatar, false);
   }
   ProfileMetrics::LogProfileUpdate(profile_file_path);
 
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index b68a4a0..8847ae3 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -1137,6 +1137,13 @@ const char kImportSavedPasswords[] = "import_saved_passwords";
 // Profile avatar and name
 const char kProfileAvatarIndex[] = "profile.avatar_index";
 const char kProfileName[] = "profile.name";
+// Whether a profile is using a default avatar name (eg. Pickles or Person 1)
+// because it was randomly assigned at profile creation time.
+const char kProfileUsingDefaultName[] = "profile.using_default_name";
+// Whether a profile is using an avatar without having explicitely chosen it
+// (i.e. was assigned by default by legacy profile creation).
+const char kProfileUsingDefaultAvatar[] = "profile.using_default_avatar";
+const char kProfileUsingGAIAAvatar[] = "profile.using_gaia_avatar";
 
 // The supervised user ID.
 const char kSupervisedUserId[] = "profile.managed_user_id";
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 094381c..c95ca68 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -356,7 +356,10 @@ extern const char kImportSearchEngine[];
 extern const char kImportSavedPasswords[];
 
 extern const char kProfileAvatarIndex[];
+extern const char kProfileUsingDefaultName[];
 extern const char kProfileName[];
+extern const char kProfileUsingDefaultAvatar[];
+extern const char kProfileUsingGAIAAvatar[];
 extern const char kSupervisedUserId[];
 
 extern const char kProfileGAIAInfoUpdateTime[];