[Signin]: Change strategy for the profile name and avatar icon.

The profile name and avatar icon will not be syncable prefs anymore. We
have decided that the user will need to customize their profile once per
device if they wish to. We also change the strategy for showing the
avatar icon of the profile to the following:
* For profiles that are not signed in, on events like sign in or enable
sync, Gaia avatar wins.
* If the user customizes their profile while having a butter account or
a primary account on this device, the customized avatar wins.
* If the profile has a butter account A with customized avatar, if the
user decides to start syncing with account A, the customized avatar
wins. We do not want to change the state of the profile given that it is
the same account.
* If the profile has a butter account A with customized avatar, if the
user decides to start syncing with account B, the Gaia avatar wins.

Bug: 1013597
Change-Id: Ic60cca56d31e20a31d010e9f8995a03ca7819623
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1855968
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706062}

chrome/browser/profiles/profile_impl.cc

@@ -396,10 +396,6 @@ void ProfileImpl::RegisterProfilePrefs(
                                 safe_search_util::YOUTUBE_RESTRICT_OFF);
   registry->RegisterStringPref(prefs::kAllowedDomainsForApps, std::string());
 
-#if defined(OS_ANDROID)
-  // The following prefs don't need to be sync'd to mobile. This file isn't
-  // compiled on iOS so we only need to exclude them syncing from the Android
-  // build.
   registry->RegisterIntegerPref(prefs::kProfileAvatarIndex, -1);
   // Whether a profile is using an avatar without having explicitely chosen it
   // (i.e. was assigned by default by legacy profile creation).
