Change ProfileDownloader to use AccountTrackerService.

BUG=466799

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

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

chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc

@@ -39,6 +39,7 @@
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_downloader.h"
+#include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -216,6 +217,23 @@ class UserImageManagerTest : public LoginManagerTest,
     return user_data_dir_.Append(username).AddExtension(extension);
   }
 
+  // Seeds the AccountTrackerService with test data so the ProfileDownloader can
+  // retrieve the picture URL and fetch the image.
+  void SeedAccountTrackerService(const std::string& username,
+                                 Profile* profile) {
+    AccountTrackerService::AccountInfo info;
+    info.account_id = std::string();
+    info.gaia = username;
+    info.email = username;
+    info.full_name = username;
+    info.given_name = username;
+    info.hosted_domain = std::string();
+    info.locale = username;
+    info.picture_url = "http://localhost/avatar.jpg";
+
+    AccountTrackerServiceFactory::GetForProfile(profile)->SeedAccountInfo(info);
+  }
+
   // Completes the download of all non-image profile data for the user
   // |username|.  This method must only be called after a profile data
   // download has been started.  |url_fetcher_factory| will capture
@@ -234,17 +252,6 @@ class UserImageManagerTest : public LoginManagerTest,
         OnGetTokenSuccess(NULL,
                           std::string(),
                           base::Time::Now() + base::TimeDelta::FromDays(1));
-
-    net::TestURLFetcher* fetcher = url_fetcher_factory->GetFetcherByID(
-        gaia::GaiaOAuthClient::kUrlFetcherId);
-    ASSERT_TRUE(fetcher);
-    fetcher->SetResponseString(
-        "{ \"picture\": \"http://localhost/avatar.jpg\" }");
-    fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
-                                              net::OK));
-    fetcher->set_response_code(200);
-    fetcher->delegate()->OnURLFetchComplete(fetcher);
-    base::RunLoop().RunUntilIdle();
   }
 
   // Completes the download of the currently logged-in user's profile image.
@@ -523,6 +530,8 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserImageFromProfileImage) {
 
   UserImageManagerImpl::IgnoreProfileDataDownloadDelayForTesting();
   LoginUser(kTestUser1);
+  Profile* profile = ProfileHelper::Get()->GetProfileByUserUnsafe(user);
+  SeedAccountTrackerService(kTestUser1, profile);
 
   run_loop_.reset(new base::RunLoop);
   UserImageManager* user_image_manager =
@@ -574,6 +583,8 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest,
 
   UserImageManagerImpl::IgnoreProfileDataDownloadDelayForTesting();
   LoginUser(kTestUser1);
+  Profile* profile = ProfileHelper::Get()->GetProfileByUserUnsafe(user);
+  SeedAccountTrackerService(kTestUser1, profile);
 
   run_loop_.reset(new base::RunLoop);
   UserImageManager* user_image_manager =

chrome/browser/profiles/profile_downloader.h

@@ -12,7 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/image_decoder.h"
-#include "google_apis/gaia/gaia_oauth_client.h"
+#include "components/signin/core/browser/account_tracker_service.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -27,11 +27,11 @@ class URLFetcher;
 
 // Downloads user profile information. The profile picture is decoded in a
 // sandboxed process.
-class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
-                          public net::URLFetcherDelegate,
+class ProfileDownloader : public net::URLFetcherDelegate,
                           public ImageDecoder::ImageRequest,
                           public OAuth2TokenService::Observer,
-                          public OAuth2TokenService::Consumer {
+                          public OAuth2TokenService::Consumer,
+                          public AccountTrackerService::Observer {
  public:
   enum PictureStatus {
     PICTURE_SUCCESS,
@@ -82,14 +82,11 @@ class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
 
  private:
   friend class ProfileDownloaderTest;
-  FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, ParseData);
+  FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, AccountInfoReady);
+  FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, AccountInfoNotReady);
   FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, DefaultURL);
 
