Populate the AccountInfo Structure with additional info.

This CL adds the following fields to the AccountInfo structure:
- Full Name
- Given Name
- Locale

BUG=466799

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

Cr-Commit-Position: refs/heads/master@{#324929}

3 files changed, 155 insertions, 37 deletions

diff --git a/components/signin/core/browser/account_tracker_service.cc b/components/signin/core/browser/account_tracker_service.cc
index 2da5a98..1191a96 100644
--- a/components/signin/core/browser/account_tracker_service.cc
+++ b/components/signin/core/browser/account_tracker_service.cc
@@ -28,6 +28,9 @@ const char kAccountKeyPath[] = "account_id";
 const char kAccountEmailPath[] = "email";
 const char kAccountGaiaPath[] = "gaia";
 const char kAccountHostedDomainPath[] = "hd";
+const char kAccountFullNamePath[] = "full_name";
+const char kAccountGivenNamePath[] = "given_name";
+const char kAccountLocalePath[] = "locale";
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
 // IsRefreshTokenDeviceIdExperimentEnabled is called from
@@ -167,8 +170,9 @@ AccountTrackerService::AccountInfo::AccountInfo() {}
 AccountTrackerService::AccountInfo::~AccountInfo() {}
 
 bool AccountTrackerService::AccountInfo::IsValid() {
-  return account_id.empty() || email.empty() || gaia.empty() ||
-         hosted_domain.empty();
+  return !account_id.empty() && !email.empty() && !gaia.empty() &&
+         !hosted_domain.empty() && !full_name.empty() && !given_name.empty() &&
+         !locale.empty();
 }
 
 
@@ -319,7 +323,7 @@ void AccountTrackerService::OnRefreshTokenAvailable(
   // ensure the Fetch doesn't occur until after ProfileImpl::OnPrefsLoaded().
   if (state.info.gaia.empty())
 #else
-  if (state.info.IsValid())
+  if (!state.info.IsValid())
 #endif
     StartFetchingUserInfo(account_id);
 
@@ -420,6 +424,10 @@ void AccountTrackerService::SetAccountStateFromUserInfo(
       state.info.hosted_domain = kNoHostedDomainFound;
     }
 
+    user_info->GetString("name", &state.info.full_name);
+    user_info->GetString("given_name", &state.info.given_name);
+    user_info->GetString("locale", &state.info.locale);
+
     NotifyAccountUpdated(state);
     SaveToPrefs(state);
   }
@@ -470,7 +478,13 @@ void AccountTrackerService::LoadFromPrefs() {
           state.info.email = base::UTF16ToUTF8(value);
         if (dict->GetString(kAccountHostedDomainPath, &value))
           state.info.hosted_domain = base::UTF16ToUTF8(value);
-        if (!state.info.IsValid())
+        if (dict->GetString(kAccountFullNamePath, &value))
+          state.info.full_name = base::UTF16ToUTF8(value);
+        if (dict->GetString(kAccountGivenNamePath, &value))
+          state.info.given_name = base::UTF16ToUTF8(value);
+        if (dict->GetString(kAccountLocalePath, &value))
+          state.info.locale = base::UTF16ToUTF8(value);
+        if (state.info.IsValid())
           NotifyAccountUpdated(state);
       }
     }