@@ -408,25 +404,6 @@ void ProfileImpl::RegisterProfilePrefs(
   // Whether a profile is using a default avatar name (eg. Pickles or Person 1).
   registry->RegisterBooleanPref(prefs::kProfileUsingDefaultName, true);
   registry->RegisterStringPref(prefs::kProfileName, std::string());
-#else
-  registry->RegisterIntegerPref(
-      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::kProfileName, std::string(),
-                               user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
-#endif
 
   registry->RegisterStringPref(prefs::kSupervisedUserId, std::string());
 #if defined(OS_ANDROID)

chrome/browser/signin/signin_profile_attributes_updater.cc

@@ -10,17 +10,21 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/signin/signin_util.h"
+#include "chrome/common/pref_names.h"
 #include "components/signin/public/identity_manager/account_info.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 
 SigninProfileAttributesUpdater::SigninProfileAttributesUpdater(
     signin::IdentityManager* identity_manager,
     SigninErrorController* signin_error_controller,
     ProfileAttributesStorage* profile_attributes_storage,
-    const base::FilePath& profile_path)
+    const base::FilePath& profile_path,
+    PrefService* prefs)
     : identity_manager_(identity_manager),
       signin_error_controller_(signin_error_controller),
       profile_attributes_storage_(profile_attributes_storage),
-      profile_path_(profile_path) {
+      profile_path_(profile_path),
+      prefs_(prefs) {
   DCHECK(identity_manager_);
   DCHECK(signin_error_controller_);
   DCHECK(profile_attributes_storage_);
@@ -55,6 +59,15 @@ void SigninProfileAttributesUpdater::UpdateProfileAttributes() {
       (!base::FeatureList::IsEnabled(kPersistUPAInProfileInfoCache) &&
        !identity_manager_->HasPrimaryAccount());
 
+  if (account_info.gaia != entry->GetGAIAId() ||
+      !gaia::AreEmailsSame(account_info.email,
+                           base::UTF16ToUTF8(entry->GetUserName()))) {
+    // Reset prefs. Note: this will also update the |ProfileAttributesEntry|.
+    prefs_->ClearPref(prefs::kProfileUsingDefaultName);
+    prefs_->ClearPref(prefs::kProfileUsingDefaultAvatar);
+    prefs_->ClearPref(prefs::kProfileUsingGAIAAvatar);
+  }
+
   if (clear_profile) {
     entry->SetLocalAuthCredentials(std::string());
     entry->SetAuthInfo(std::string(), base::string16(), false);

chrome/browser/signin/signin_profile_attributes_updater.h

@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 
@@ -25,7 +26,8 @@ class SigninProfileAttributesUpdater
       signin::IdentityManager* identity_manager,
       SigninErrorController* signin_error_controller,
       ProfileAttributesStorage* profile_attributes_storage,
-      const base::FilePath& profile_path);
+      const base::FilePath& profile_path,
+      PrefService* prefs);
 
   ~SigninProfileAttributesUpdater() override;
 
@@ -51,6 +53,7 @@ class SigninProfileAttributesUpdater
   SigninErrorController* signin_error_controller_;
   ProfileAttributesStorage* profile_attributes_storage_;
   const base::FilePath profile_path_;
+  PrefService* prefs_;
   ScopedObserver<signin::IdentityManager, signin::IdentityManager::Observer>
       identity_manager_observer_{this};
   ScopedObserver<SigninErrorController, SigninErrorController::Observer>

chrome/browser/signin/signin_profile_attributes_updater_factory.cc

@@ -47,7 +47,7 @@ KeyedService* SigninProfileAttributesUpdaterFactory::BuildServiceInstanceFor(
       IdentityManagerFactory::GetForProfile(profile),
       SigninErrorControllerFactory::GetForProfile(profile),
       &g_browser_process->profile_manager()->GetProfileAttributesStorage(),
-      profile->GetPath());
+      profile->GetPath(), profile->GetPrefs());
 }
 
 bool SigninProfileAttributesUpdaterFactory::ServiceIsCreatedWithBrowserContext()

chrome/browser/signin/signin_profile_attributes_updater_unittest.cc

@@ -7,9 +7,11 @@
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/signin/signin_util.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
@@ -22,9 +24,29 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
-
 const char kEmail[] = "example@email.com";
 
+#if !defined(OS_CHROMEOS)
+void CheckProfilePrefsReset(PrefService* pref_service) {
+  EXPECT_TRUE(pref_service->GetBoolean(prefs::kProfileUsingDefaultName));
+  EXPECT_TRUE(pref_service->GetBoolean(prefs::kProfileUsingDefaultAvatar));
+  EXPECT_FALSE(pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar));
+}
+
+void CheckProfilePrefsSet(PrefService* pref_service) {
+  EXPECT_FALSE(pref_service->GetBoolean(prefs::kProfileUsingDefaultName));
+  EXPECT_FALSE(pref_service->GetBoolean(prefs::kProfileUsingDefaultAvatar));
+  EXPECT_TRUE(pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar));
+}
+
+// Set the prefs to nondefault values.
+void SetProfilePrefs(PrefService* pref_service) {
+  pref_service->SetBoolean(prefs::kProfileUsingDefaultName, false);
+  pref_service->SetBoolean(prefs::kProfileUsingDefaultAvatar, false);
+  pref_service->SetBoolean(prefs::kProfileUsingGAIAAvatar, true);
+  CheckProfilePrefsSet(pref_service);
+}
+#endif  // !defined(OS_CHROMEOS)
 }  // namespace
 
 class SigninProfileAttributesUpdaterTest : public testing::Test {
@@ -41,7 +63,8 @@ class SigninProfileAttributesUpdaterTest : public testing::Test {
     signin_profile_attributes_updater_ =
         std::make_unique<SigninProfileAttributesUpdater>(
             identity_test_env_.identity_manager(), &signin_error_controller_,
-            profile_manager_.profile_attributes_storage(), profile_path_);
+            profile_manager_.profile_attributes_storage(), profile_->GetPath(),
+            profile_->GetPrefs());
   }
 
   void SetUp() override {
@@ -49,18 +72,16 @@ class SigninProfileAttributesUpdaterTest : public testing::Test {
 
     ASSERT_TRUE(profile_manager_.SetUp());
     std::string name = "profile_name";
-    profile_path_ = profile_manager_
-                        .CreateTestingProfile(
-                            name, /*prefs=*/nullptr, base::UTF8ToUTF16(name), 0,
-                            std::string(), TestingProfile::TestingFactories())
-                        ->GetPath();
+    profile_ = profile_manager_.CreateTestingProfile(
+        name, /*prefs=*/nullptr, base::UTF8ToUTF16(name), 0, std::string(),
+        TestingProfile::TestingFactories());
 
     RecreateSigninProfileAttributesUpdater();
   }
 
   content::BrowserTaskEnvironment task_environment_;
   TestingProfileManager profile_manager_;
-  base::FilePath profile_path_;
+  TestingProfile* profile_;
   signin::IdentityTestEnvironment identity_test_env_;
   SigninErrorController signin_error_controller_;
   std::unique_ptr<SigninProfileAttributesUpdater>
@@ -73,7 +94,7 @@ class SigninProfileAttributesUpdaterTest : public testing::Test {
 TEST_F(SigninProfileAttributesUpdaterTest, SigninSignout) {
   ProfileAttributesEntry* entry;
   ASSERT_TRUE(profile_manager_.profile_attributes_storage()
-                  ->GetProfileAttributesWithPath(profile_path_, &entry));
+                  ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
   ASSERT_EQ(entry->GetSigninState(), SigninState::kNotSignedIn);
   EXPECT_FALSE(entry->IsSigninRequired());
 
@@ -94,7 +115,7 @@ TEST_F(SigninProfileAttributesUpdaterTest, SigninSignout) {
 TEST_F(SigninProfileAttributesUpdaterTest, AuthError) {
   ProfileAttributesEntry* entry;
   ASSERT_TRUE(profile_manager_.profile_attributes_storage()
-                  ->GetProfileAttributesWithPath(profile_path_, &entry));
+                  ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
 
   std::string account_id =
       identity_test_env_.MakePrimaryAccountAvailable(kEmail).account_id;
@@ -121,6 +142,78 @@ TEST_F(SigninProfileAttributesUpdaterTest, AuthError) {
 }
 
 #if !defined(OS_CHROMEOS)
+TEST_F(SigninProfileAttributesUpdaterTest, SigninSignoutResetsProfilePrefs) {
+  PrefService* pref_service = profile_->GetPrefs();
+  ProfileAttributesEntry* entry;
+  ASSERT_TRUE(profile_manager_.profile_attributes_storage()
+                  ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
+
+  // Set profile prefs.
+  CheckProfilePrefsReset(pref_service);
+#if !defined(OS_ANDROID)
+  SetProfilePrefs(pref_service);
+
+  // Set UPA should reset profile prefs.
+  AccountInfo account_info = identity_test_env_.MakeAccountAvailableWithCookies(
+      "email1@example.com", "gaia_id_1");
+  EXPECT_FALSE(entry->IsAuthenticated());
+  CheckProfilePrefsReset(pref_service);
+  SetProfilePrefs(pref_service);
+  // Signout should reset profile prefs.
+  identity_test_env_.SetCookieAccounts({});
+  CheckProfilePrefsReset(pref_service);
+#endif  // !defined(OS_ANDROID)
+
+  SetProfilePrefs(pref_service);
+  // Set primary account should reset profile prefs.
+  AccountInfo primary_account =
+      identity_test_env_.MakePrimaryAccountAvailable("primary@example.com");
+  CheckProfilePrefsReset(pref_service);
+  SetProfilePrefs(pref_service);
+  // Disabling sync should reset profile prefs.
+  identity_test_env_.ClearPrimaryAccount();
+  CheckProfilePrefsReset(pref_service);
+}
+
+#if !defined(OS_ANDROID)
+TEST_F(SigninProfileAttributesUpdaterTest,
+       EnablingSyncWithUPAAccountShouldNotResetProfilePrefs) {
+  PrefService* pref_service = profile_->GetPrefs();
+  ProfileAttributesEntry* entry;
+  ASSERT_TRUE(profile_manager_.profile_attributes_storage()
+                  ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
+  // Set UPA.
+  AccountInfo account_info = identity_test_env_.MakeAccountAvailableWithCookies(
+      "email1@example.com", "gaia_id_1");
+  EXPECT_FALSE(entry->IsAuthenticated());
+  SetProfilePrefs(pref_service);
+  // Set primary account to be the same as the UPA.
+  // Given it is the same account, profile prefs should keep the same state.
+  identity_test_env_.SetPrimaryAccount(account_info.email);
+  EXPECT_TRUE(entry->IsAuthenticated());
+  CheckProfilePrefsSet(pref_service);
+  identity_test_env_.ClearPrimaryAccount();
+  CheckProfilePrefsReset(pref_service);
+}
+
+TEST_F(SigninProfileAttributesUpdaterTest,
+       EnablingSyncWithDifferentAccountThanUPAResetsProfilePrefs) {
+  PrefService* pref_service = profile_->GetPrefs();
+  ProfileAttributesEntry* entry;
+  ASSERT_TRUE(profile_manager_.profile_attributes_storage()
+                  ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
+  AccountInfo account_info = identity_test_env_.MakeAccountAvailableWithCookies(
+      "email1@example.com", "gaia_id_1");
+  EXPECT_FALSE(entry->IsAuthenticated());
+  SetProfilePrefs(pref_service);
+  // Set primary account to a different account than the UPA.
+  AccountInfo primary_account =
+      identity_test_env_.MakePrimaryAccountAvailable("primary@example.com");
+  EXPECT_TRUE(entry->IsAuthenticated());
+  CheckProfilePrefsReset(pref_service);
+}
+#endif  // !defined(OS_ANDROID)
+
 class SigninProfileAttributesUpdaterWithForceSigninTest
     : public SigninProfileAttributesUpdaterTest {
   void SetUp() override {
@@ -137,7 +230,7 @@ class SigninProfileAttributesUpdaterWithForceSigninTest
 TEST_F(SigninProfileAttributesUpdaterWithForceSigninTest, IsSigninRequired) {
   ProfileAttributesEntry* entry;
   ASSERT_TRUE(profile_manager_.profile_attributes_storage()
-                  ->GetProfileAttributesWithPath(profile_path_, &entry));
+                  ->GetProfileAttributesWithPath(profile_->GetPath(), &entry));
   EXPECT_FALSE(entry->IsAuthenticated());
   EXPECT_TRUE(entry->IsSigninRequired());
 
@@ -152,4 +245,4 @@ TEST_F(SigninProfileAttributesUpdaterWithForceSigninTest, IsSigninRequired) {
   EXPECT_EQ(entry->GetSigninState(), SigninState::kNotSignedIn);
   EXPECT_TRUE(entry->IsSigninRequired());
 }
-#endif
+#endif  // !defined(OS_CHROMEOS)

components/signin/public/identity_manager/identity_test_environment.cc

@@ -278,6 +278,15 @@ AccountInfo IdentityTestEnvironment::MakeAccountAvailable(
   return signin::MakeAccountAvailable(identity_manager(), email);
 }
 
+AccountInfo IdentityTestEnvironment::MakeAccountAvailableWithCookies(
+    const std::string& email,
+    const std::string& gaia_id) {
+  return signin::MakeAccountAvailableWithCookies(
+      identity_manager(),
+      dependencies_owner_->signin_client()->GetTestURLLoaderFactory(), email,
+      gaia_id);
+}
+
 void IdentityTestEnvironment::SetRefreshTokenForAccount(
     const CoreAccountId& account_id) {
   return signin::SetRefreshTokenForAccount(identity_manager(), account_id);

components/signin/public/identity_manager/identity_test_environment.h

@@ -114,6 +114,14 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
   // Returns the AccountInfo of the newly-available account.
   AccountInfo MakePrimaryAccountAvailable(const std::string& email);
 
+  // Combination of MakeAccountAvailable() and SetCookieAccounts() for a single
+  // account. It makes an account available for the given email address, and
+  // GAIA ID, setting the cookies and the refresh token that correspond uniquely
+  // to that email address. Blocks until the account is available. Returns the
+  // AccountInfo of the newly-available account.
+  AccountInfo MakeAccountAvailableWithCookies(const std::string& email,
+                                              const std::string& gaia_id);
+
   // Clears the primary account if present, with |policy| used to determine
   // whether to keep or remove all accounts. On non-ChromeOS, results in the
   // firing of the IdentityManager and PrimaryAccountManager callbacks for