Delay fetching account info until OnRefreshTokensLoaded().

components/signin/core/browser/account_fetcher_service.cc

Index: components/signin/core/browser/account_fetcher_service.cc
diff --git a/components/signin/core/browser/account_fetcher_service.cc b/components/signin/core/browser/account_fetcher_service.cc
index 22c804f6b496723a0a935e75abe82b37765e7d51..336f7b539fc4b279de2a59efc10e2bd6aed36020 100644
--- a/components/signin/core/browser/account_fetcher_service.cc
+++ b/components/signin/core/browser/account_fetcher_service.cc
@@ -16,6 +16,7 @@
 #include "components/signin/core/browser/refresh_token_annotation_request.h"
 #include "components/signin/core/browser/signin_client.h"
 #include "components/signin/core/common/signin_switches.h"
+#include "google_apis/gaia/oauth2_token_service_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace {
@@ -102,12 +103,7 @@ void AccountFetcherService::EnableNetworkFetches() {
   DCHECK(CalledOnValidThread());
   DCHECK(!network_fetches_enabled_);
   network_fetches_enabled_ = true;
-  // If there are accounts in |pending_user_info_fetches_|, they were deemed
-  // invalid after being loaded from prefs and need to be fetched now instead of
-  // waiting after the timer.
-  for (const std::string& account_id : pending_user_info_fetches_)
-    StartFetchingUserInfo(account_id);
-  pending_user_info_fetches_.clear();
+  FetchPendingAccountInfo();
 
   // Now that network fetches are enabled, schedule the next refresh.
   ScheduleNextRefresh();
@@ -122,6 +118,14 @@ void AccountFetcherService::FetchUserInfoBeforeSignin(
   RefreshAccountInfo(account_id, false);
 }
 
+#if defined(OS_ANDROID)
+void AccountFetcherService::OnAccountIdNameMapSeeded() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(token_service_->GetDelegate()->NeedsAccountIdNameMap());
+  FetchPendingAccountInfo();
+}
+#endif
+
 void AccountFetcherService::RefreshAllAccountInfo(bool only_fetch_if_invalid) {
   std::vector<std::string> accounts = token_service_->GetAccounts();
   for (std::vector<std::string>::const_iterator it = accounts.begin();
@@ -146,6 +150,8 @@ void AccountFetcherService::UpdateChildInfo() {
       ResetChildInfo();
     if (!AccountSupportsUserInfo(candidate))
       return;
+    if (!CanFetchAccountInfo(candidate))
+      return;
     child_request_account_id_ = candidate;
     StartFetchingChildInfo(candidate);
   } else {
@@ -180,10 +186,8 @@ void AccountFetcherService::ScheduleNextRefresh() {
 void AccountFetcherService::StartFetchingUserInfo(
     const std::string& account_id) {
   DCHECK(CalledOnValidThread());
-  if (!network_fetches_enabled_) {
-    pending_user_info_fetches_.push_back(account_id);
+  if (!CanFetchAccountInfo(account_id))
     return;
-  }
 
   if (!ContainsKey(user_info_requests_, account_id)) {
     DVLOG(1) << "StartFetching " << account_id;
@@ -320,3 +324,28 @@ void AccountFetcherService::OnRefreshTokensLoaded() {
   RefreshAllAccountInfo(true);
   UpdateChildInfo();
 }
+
+bool AccountFetcherService::CanFetchAccountInfo(const std::string& account_id) {

[anthonyvd, 2015/10/06 18:18:55]

Maybe rename this so that it's clearer that it can have side effects? As-is the
name implies it's only a boolean check but it can insert things in
|pending_user_info_fetches_|.

[knn, 2015/10/13 13:15:41]

Will do.

+  // There are two reasons for an account to be in |pending_user_info_fetches_|,
+  // either they were fetched before network fetch was enabled or the
+  // Id <-> Name mapping was required and unavailable for the account.
+  bool defer_fetch =
+      !network_fetches_enabled_ ||
+      (token_service_->GetDelegate()->NeedsAccountIdNameMap() &&
+       !account_tracker_service_->HasIdNameMappingForAccount(account_id));
+  if (defer_fetch)
+    pending_user_info_fetches_.insert(account_id);
+  return !defer_fetch;
+}
+
+void AccountFetcherService::FetchPendingAccountInfo() {
+  // Since further failures will be added back, we create a new copy of the
+  // accounts list before processing it.
+  std::set<std::string> accounts_to_process;
+  pending_user_info_fetches_.swap(accounts_to_process);
+  for (const std::string& account : accounts_to_process) {
+    if (account_tracker_service_->IsTrackingAccount(account))
+      StartFetchingUserInfo(account);
+  }
+  UpdateChildInfo();
+}