-  // gaia::GaiaOAuthClient::Delegate implementation.
-  void OnGetUserInfoResponse(
-      scoped_ptr<base::DictionaryValue> user_info) override;
-  void OnOAuthError() override;
-  void OnNetworkError(int response_code) override;
+  void FetchImageData();
 
   // Overriden from net::URLFetcherDelegate:
   void OnURLFetchComplete(const net::URLFetcher* source) override;
@@ -108,16 +105,11 @@ class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
   void OnGetTokenFailure(const OAuth2TokenService::Request* request,
                          const GoogleServiceAuthError& error) override;
 
-  // Parses the entry response and gets the name, profile image URL and locale.
-  // |data| should be the JSON formatted data return by the response.
-  // Returns false to indicate a parsing error.
-  static bool ParseProfileJSON(base::DictionaryValue* root_dictionary,
-                               base::string16* full_name,
-                               base::string16* given_name,
-                               std::string* url,
-                               int image_size,
-                               std::string* profile_locale,
-                               base::string16* hosted_domain);
+
+  // Implementation of AccountTrackerService::Observer.
+  void OnAccountUpdated(
+      const AccountTrackerService::AccountInfo& info) override;
+
   // Returns true if the image url is url of the default profile picture.
   static bool IsDefaultProfileImageURL(const std::string& url);
 
@@ -134,16 +126,13 @@ class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
   ProfileDownloaderDelegate* delegate_;
   std::string account_id_;
   std::string auth_token_;
-  scoped_ptr<gaia::GaiaOAuthClient> gaia_client_;
   scoped_ptr<net::URLFetcher> profile_image_fetcher_;
   scoped_ptr<OAuth2TokenService::Request> oauth2_access_token_request_;
-  base::string16 profile_hosted_domain_;
-  base::string16 profile_full_name_;
-  base::string16 profile_given_name_;
-  std::string profile_locale_;
+  AccountTrackerService::AccountInfo account_info_;
   SkBitmap profile_picture_;
   PictureStatus picture_status_;
-  std::string picture_url_;
+  AccountTrackerService* account_tracker_service_;
+  bool waiting_for_account_info_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileDownloader);
 };

chrome/browser/signin/fake_account_tracker_service.cc

