[sync] Automatic bootstrapping of Sync on Win 8 from cached credentials

chrome/browser/signin/signin_manager.cc

Index: chrome/browser/signin/signin_manager.cc
diff --git a/chrome/browser/signin/signin_manager.cc b/chrome/browser/signin/signin_manager.cc
index ef77dee975984dd7dfccedaabc275098230dd0c5..47120b068642d038c17af427c26cf0586cdd10c7 100644
--- a/chrome/browser/signin/signin_manager.cc
+++ b/chrome/browser/signin/signin_manager.cc
@@ -8,9 +8,13 @@
 #include <vector>
 
 #include "base/command_line.h"
+#include "base/memory/ref_counted.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -18,12 +22,15 @@
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/net/gaia/gaia_auth_fetcher.h"
 #include "chrome/common/net/gaia/gaia_constants.h"
 #include "chrome/common/net/gaia/gaia_urls.h"
 #include "chrome/common/pref_names.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "net/cookies/cookie_monster.h"
 #include "unicode/regex.h"
@@ -446,6 +453,24 @@ void SigninManager::OnGetUserInfoSuccess(const UserInfoMap& data) {
     bool isGPlusUser = (iter != services.end());
     profile_->GetPrefs()->SetBoolean(prefs::kIsGooglePlusUser, isGPlusUser);
   }
+
+  ConsumeUserInfoAndFinishSignin();
+
+#if defined(OS_WIN)
+  // On Windows 8, we persist credentials after a successful sign in on Desktop
+  // (or Metro) Chrome so that when the corresponding Metro (or Desktop) Chrome
+  // is subsequently launched, we can automatically bootstrap sync without the
+  // user having to manually sign in to sync all over again.
+  // Note: We currently do this only for the "Default" profile, because there is
+  // no 1:1 correspondence between non-default profiles on Metro and Desktop.
+  if (base::win::GetVersion() >= base::win::VERSION_WIN8 &&
+      csync::CredentialCache::IsDefaultProfileDir(profile_->GetPath())) {
+    PersistAuthTokensAsync();

[Andrew T Wilson (Slow), 2012/06/26 23:26:13]

I wonder if we need to do this, or if it's better/less coupled for some other
component to just listen for GOOGLE_SIGNIN_SUCCESSFUL and write out the
credentials when that happens?

I'm not sure why SigninManager is the right place for this persistence code.

[Raghu Simha, 2012/06/27 00:17:04]

I agree with the premise of your suggestion. It turns out that the
GOOGLE_SIGNIN_SUCCESSFUL notification sends out a username and password, instead
of what we use, which is a username, sid and lsid. Do you think it's preferable
to store the gaia password in an encrypted form? Is that not a bigger concern?
ProfileSyncService doesn't have access to the ClientLoginResult returned to
SigninManager on a successful signin. I'll investigate the approach you
suggested.

[Raghu Simha, 2012/07/19 06:57:07]

Done. All code removed from SigninManager.

+  }
+#endif  // OS_WIN
+}
+
+void SigninManager::ConsumeUserInfoAndFinishSignin() {
   GoogleServiceSigninSuccessDetails details(authenticated_username_,
                                             password_);
   content::NotificationService::current()->Notify(
@@ -461,8 +486,44 @@ void SigninManager::OnGetUserInfoSuccess(const UserInfoMap& data) {
   token_service->StartFetchingTokens();
 }
 
+#if defined(OS_WIN)
+
+void SigninManager::StartSignInWithCachedCredentials(
+    scoped_refptr<csync::CredentialCache> credentials) {

[Roger Tawa OOO till Jul 10th, 2012/06/27 21:23:28]

The order of implementation in the cc file should match the order in the h file,
so this definition should move up.

[Raghu Simha, 2012/07/19 06:57:07]

Moot for this file, but I've followed this rule in other files.

+  DCHECK(credentials.get() != NULL);
+  SetAuthenticatedUsername(credentials->authenticated_username());
+  possibly_invalid_username_.clear();
+  profile_->GetPrefs()->SetString(prefs::kGoogleServicesUsername,
+                                  authenticated_username_);
+  last_result_.sid = credentials->sid();
+  last_result_.lsid = credentials->lsid();
+  password_.clear();

[Andrew T Wilson (Slow), 2012/06/26 23:26:13]

Hmmm. It previously has been illegal to send out SIGNIN_SUCCESSFUL notifications
with an empty password. Are we sure we want to change this?

[Raghu Simha, 2012/06/27 00:17:04]

I had to do this due to a DCHECK. This problem should go away when I add a new
notification type. Coming up.

+
+  TokenService* token_service = TokenServiceFactory::GetForProfile(profile_);
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_TOKEN_AVAILABLE,
+                 content::Source<TokenService>(token_service));

[Roger Tawa OOO till Jul 10th, 2012/06/27 21:23:28]

why do you need to register?  Note that the case for this notif in
SigninManager::Observe() is wrapped in #ifdef !OS_CHROMEOS

[Raghu Simha, 2012/07/19 06:57:07]

We no longer do this.

+
+  ConsumeUserInfoAndFinishSignin();
+}
+
+void SigninManager::PersistAuthTokensAsync() {
+  scoped_refptr<csync::CredentialCache> credentials =
+      new csync::CredentialCache(authenticated_username_,
+                                 last_result_.sid,
+                                 last_result_.lsid,
+                                 "",
+                                 profile_->GetPath());
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&csync::CredentialCache::PersistAuthTokens, credentials));
+}
+
+#endif  // OS_WIN
+
 void SigninManager::OnGetUserInfoFailure(const GoogleServiceAuthError& error) {
-  LOG(ERROR) << "Unable to retreive the canonical email address. Login failed.";
+  LOG(ERROR) << "Unable to retrieve the canonical email address. Login failed.";
   // REVIEW: why does this call OnClientLoginFailure?
   OnClientLoginFailure(error);
 }