@@ -501,6 +515,9 @@ void AccountTrackerService::SaveToPrefs(const AccountState& state) {
   dict->SetString(kAccountEmailPath, state.info.email);
   dict->SetString(kAccountGaiaPath, state.info.gaia);
   dict->SetString(kAccountHostedDomainPath, state.info.hosted_domain);
+  dict->SetString(kAccountFullNamePath, state.info.full_name);
+  dict->SetString(kAccountGivenNamePath, state.info.given_name);
+  dict->SetString(kAccountLocalePath, state.info.locale);
 }
 
 void AccountTrackerService::RemoveFromPrefs(const AccountState& state) {
diff --git a/components/signin/core/browser/account_tracker_service.h b/components/signin/core/browser/account_tracker_service.h
index d037eb2..20249dd 100644
--- a/components/signin/core/browser/account_tracker_service.h
+++ b/components/signin/core/browser/account_tracker_service.h
@@ -49,7 +49,10 @@ class AccountTrackerService : public KeyedService,
     std::string account_id;  // The account ID used by OAuth2TokenService.
     std::string gaia;
     std::string email;
+    std::string full_name;
+    std::string given_name;
     std::string hosted_domain;
+    std::string locale;
     // TODO(rogerta): eventually this structure will include other information
     // about the account, like full name, profile picture URL, etc.
 
diff --git a/components/signin/core/browser/account_tracker_service_unittest.cc b/components/signin/core/browser/account_tracker_service_unittest.cc
index c5bb7b4..137e8e3 100644
--- a/components/signin/core/browser/account_tracker_service_unittest.cc
+++ b/components/signin/core/browser/account_tracker_service_unittest.cc
@@ -22,6 +22,23 @@
 
 namespace {
 
+const std::string kTokenInfoResponseFormat =
+    "{                        \
+      \"id\": \"%s\",         \
+      \"email\": \"%s\",      \
+      \"hd\": \"\",           \
+      \"name\": \"%s\",       \
+      \"given_name\": \"%s\", \
+      \"locale\": \"%s\"      \
+    }";
+
+const std::string kTokenInfoIncompleteResponseFormat =
+    "{                        \
+      \"id\": \"%s\",         \
+      \"email\": \"%s\",      \
+      \"hd\": \"\",           \
+    }";
+
 enum TrackingEventType {
   UPDATED,
   REMOVED,
@@ -35,6 +52,30 @@ std::string AccountIdToGaiaId(const std::string account_id) {
   return "gaia-" + account_id;
 }
 
+std::string AccountIdToFullName(const std::string account_id) {
+  return "full-name-" + account_id;
+}
+
+std::string AccountIdToGivenName(const std::string account_id) {
+  return "given-name-" + account_id;
+}
+
+std::string AccountIdToLocale(const std::string account_id) {
+  return "locale-" + account_id;
+}
+
+void CheckAccountDetails(const std::string account_id,
+                         const AccountTrackerService::AccountInfo& info) {
+  EXPECT_EQ(account_id, info.account_id);
+  EXPECT_EQ(AccountIdToGaiaId(account_id), info.gaia);
+  EXPECT_EQ(AccountIdToEmail(account_id), info.email);
+  EXPECT_EQ(AccountTrackerService::kNoHostedDomainFound,
+            info.hosted_domain);
+  EXPECT_EQ(AccountIdToFullName(account_id), info.full_name);
+  EXPECT_EQ(AccountIdToGivenName(account_id), info.given_name);
+  EXPECT_EQ(AccountIdToLocale(account_id), info.locale);
+}
+
 class TrackingEvent {
  public:
   TrackingEvent(TrackingEventType type,
@@ -236,11 +277,23 @@ class AccountTrackerServiceTest : public testing::Test {
 
   std::string GenerateValidTokenInfoResponse(const std::string& account_id) {
     return base::StringPrintf(
-        "{\"id\": \"%s\", \"email\": \"%s\", \"hd\": \"\"}",
+        kTokenInfoResponseFormat.c_str(),
+        AccountIdToGaiaId(account_id).c_str(),
+        AccountIdToEmail(account_id).c_str(),
+        AccountIdToFullName(account_id).c_str(),
+        AccountIdToGivenName(account_id).c_str(),
+        AccountIdToLocale(account_id).c_str());
+  }
+
+  std::string GenerateIncompleteTokenInfoResponse(
+      const std::string& account_id) {
+    return base::StringPrintf(
+        kTokenInfoIncompleteResponseFormat.c_str(),
         AccountIdToGaiaId(account_id).c_str(),
         AccountIdToEmail(account_id).c_str());
   }
   void ReturnOAuthUrlFetchSuccess(const std::string& account_id);
+  void ReturnOAuthUrlFetchSuccessIncomplete(const std::string& account_id);
   void ReturnOAuthUrlFetchFailure(const std::string& account_id);
 
   net::TestURLFetcherFactory* test_fetcher_factory() {
@@ -287,6 +340,14 @@ void AccountTrackerServiceTest::ReturnOAuthUrlFetchSuccess(
                              GenerateValidTokenInfoResponse(account_id));
 }
 
+void AccountTrackerServiceTest::ReturnOAuthUrlFetchSuccessIncomplete(
+    const std::string& account_id) {
+  IssueAccessToken(account_id);
+  ReturnOAuthUrlFetchResults(gaia::GaiaOAuthClient::kUrlFetcherId,
+                             net::HTTP_OK,
+                             GenerateIncompleteTokenInfoResponse(account_id));
+}
+
 void AccountTrackerServiceTest::ReturnOAuthUrlFetchFailure(
     const std::string& account_id) {
   IssueAccessToken(account_id);
@@ -390,21 +451,9 @@ TEST_F(AccountTrackerServiceTest, GetAccounts) {
       account_tracker()->GetAccounts();
 
   EXPECT_EQ(3u, infos.size());
-  EXPECT_EQ("alpha", infos[0].account_id);
-  EXPECT_EQ(AccountIdToGaiaId("alpha"), infos[0].gaia);
-  EXPECT_EQ(AccountIdToEmail("alpha"), infos[0].email);
-  EXPECT_EQ(AccountTrackerService::kNoHostedDomainFound,
-            infos[0].hosted_domain);
-  EXPECT_EQ("beta", infos[1].account_id);
-  EXPECT_EQ(AccountIdToGaiaId("beta"), infos[1].gaia);
-  EXPECT_EQ(AccountIdToEmail("beta"), infos[1].email);
-  EXPECT_EQ(AccountTrackerService::kNoHostedDomainFound,
-            infos[1].hosted_domain);
-  EXPECT_EQ("gamma", infos[2].account_id);
-  EXPECT_EQ(AccountIdToGaiaId("gamma"), infos[2].gaia);
-  EXPECT_EQ(AccountIdToEmail("gamma"), infos[2].email);
-  EXPECT_EQ(AccountTrackerService::kNoHostedDomainFound,
-            infos[2].hosted_domain);
+  CheckAccountDetails("alpha", infos[0]);
+  CheckAccountDetails("beta", infos[1]);
+  CheckAccountDetails("gamma", infos[2]);
 }
 
 TEST_F(AccountTrackerServiceTest, GetAccountInfo_Empty) {
@@ -427,10 +476,7 @@ TEST_F(AccountTrackerServiceTest, GetAccountInfo_TokenAvailable_UserInfo) {
   ReturnOAuthUrlFetchSuccess("alpha");
   AccountTrackerService::AccountInfo info =
       account_tracker()->GetAccountInfo("alpha");
-  ASSERT_EQ("alpha", info.account_id);
-  ASSERT_EQ(AccountIdToGaiaId("alpha"), info.gaia);
-  ASSERT_EQ(AccountIdToEmail("alpha"), info.email);
-  ASSERT_EQ(AccountTrackerService::kNoHostedDomainFound, info.hosted_domain);
+  CheckAccountDetails("alpha", info);
 }
 
 TEST_F(AccountTrackerServiceTest, GetAccountInfo_TokenAvailable_EnableNetwork) {
@@ -457,10 +503,7 @@ TEST_F(AccountTrackerServiceTest, GetAccountInfo_TokenAvailable_EnableNetwork) {
 
   AccountTrackerService::AccountInfo info =
       tracker.GetAccountInfo("alpha");
-  ASSERT_EQ("alpha", info.account_id);
-  ASSERT_EQ(AccountIdToGaiaId("alpha"), info.gaia);
-  ASSERT_EQ(AccountIdToEmail("alpha"), info.email);
-  ASSERT_EQ(AccountTrackerService::kNoHostedDomainFound, info.hosted_domain);
+  CheckAccountDetails("alpha", info);
   tracker.Shutdown();
 }
 
@@ -516,7 +559,7 @@ TEST_F(AccountTrackerServiceTest, Persistence) {
     tracker.Shutdown();
   }
 
-  // Create a new tracker and make sure it loads the accounts corectly from
+  // Create a new tracker and make sure it loads the accounts correctly from
   // persistence.
   {
     AccountTrackerService tracker;
@@ -529,11 +572,8 @@ TEST_F(AccountTrackerServiceTest, Persistence) {
     std::vector<AccountTrackerService::AccountInfo> infos =
         tracker.GetAccounts();
     ASSERT_EQ(2u, infos.size());
-    EXPECT_EQ(AccountIdToGaiaId("alpha"), infos[0].gaia);
-    EXPECT_EQ(AccountIdToEmail("alpha"), infos[0].email);
-    EXPECT_EQ("beta", infos[1].account_id);
-    EXPECT_EQ(AccountIdToGaiaId("beta"), infos[1].gaia);
-    EXPECT_EQ(AccountIdToEmail("beta"), infos[1].email);
+    CheckAccountDetails("alpha", infos[0]);
+    CheckAccountDetails("beta", infos[1]);
 
     // Remove account.
     SimulateTokenRevoked("alpha");
@@ -551,9 +591,7 @@ TEST_F(AccountTrackerServiceTest, Persistence) {
     std::vector<AccountTrackerService::AccountInfo> infos =
         tracker.GetAccounts();
     ASSERT_EQ(1u, infos.size());
-    EXPECT_EQ("beta", infos[0].account_id);
-    EXPECT_EQ(AccountIdToGaiaId("beta"), infos[0].gaia);
-    EXPECT_EQ(AccountIdToEmail("beta"), infos[0].email);
+    CheckAccountDetails("beta", infos[0]);
     tracker.Shutdown();
   }
 }
@@ -575,3 +613,63 @@ TEST_F(AccountTrackerServiceTest, SeedAccountInfo) {
   EXPECT_EQ(gaia_id, infos[0].gaia);
   EXPECT_EQ(email, infos[0].email);
 }
+
+TEST_F(AccountTrackerServiceTest, UpgradeToFullAccountInfo) {
+  // Start by simulating an incomplete account info and let it be saved to
+  // prefs.
+  {
+    AccountTrackerService tracker;
+    tracker.Initialize(token_service(), signin_client());
+    tracker.EnableNetworkFetches();
+    SimulateTokenAvailable("incomplete");
+    ReturnOAuthUrlFetchSuccessIncomplete("incomplete");
+    tracker.Shutdown();
+  }
+
+  {
+    AccountTrackerService tracker;
+    tracker.Initialize(token_service(), signin_client());
+
+    // Validate that the loaded AccountInfo from prefs is considered invalid.
+    std::vector<AccountTrackerService::AccountInfo> infos =
+        tracker.GetAccounts();
+    ASSERT_EQ(1u, infos.size());
+    ASSERT_FALSE(infos[0].IsValid());
+
+    // Enable network fetches and simulate the same account getting a refresh
+    // token containing all the info.
+    tracker.EnableNetworkFetches();
+    SimulateTokenAvailable("incomplete");
+    ReturnOAuthUrlFetchSuccess("incomplete");
+
+    // Validate that the account is now considered valid.
+    infos = tracker.GetAccounts();
+    ASSERT_EQ(1u, infos.size());
+    ASSERT_TRUE(infos[0].IsValid());
+
+    tracker.Shutdown();
+  }
+
+  // Reinstantiate a tracker to validate that the AccountInfo saved to prefs is
+  // now the upgraded one, considered valid.
+  {
+    AccountTrackerService tracker;
+    tracker.AddObserver(observer());
+    tracker.Initialize(token_service(), signin_client());
+    ASSERT_TRUE(observer()->CheckEvents(TrackingEvent(UPDATED, "incomplete")));
+    // Make sure there are no events in the observer
+    observer()->Clear();
+    // Enabling network fetches shouldn't cause any actual fetch since the
+    // AccountInfos loaded from prefs should be valid.
+    tracker.EnableNetworkFetches();
+
+    std::vector<AccountTrackerService::AccountInfo> infos =
+        tracker.GetAccounts();
+    ASSERT_EQ(1u, infos.size());
+    ASSERT_TRUE(infos[0].IsValid());
+    // Check that no network fetches were made.
+    ASSERT_TRUE(observer()->CheckEvents());
+
+    tracker.Shutdown();
+  }
+}