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

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/profile_sync_service.h"
 
 #include <cstddef>
 #include <map>
 #include <set>
 #include <utility>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/string16.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
 #include "chrome/browser/about_flags.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/net/chrome_cookie_notification_details.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/browser/sync/backend_migrator.h"
+#include "chrome/browser/sync/credential_cache_service_win.h"
+#include "chrome/browser/sync/credential_cache_service_factory_win.h"
 #include "chrome/browser/sync/glue/change_processor.h"
 #include "chrome/browser/sync/glue/chrome_encryptor.h"
 #include "chrome/browser/sync/glue/chrome_report_unrecoverable_error.h"
 #include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/browser/sync/glue/session_data_type_controller.h"
 #include "chrome/browser/sync/glue/session_model_associator.h"
 #include "chrome/browser/sync/glue/typed_url_data_type_controller.h"
 #include "chrome/browser/sync/profile_sync_components_factory_impl.h"
 #include "chrome/browser/sync/sync_global_error.h"
 #include "chrome/browser/sync/user_selectable_sync_type.h"
@@ skipping 88 matching lines @@
       signin_(signin_manager),
       unrecoverable_error_reason_(ERROR_REASON_UNSET),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       expect_sync_configuration_aborted_(false),
       encrypted_types_(syncer::Cryptographer::SensitiveTypes()),
       encrypt_everything_(false),
       encryption_pending_(false),
       auto_start_enabled_(start_behavior == AUTO_START),
       failed_datatypes_handler_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       configure_status_(DataTypeManager::UNKNOWN),
-      setup_in_progress_(false) {
+      setup_in_progress_(false),
+      credential_cache_service_(NULL) {
 #if defined(OS_ANDROID)
   chrome::VersionInfo version_info;
   if (version_info.IsOfficialBuild()) {
     sync_service_url_ = GURL(kSyncServerUrl);
   }
 #else
   // By default, dev, canary, and unbranded Chromium users will go to the
   // development servers. Development servers have more features than standard
   // sync servers. Users with officially-branded Chrome stable and beta builds
   // will go to the standard sync servers.
@@ skipping 48 matching lines @@
   RegisterAuthNotifications();
 
   if (!HasSyncSetupCompleted() || signin_->GetAuthenticatedUsername().empty()) {
     // Clean up in case of previous crash / setup abort / signout.
     DisableForUser();
   }
 
   TryStart();
 }
 
+bool ProfileSyncService::ShouldInitializeCredentialCacheService() {
+#if defined(OS_WIN)
+  return
+      base::win::GetVersion() >= base::win::VERSION_WIN8 &&

[Andrew T Wilson (Slow), 2012/07/20 01:04:05]

This seems like a non-optimal pattern - why not just have this routine only be
defined on OS_WIN (why do we need it on other platforms at all?).

[Raghu Simha, 2012/07/20 23:35:22]

I did this based on Roger's comment that we shouldn't conditionally define
methods for only some datatypes. But on taking a look around the codebase, I
notice other places where this is done. I've cleaned things up a little bit.

+      syncer::CredentialCacheService::IsDefaultProfileDir(
+          profile()->GetPath()) &&
+      !IsManaged() &&
+      !sync_prefs_.IsStartSuppressed();
+#else
+  return false;
+#endif
+}
+
+bool ProfileSyncService::LoadedCachedCredentials() {
+#if defined(OS_WIN)
+  return (credential_cache_service_ &&
+          credential_cache_service_->SuccessfullyLoadedCredentials());
+#else
+  return false;
+#endif
+}
+
 void ProfileSyncService::TryStart() {
+#if defined(OS_WIN)
+  if (ShouldInitializeCredentialCacheService()) {

[Andrew T Wilson (Slow), 2012/07/20 01:04:05]

I'm not clear on why we need to do this initialization - why doesn't the
CredentialCacheService self-initialize on startup?

[Raghu Simha, 2012/07/20 23:35:22]

At first, I thought we would not initialize the credential cache service if sync
was disabled or suppressed. I suppose this logic lives in the PSS startup code
anyway, so I might as well move the initialization there.

+    // Lazily instantiate the credential cache service if required.
+    credential_cache_service_ =
+        CredentialCacheServiceFactory::GetInstance()->GetForProfile(profile_);
+    if (!setup_in_progress_ && signin_->GetAuthenticatedUsername().empty()) {
+      credential_cache_service_->LookForCachedCredentialsInAlternateProfile();
+    }
+    return;
+  }
+#endif
+
   if (!IsSyncEnabledAndLoggedIn())
     return;
   TokenService* token_service = TokenServiceFactory::GetForProfile(profile_);
   if (!token_service)
     return;
   // Don't start the backend if the token service hasn't finished loading tokens
   // yet (if the backend is started before the sync token has been loaded,
   // GetCredentials() will return bogus credentials). On auto_start platforms
   // (like ChromeOS) we don't start sync until tokens are loaded, because the
   // user can be "signed in" on those platforms long before the tokens get
@@ skipping 467 matching lines @@
   ConsumeCachedPassphraseIfPossible();
 
   // The very first time the backend initializes is effectively the first time
   // we can say we successfully "synced".  last_synced_time_ will only be null
   // in this case, because the pref wasn't restored on StartUp.
   if (last_synced_time_.is_null()) {
     UpdateLastSyncedTime();
   }
   NotifyObservers();
 
-  if (auto_start_enabled_ && !FirstSetupInProgress()) {
-    // Backend is initialized but we're not in sync setup, so this must be an
-    // autostart - mark our sync setup as completed and we'll start syncing
-    // below.
+  if ((auto_start_enabled_ && !FirstSetupInProgress()) ||
+      LoadedCachedCredentials()) {

[Andrew T Wilson (Slow), 2012/07/20 01:04:05]

I think a better way to do this would be for the CCS to set
SyncSetupCompleted=true when it loads cached credentials, then we don't need
this logic here.

[Raghu Simha, 2012/07/20 23:35:22]

Agreed. Done.

+    // If one of following is true, mark setup as completed and start syncing.
+    // 1) Backend is initialized but we're not in sync setup, so this must be an
+    //    autostart.
+    // 2) We are starting up using cached credentials.
     SetSyncSetupCompleted();
     NotifyObservers();
   }
 
   if (HasSyncSetupCompleted()) {
     ConfigureDataTypeManager();
   } else {
     DCHECK(FirstSetupInProgress());
   }
 }
@@ skipping 1050 matching lines @@
   // See http://stackoverflow.com/questions/6224121/is-new-this-myclass-undefined-behaviour-after-directly-calling-the-destru.
   ProfileSyncService* old_this = this;
   this->~ProfileSyncService();
   new(old_this) ProfileSyncService(
       new ProfileSyncComponentsFactoryImpl(profile,
                                            CommandLine::ForCurrentProcess()),
       profile,
       signin,
       behavior);
 }