Merge 174356

> Improved GAIA cookie retrieval logic in ChromeOS login
> 
> * /MergeSession should not really run when we do cookie jar transfer
> * cookie jar transfer should be running just for new users and not be running for users who went through OAuth1 recovery (i.e. pwd change)
> 
> 
> BUG=166919
> TEST=existing ChromeOS integration tests
> 
> Review URL: https://chromiumcodereview.appspot.com/11576065

TBR=zelidrag@chromium.org
Review URL: https://codereview.chromium.org/11946034

git-svn-id: svn://svn.chromium.org/chrome/branches/1364/src@177312 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 206 insertions, 77 deletions

diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index 5b3f775..483528f 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -188,7 +188,7 @@ class LoginUtilsImpl
   LoginUtilsImpl()
       : pending_requests_(false),
         using_oauth_(false),
-        has_cookies_(false),
+        has_web_auth_cookies_(false),
         delegate_(NULL),
         job_restart_request_(NULL),
         should_restore_auth_session_(false),
@@ -232,6 +232,7 @@ class LoginUtilsImpl
       const GaiaAuthConsumer::ClientLoginResult& credentials) OVERRIDE;
   virtual void StopBackgroundFetchers() OVERRIDE;
   virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE;
+  virtual void CompleteProfileCreate(Profile* user_profile) OVERRIDE;
 
   // OAuth1TokenFetcher::Delegate overrides.
   void OnOAuth1AccessTokenAvailable(const std::string& token,
@@ -298,17 +299,28 @@ class LoginUtilsImpl
   // Check user's profile for kApplicationLocale setting.
   void RespectLocalePreference(Profile* pref);
 
+  // Initializes basic preferences for newly created profile.
+  void InitProfilePreferences(Profile* user_profile);
+
   // Callback for asynchronous profile creation.
   void OnProfileCreated(Profile* profile,
                         Profile::CreateStatus status);
 
+  // Finalized profile preparation.
+  void FinalizePrepareProfile(Profile* user_profile);
+
+  // Restores GAIA auth cookies for the created profile.
+  void RestoreAuthCookies(Profile* user_profile);
+
   // Initializes RLZ. If |disabled| is true, financial pings are turned off.
   void InitRlz(Profile* user_profile, bool disabled);
 
   std::string password_;
   bool pending_requests_;
   bool using_oauth_;
-  bool has_cookies_;
+  // True if the authenrication profile's cookie jar should contain
+  // authentication cookies from the authentication extension log in flow.
+  bool has_web_auth_cookies_;
   // Has to be scoped_refptr, see comment for CreateAuthenticator(...).
   scoped_refptr<Authenticator> authenticator_;
   scoped_ptr<PolicyOAuthFetcher> policy_oauth_fetcher_;
@@ -428,7 +440,7 @@ void LoginUtilsImpl::PrepareProfile(
 
   pending_requests_ = pending_requests;
   using_oauth_ = using_oauth;
-  has_cookies_ = has_cookies;
+  has_web_auth_cookies_ = has_cookies;
   delegate_ = delegate;
 
   policy::BrowserPolicyConnector* connector =
@@ -480,36 +492,44 @@ void LoginUtilsImpl::DelegateDeleted(LoginUtils::Delegate* delegate) {
     delegate_ = NULL;
 }
 
+void LoginUtilsImpl::InitProfilePreferences(Profile* user_profile) {
+  if (UserManager::Get()->IsCurrentUserNew())
+    SetFirstLoginPrefs(user_profile->GetPrefs());
+  // Make sure that the google service username is properly set (we do this
+  // on every sign in, not just the first login, to deal with existing
+  // profiles that might not have it set yet).
+  StringPrefMember google_services_username;
+  google_services_username.Init(prefs::kGoogleServicesUsername,
+                                user_profile->GetPrefs());
+  google_services_username.SetValue(
+      UserManager::Get()->GetLoggedInUser()->display_email());
+  // Make sure we flip every profile to not share proxies if the user hasn't
+  // specified so explicitly.
+  const PrefService::Preference* use_shared_proxies_pref =
+      user_profile->GetPrefs()->FindPreference(prefs::kUseSharedProxies);
+  if (use_shared_proxies_pref->IsDefaultValue())
+    user_profile->GetPrefs()->SetBoolean(prefs::kUseSharedProxies, false);
+  policy::NetworkConfigurationUpdater* network_configuration_updater =
+      g_browser_process->browser_policy_connector()->
+      GetNetworkConfigurationUpdater();
+  if (network_configuration_updater)
+    network_configuration_updater->OnUserPolicyInitialized();
+  RespectLocalePreference(user_profile);
+}
+
 void LoginUtilsImpl::OnProfileCreated(
     Profile* user_profile,
     Profile::CreateStatus status) {
   CHECK(user_profile);
+
+  if (delegate_)
+    delegate_->OnProfileCreated(user_profile);
+
   switch (status) {
     case Profile::CREATE_STATUS_INITIALIZED:
       break;
     case Profile::CREATE_STATUS_CREATED: {
-      if (UserManager::Get()->IsCurrentUserNew())
-        SetFirstLoginPrefs(user_profile->GetPrefs());
-      // Make sure that the google service username is properly set (we do this
-      // on every sign in, not just the first login, to deal with existing
-      // profiles that might not have it set yet).
-      StringPrefMember google_services_username;
-      google_services_username.Init(prefs::kGoogleServicesUsername,
-                                    user_profile->GetPrefs());
-      google_services_username.SetValue(
-          UserManager::Get()->GetLoggedInUser()->display_email());
-      // Make sure we flip every profile to not share proxies if the user hasn't
-      // specified so explicitly.
-      const PrefService::Preference* use_shared_proxies_pref =
-          user_profile->GetPrefs()->FindPreference(prefs::kUseSharedProxies);
-      if (use_shared_proxies_pref->IsDefaultValue())
-        user_profile->GetPrefs()->SetBoolean(prefs::kUseSharedProxies, false);
-      policy::NetworkConfigurationUpdater* network_configuration_updater =
-          g_browser_process->browser_policy_connector()->
-          GetNetworkConfigurationUpdater();
-      if (network_configuration_updater)
-        network_configuration_updater->OnUserPolicyInitialized();
-      RespectLocalePreference(user_profile);
+      InitProfilePreferences(user_profile);
       return;
     }
     case Profile::CREATE_STATUS_FAIL:
@@ -532,40 +552,57 @@ void LoginUtilsImpl::OnProfileCreated(
                              policy_oauth_fetcher_->oauth1_secret());
     }
 
-    // Transfer proxy authentication cache and optionally cookies and server
+    // Transfer proxy authentication cache, cookies (optionally) and server
     // bound certs from the profile that was used for authentication.  This
     // profile contains cookies that auth extension should have already put in
     // place that will ensure that the newly created session is authenticated
     // for the websites that work with the used authentication schema.
     ProfileAuthData::Transfer(authenticator_->authentication_profile(),
                               user_profile,
-                              has_cookies_);   // transfer_cookies
-
-    std::string oauth1_token;
-    std::string oauth1_secret;
-    if (ReadOAuth1AccessToken(user_profile, &oauth1_token, &oauth1_secret) ||
-        !has_cookies_) {
-      // Verify OAuth access token when we find it in the profile and always if
-      // if we don't have cookies.
-      // TODO(xiyuan): Change back to use authenticator to verify token when
-      // we support Gaia in lock screen.
-      VerifyOAuth1AccessToken(user_profile, oauth1_token, oauth1_secret);
-    } else {
-      // If we don't have it, fetch OAuth1 access token.
-      // Once we get that, we will kick off individual requests for OAuth2
-      // tokens for all our services.
-      // Use off-the-record profile that was used for this step. It should
-      // already contain all needed cookies that will let us skip GAIA's user
-      // authentication UI.
-      //
-      // TODO(rickcam) We should use an isolated App here.
-      oauth1_token_fetcher_.reset(
-          new OAuth1TokenFetcher(this,
-                                 authenticator_->authentication_profile()));
-      oauth1_token_fetcher_->Start();
-    }
+                              has_web_auth_cookies_,  // transfer_cookies
+                              base::Bind(
+                                  &LoginUtilsImpl::CompleteProfileCreate,
+                                  AsWeakPtr(),
+                                  user_profile));
+    return;
   }
 
+  FinalizePrepareProfile(user_profile);
+}
+
+void LoginUtilsImpl::RestoreAuthCookies(Profile* user_profile) {
+  std::string oauth1_token;
+  std::string oauth1_secret;
+  if (ReadOAuth1AccessToken(user_profile, &oauth1_token, &oauth1_secret) ||
+      !has_web_auth_cookies_) {
+    // Verify OAuth access token when we find it in the profile and always if
+    // if we don't have cookies.
+    // TODO(xiyuan): Change back to use authenticator to verify token when
+    // we support Gaia in lock screen.
+    VerifyOAuth1AccessToken(user_profile, oauth1_token, oauth1_secret);
+  } else {
+    // If we don't have it, fetch OAuth1 access token.
+    // Once we get that, we will kick off individual requests for OAuth2
+    // tokens for all our services.
+    // Use off-the-record profile that was used for this step. It should
+    // already contain all needed cookies that will let us skip GAIA's user
+    // authentication UI.
+    //
+    // TODO(rickcam) We should use an isolated App here.
+    oauth1_token_fetcher_.reset(
+        new OAuth1TokenFetcher(this,
+                               authenticator_->authentication_profile()));
+    oauth1_token_fetcher_->Start();
+  }
+}
+
+void LoginUtilsImpl::CompleteProfileCreate(Profile* user_profile) {
+  RestoreAuthCookies(user_profile);
+  FinalizePrepareProfile(user_profile);
+}
+
+void LoginUtilsImpl::FinalizePrepareProfile(Profile* user_profile) {
+  BootTimesLoader* btl = BootTimesLoader::Get();
   // Own TPM device if, for any reason, it has not been done in EULA
   // wizard screen.
   CryptohomeLibrary* cryptohome = CrosLibrary::Get()->GetCryptohomeLibrary();
@@ -1141,7 +1178,9 @@ void LoginUtilsImpl::OnOAuth1AccessTokenAvailable(const std::string& token,
   Profile* user_profile = ProfileManager::GetDefaultProfile();
   StoreOAuth1AccessToken(user_profile, token, secret);
 
-  // Verify OAuth1 token by doing OAuthLogin and fetching credentials.
+  // Verify OAuth1 token by doing OAuthLogin and fetching credentials. If we
+  // have just transfered auth cookies out of authenticated cookie jar, there
+  // is no need to try to mint them from OAuth token again.
   VerifyOAuth1AccessToken(user_profile, token, secret);
 }
 
diff --git a/chrome/browser/chromeos/login/login_utils.h b/chrome/browser/chromeos/login/login_utils.h
index 90a262c..2e3300a 100644
--- a/chrome/browser/chromeos/login/login_utils.h
+++ b/chrome/browser/chromeos/login/login_utils.h
@@ -29,13 +29,17 @@ class LoginUtils {
  public:
   class Delegate {
    public:
-    // Called after profile is loaded and prepared for the session.
+   // Called after profile is loaded and prepared for the session.
     virtual void OnProfilePrepared(Profile* profile) = 0;
 
 #if defined(ENABLE_RLZ)
     // Called after post-profile RLZ initialization.
     virtual void OnRlzInitialized(Profile* profile) {}
 #endif
+
+    // Called immediately after profile is created, should be used as a test
+    // se
+    virtual void OnProfileCreated(Profile* profile) {}
   };
 
   // Get LoginUtils singleton object. If it was not set before, new default
@@ -116,6 +120,9 @@ class LoginUtils {
   // Initialize RLZ.
   virtual void InitRlzDelayed(Profile* user_profile) = 0;
 
+  // Completed profile creation process.
+  virtual void CompleteProfileCreate(Profile* user_profile) {}
+
  protected:
   friend class ::BrowserGuestSessionNavigatorTest;
 
diff --git a/chrome/browser/chromeos/login/login_utils_browsertest.cc b/chrome/browser/chromeos/login/login_utils_browsertest.cc
index af31e83..11970ed 100644
--- a/chrome/browser/chromeos/login/login_utils_browsertest.cc
+++ b/chrome/browser/chromeos/login/login_utils_browsertest.cc
@@ -144,7 +144,8 @@ class LoginUtilsTest : public testing::Test,
         mock_async_method_caller_(NULL),
         connector_(NULL),
         cryptohome_(NULL),
-        prepared_profile_(NULL) {}
+        prepared_profile_(NULL),
+        created_profile_(NULL) {}
 
   virtual void SetUp() OVERRIDE {
     // This test is not a full blown InProcessBrowserTest, and doesn't have
@@ -354,6 +355,10 @@ class LoginUtilsTest : public testing::Test,
     prepared_profile_ = profile;
   }
 
+  virtual void OnProfileCreated(Profile* profile) OVERRIDE {
+    created_profile_ = profile;
+  }
+
 #if defined(ENABLE_RLZ)
   virtual void OnRlzInitialized(Profile* profile) OVERRIDE {
     rlz_initialized_cb_.Run();
@@ -480,6 +485,7 @@ class LoginUtilsTest : public testing::Test,
   policy::BrowserPolicyConnector* connector_;
   MockCryptohomeLibrary* cryptohome_;
   Profile* prepared_profile_;
+  Profile* created_profile_;
 
   base::Closure rlz_initialized_cb_;
 
@@ -505,6 +511,11 @@ TEST_F(LoginUtilsTest, NormalLoginDoesntBlock) {
   // The profile will be created without waiting for a policy response.
   PrepareProfile(kUsername);
 
+  // This should shortcut cookie transfer step that is missing due to
+  // IO thread being mocked.
+  EXPECT_TRUE(created_profile_);
+  LoginUtils::Get()->CompleteProfileCreate(created_profile_);
+
   EXPECT_TRUE(prepared_profile_);
   ASSERT_TRUE(user_manager->IsUserLoggedIn());
   EXPECT_EQ(kUsername, user_manager->GetLoggedInUser()->email());
@@ -527,6 +538,11 @@ TEST_F(LoginUtilsTest, EnterpriseLoginDoesntBlockForNormalUser) {
   // Login with a non-enterprise user shouldn't block.
   PrepareProfile(kUsernameOtherDomain);
 
+  // This should shortcut cookie transfer step that is missing due to
+  // IO thread being mocked.
+  EXPECT_TRUE(created_profile_);
+  LoginUtils::Get()->CompleteProfileCreate(created_profile_);
+
   EXPECT_TRUE(prepared_profile_);
   ASSERT_TRUE(user_manager->IsUserLoggedIn());
   EXPECT_EQ(kUsernameOtherDomain, user_manager->GetLoggedInUser()->email());
@@ -546,6 +562,11 @@ TEST_F(LoginUtilsTest, RlzInitialized) {
   // Wait for blocking RLZ tasks to complete.
   RunUntilIdle();
 
+  // This should shortcut cookie transfer step that is missing due to
+  // IO thread being mocked.
+  EXPECT_TRUE(created_profile_);
+  LoginUtils::Get()->CompleteProfileCreate(created_profile_);
+
   // RLZ brand code has been set to empty string.
   EXPECT_TRUE(local_state_.Get()->HasPrefPath(prefs::kRLZBrand));
   EXPECT_EQ(std::string(), local_state_.Get()->GetString(prefs::kRLZBrand));
@@ -645,6 +666,11 @@ TEST_P(LoginUtilsBlockingLoginTest, EnterpriseLoginBlocksForEnterpriseUser) {
     fetcher->delegate()->OnURLFetchComplete(fetcher);
   }
 
+  // This should shortcut cookie transfer step that is missing due to
+  // IO thread being mocked.
+  EXPECT_TRUE(created_profile_);
+  LoginUtils::Get()->CompleteProfileCreate(created_profile_);
+
   // The profile is finally ready:
   EXPECT_TRUE(prepared_profile_);
 }
diff --git a/chrome/browser/chromeos/login/profile_auth_data.cc b/chrome/browser/chromeos/login/profile_auth_data.cc
index e6324fd..70e25e0 100644
--- a/chrome/browser/chromeos/login/profile_auth_data.cc
+++ b/chrome/browser/chromeos/login/profile_auth_data.cc
@@ -22,23 +22,64 @@ namespace chromeos {
 
 namespace {
 
+// Callback for transferring |cookies_to_transfer| into |cookie_monster| if
+// its jar is completely empty.
+void OnTransferCookiesIfEmptyJar(
+    net::CookieMonster* cookie_monster,
+    const net::CookieList& cookies_to_transfer,
+    const base::Callback<void()>& cookies_transfered_callback,
+    const net::CookieList& cookies_in_jar) {
+  std::string sid;
+  std::string lsid;
+  // Transfer only if the existing cookie jar is empty.
+  if (!cookies_in_jar.size())
+    cookie_monster->InitializeFrom(cookies_to_transfer);
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE, cookies_transfered_callback);
+  return;
+}
+
+// Callback for receiving |cookies_to_transfer| from the authentication profile
+// cookie jar.
+void OnGetCookiesToTransfer(
+    net::CookieMonster* cookie_monster,
+    const base::Callback<void()>& cookies_transfered_callback,
+    const net::CookieList& cookies_to_transfer) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Nothing to transfer over?
+  if (!cookies_to_transfer.size()) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, cookies_transfered_callback);
+    return;
+  }
+  // Now let's see if the target cookie monster's jar is even empty.
+  cookie_monster->GetAllCookiesAsync(
+      base::Bind(&OnTransferCookiesIfEmptyJar,
+                 make_scoped_refptr(cookie_monster),
+                 cookies_to_transfer,
+                 cookies_transfered_callback));
+}
+
 // Transfers initial set of Profile cookies from the |from_context| to cookie
 // jar of |to_context|.
 void TransferDefaultCookiesOnIOThread(
     net::URLRequestContextGetter* from_context,
-    net::URLRequestContextGetter* to_context) {
+    net::URLRequestContextGetter* to_context,
+    const base::Callback<void()>& cookies_transfered_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  net::CookieStore* new_store =
+  net::CookieStore* to_store =
       to_context->GetURLRequestContext()->cookie_store();
-  net::CookieMonster* new_monster = new_store->GetCookieMonster();
+  net::CookieMonster* to_monster = to_store->GetCookieMonster();
 
-  net::CookieStore* default_store =
+  net::CookieStore* from_store =
       from_context->GetURLRequestContext()->cookie_store();
-  net::CookieMonster* default_monster = default_store->GetCookieMonster();
-  default_monster->SetKeepExpiredCookies();
-  default_monster->GetAllCookiesAsync(
-      base::Bind(base::IgnoreResult(&net::CookieMonster::InitializeFrom),
-                 new_monster));
+  net::CookieMonster* from_monster = from_store->GetCookieMonster();
+  from_monster->SetKeepExpiredCookies();
+  from_monster->GetAllCookiesAsync(base::Bind(&OnGetCookiesToTransfer,
+                                              make_scoped_refptr(to_monster),
+                                              cookies_transfered_callback));
 }
 
 // Transfers default server bound certs of |from_context| to server bound certs
@@ -76,12 +117,14 @@ void TransferDefaultAuthCacheOnIOThread(
 // automatically transfered into the profile.
 void TransferDefaultCookiesAndServerBoundCerts(
     Profile* from_profile,
-    Profile* to_profile) {
+    Profile* to_profile,
+    const base::Callback<void()>& cookies_transfered_callback) {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&TransferDefaultCookiesOnIOThread,
                  make_scoped_refptr(from_profile->GetRequestContext()),
-                 make_scoped_refptr(to_profile->GetRequestContext())));
+                 make_scoped_refptr(to_profile->GetRequestContext()),
+                 cookies_transfered_callback));
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&TransferDefaultServerBoundCertsIOThread,
@@ -104,12 +147,20 @@ void TransferDefaultAuthCache(Profile* from_profile,
 
 }  // namespace
 
-void ProfileAuthData::Transfer(Profile* from_profile,
-                               Profile* to_profile,
-                               bool transfer_cookies) {
+void ProfileAuthData::Transfer(
+    Profile* from_profile,
+    Profile* to_profile,
+    bool transfer_cookies,
+    const base::Callback<void()>& cookies_transfered_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (transfer_cookies)
-    TransferDefaultCookiesAndServerBoundCerts(from_profile, to_profile);
+  if (transfer_cookies) {
+    TransferDefaultCookiesAndServerBoundCerts(from_profile,
+                                              to_profile,
+                                              cookies_transfered_callback);
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, cookies_transfered_callback);
+  }
 
   TransferDefaultAuthCache(from_profile, to_profile);
 }
diff --git a/chrome/browser/chromeos/login/profile_auth_data.h b/chrome/browser/chromeos/login/profile_auth_data.h
index 67e0f28..cd4a0fa 100644
--- a/chrome/browser/chromeos/login/profile_auth_data.h
+++ b/chrome/browser/chromeos/login/profile_auth_data.h
@@ -2,10 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROMEOS_PROFILE_AUTH_DATA_H_
-#define CHROME_BROWSER_CHROMEOS_PROFILE_AUTH_DATA_H_
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_PROFILE_AUTH_DATA_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_PROFILE_AUTH_DATA_H_
 
-#include "base/memory/ref_counted.h"
+#include <string>
+#include "base/callback.h"
 
 class Profile;
 
@@ -17,13 +18,18 @@ class ProfileAuthData {
  public:
   // Transfers proxy authentication cache and optionally |transfer_cookies| and
   // server bound certs from the profile that was used for authentication.
-  static void Transfer(Profile* from_profile,
-                       Profile* to_profile,
-                       bool transfer_cookies);
+  // |cookies_transfered_callback| will be called on UI thread after cookie
+  // transfer part of this operation is completed.
+  static void Transfer(
+      Profile* from_profile,
+      Profile* to_profile,
+      bool transfer_cookies,
+      const base::Callback<void()>& cookies_transfered_callback);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(ProfileAuthData);
 };
 
 }  // namespace chromeos
 
-#endif  // CHROME_BROWSER_CHROMEOS_PROFILE_AUTH_DATA_H_
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_PROFILE_AUTH_DATA_H_