@@ -31,16 +31,24 @@ void FakeAccountTrackerService::StartFetchingUserInfo(
 }
 
 void FakeAccountTrackerService::FakeUserInfoFetchSuccess(
-    const std::string& account_id,
     const std::string& email,
     const std::string& gaia,
-    const std::string& hosted_domain) {
+    const std::string& hosted_domain,
+    const std::string& full_name,
+    const std::string& given_name,
+    const std::string& locale,
+    const std::string& picture_url) {
   base::DictionaryValue user_info;
   user_info.SetString("id", gaia);
   user_info.SetString("email", email);
   user_info.SetString("hd", hosted_domain);
+  user_info.SetString("name", full_name);
+  user_info.SetString("given_name", given_name);
+  user_info.SetString("locale", locale);
+  user_info.SetString("picture", picture_url);
   std::vector<std::string> service_flags;
-  SetAccountStateFromUserInfo(account_id, &user_info, &service_flags);
+  SetAccountStateFromUserInfo(
+      PickAccountIdForAccount(gaia, email), &user_info, &service_flags);
 }
 
 void FakeAccountTrackerService::SendRefreshTokenAnnotationRequest(

chrome/browser/signin/fake_account_tracker_service.h

@@ -20,10 +20,13 @@ class FakeAccountTrackerService : public AccountTrackerService {
  public:
   static KeyedService* Build(content::BrowserContext* context);
 
-  void FakeUserInfoFetchSuccess(const std::string& account_id,
-                                const std::string& email,
+  void FakeUserInfoFetchSuccess(const std::string& email,
                                 const std::string& gaia,
-                                const std::string& hosted_domain);
+                                const std::string& hosted_domain,
+                                const std::string& full_name,
+                                const std::string& given_name,
+                                const std::string& locale,
+                                const std::string& picture_url);
 
  private:
   FakeAccountTrackerService();

chrome/browser/signin/signin_manager_unittest.cc

@@ -276,8 +276,6 @@ TEST_F(SigninManagerTest, SignInWithRefreshTokenCallsPostSignout) {
         AccountTrackerServiceFactory::GetForProfile(profile()));
   account_tracker_service->SeedAccountInfo(gaia_id, email);
   account_tracker_service->EnableNetworkFetches();
-  std::string account_id = account_tracker_service->PickAccountIdForAccount(
-      gaia_id, email);
 
   ASSERT_TRUE(signin_client()->get_signed_in_password().empty());
 
@@ -291,8 +289,13 @@ TEST_F(SigninManagerTest, SignInWithRefreshTokenCallsPostSignout) {
   // PostSignedIn is not called until the AccountTrackerService returns.
   ASSERT_EQ("", signin_client()->get_signed_in_password());
 
-  account_tracker_service->FakeUserInfoFetchSuccess(
-      account_id, email, gaia_id, "google.com");
+  account_tracker_service->FakeUserInfoFetchSuccess(email,
+                                                    gaia_id,
+                                                    "google.com",
+                                                    "full_name",
+                                                    "given_name",
+                                                    "locale",
+                                                    "http://www.google.com");
 
   // AccountTracker and SigninManager are both done and PostSignedIn was called.
   ASSERT_EQ("password", signin_client()->get_signed_in_password());

components/signin/core/browser/account_tracker_service.cc

@@ -34,6 +34,7 @@ const char kAccountHostedDomainPath[] = "hd";
 const char kAccountFullNamePath[] = "full_name";
 const char kAccountGivenNamePath[] = "given_name";
 const char kAccountLocalePath[] = "locale";
+const char kAccountPictureURLPath[] = "picture_url";
 const char kAccountServiceFlagsPath[] = "service_flags";
 
 const base::TimeDelta kRefreshFromTokenServiceDelay =
@@ -53,6 +54,9 @@ bool IsRefreshTokenDeviceIdExperimentEnabled() {
 // This must be a string which can never be a valid domain.
 const char AccountTrackerService::kNoHostedDomainFound[] = "NO_HOSTED_DOMAIN";
 
+// This must be a string which can never be a valid picture URL.
+const char AccountTrackerService::kNoPictureURLFound[] = "NO_PICTURE_URL";
+
 class AccountInfoFetcher : public OAuth2TokenService::Consumer,
                            public gaia::GaiaOAuthClient::Delegate,
                            public GaiaAuthConsumer {
@@ -230,10 +234,10 @@ void AccountInfoFetcher::OnNetworkError(int response_code) {
 AccountTrackerService::AccountInfo::AccountInfo() {}
 AccountTrackerService::AccountInfo::~AccountInfo() {}
 
-bool AccountTrackerService::AccountInfo::IsValid() {
+bool AccountTrackerService::AccountInfo::IsValid() const {
   return !account_id.empty() && !email.empty() && !gaia.empty() &&
          !hosted_domain.empty() && !full_name.empty() && !given_name.empty() &&
-         !locale.empty();
+         !locale.empty() && !picture_url.empty();
 }
 
 
@@ -520,6 +524,13 @@ void AccountTrackerService::SetAccountStateFromUserInfo(
     user_info->GetString("given_name", &state.info.given_name);
     user_info->GetString("locale", &state.info.locale);
 
+    std::string picture_url;
+    if(user_info->GetString("picture", &picture_url)) {
+      state.info.picture_url = picture_url;
+    } else {
+      state.info.picture_url = kNoPictureURLFound;
+    }
+
     state.info.service_flags = *service_flags;
 
     NotifyAccountUpdated(state);
@@ -578,6 +589,8 @@ void AccountTrackerService::LoadFromPrefs() {
           state.info.given_name = base::UTF16ToUTF8(value);
         if (dict->GetString(kAccountLocalePath, &value))
           state.info.locale = base::UTF16ToUTF8(value);
+        if (dict->GetString(kAccountPictureURLPath, &value))
+          state.info.picture_url = base::UTF16ToUTF8(value);
 
         const base::ListValue* service_flags_list;
         if (dict->GetList(kAccountServiceFlagsPath, &service_flags_list)) {
@@ -626,6 +639,7 @@ void AccountTrackerService::SaveToPrefs(const AccountState& state) {
   dict->SetString(kAccountFullNamePath, state.info.full_name);
   dict->SetString(kAccountGivenNamePath, state.info.given_name);
   dict->SetString(kAccountLocalePath, state.info.locale);
+  dict->SetString(kAccountPictureURLPath, state.info.picture_url);
 
   scoped_ptr<base::ListValue> service_flags_list;
   service_flags_list.reset(new base::ListValue);
@@ -759,3 +773,22 @@ std::string AccountTrackerService::SeedAccountInfo(const std::string& gaia,
 
   return account_id;
 }
+
+void AccountTrackerService::SeedAccountInfo(
+    AccountTrackerService::AccountInfo info) {
+  info.account_id = PickAccountIdForAccount(info.gaia, info.email);
+  if (info.hosted_domain.empty()) {
+    info.hosted_domain = kNoHostedDomainFound;
+  }
+
+  if(info.IsValid()) {
+    if(!ContainsKey(accounts_, info.account_id)) {
+      SeedAccountInfo(info.gaia, info.email);
+    }
+
+    AccountState& state = accounts_[info.account_id];
+    state.info = info;
+    NotifyAccountUpdated(state);
+    SaveToPrefs(state);
+  }
+}

components/signin/core/browser/account_tracker_service.h

@@ -46,6 +46,9 @@ class AccountTrackerService : public KeyedService,
   // Value representing no hosted domain in the kProfileHostedDomain preference.
   static const char kNoHostedDomainFound[];
 
+  // Value representing no picture URL associated with an account.
+  static const char kNoPictureURLFound[];
+
   // Information about a specific account.
   struct AccountInfo {
     AccountInfo();
@@ -58,11 +61,10 @@ class AccountTrackerService : public KeyedService,
     std::string given_name;
     std::string hosted_domain;
     std::string locale;
+    std::string picture_url;
     std::vector<std::string> service_flags;
-    // TODO(rogerta): eventually this structure will include other information
-    // about the account, like full name, profile picture URL, etc.
 
-    bool IsValid();
+    bool IsValid() const;
   };
 
   // Clients of AccountTrackerService can implement this interface and register
@@ -125,6 +127,7 @@ class AccountTrackerService : public KeyedService,
   // value PickAccountIdForAccount() when given the same arguments.
   std::string SeedAccountInfo(const std::string& gaia,
                               const std::string& email);
+  void SeedAccountInfo(AccountInfo info);
 
   AccountIdMigrationState GetMigrationState();
   static AccountIdMigrationState GetMigrationState(PrefService* pref_service);

components/signin/core/browser/account_tracker_service_unittest.cc

@@ -29,7 +29,8 @@ const std::string kTokenInfoResponseFormat =
       \"hd\": \"\",           \
       \"name\": \"%s\",       \
       \"given_name\": \"%s\", \
-      \"locale\": \"%s\"      \
+      \"locale\": \"%s\",     \
+      \"picture\": \"%s\"     \
     }";
 
 const std::string kTokenInfoIncompleteResponseFormat =
@@ -68,6 +69,10 @@ std::string AccountIdToLocale(const std::string account_id) {
   return "locale-" + account_id;
 }
 
+std::string AccountIdToPictureURL(const std::string account_id) {
+  return "picture_url-" + account_id;
+}
+
 void CheckAccountDetails(const std::string account_id,
                          const AccountTrackerService::AccountInfo& info) {
   EXPECT_EQ(account_id, info.account_id);
@@ -291,7 +296,8 @@ class AccountTrackerServiceTest : public testing::Test {
         AccountIdToEmail(account_id).c_str(),
         AccountIdToFullName(account_id).c_str(),
         AccountIdToGivenName(account_id).c_str(),
-        AccountIdToLocale(account_id).c_str());
+        AccountIdToLocale(account_id).c_str(),
+        AccountIdToPictureURL(account_id).c_str());
   }
 
   std::string GenerateIncompleteTokenInfoResponse(