Re-land 151364 - [sync] Auto-create credential cache for users who are already...

Re-land 151364 - [sync] Auto-create credential cache for users who are already signed in and go on to upgrade Chrome

Originally reverted due to crashes in GetLastUpdatedTime(). See crbug.com/143214. Turns out 151364 wasn't the root cause. Feature has been disabled via switch on canary. Root cause fix is under review in a separate patch. Re-landing 151364.

Original description:

CredentialCacheService on Windows 8 writes to the local credential cache
when a user actively signs in / reconfigures sync. Existing cached
credentials are not updated when Chrome is merely restarted after the
user is signed in.

This causes a problem when a user is already signed in, and then
upgrades (and restarts) chrome from a version that didn't originally
support credential caching. In such cases, we never end up caching
credentials, and therefore, the user will have to sign in separately to
Metro and Desktop.

This patch contains the following changes:

1) Adds logic to auto-heal already-signed-in users who upgrade from
older versions, by writing existing credentials to the local cache if
during restart, we notice that there is no local cache file, and the
user is already signed in to sync.

2) Simplifies the logic around checking if an alternate credential cache
file exists, and only then initializing |alternate_store_|. It turns out
that JsonPrefStore returns a useful PrefReadError field, and there is no
need for CCS to do funky stuff on the FILE thread.

3) Simplifies OnInitialzationCompleted, which was being used to observe
two separate JsonPrefStores. Instead of having CCS be a
PrefStore::Observer, we now use two helper classes -- LocalStoreObserver
and AlternateStoreObserver to cleanly divide what is done when each pref
store is initialized.

4) Updates prefs::kGoogleServicesUsername by listening to the notifications
NOTIFICATION_GOOGLE_SIGNED_OUT and NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL
instead of directly listening to the pref change.

5) Fixes a stray instance where we were accessing the gaia username
pref via SyncPrefs instead of via the SigninManager.

BUG=141555,143214
TEST=Sign in to chrome, exit the browser, and delete "Sync Credentials" from the default profile directory. Restart Chrome and make sure that the credential cache file is freshly written using existing sync credentials.

Original review URL: https://chromiumcodereview.appspot.com/10830239
First revert review URL: https://chromiumcodereview.appspot.com/10830362
TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10834395

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152122 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/signin/token_service.h

@@ -177,6 +177,10 @@ class TokenService : public GaiaAuthConsumer,
   void IssueAuthTokenForTest(const std::string& service,
                              const std::string& auth_token);
 
+  const GaiaAuthConsumer::ClientLoginResult& credentials() const {
+    return credentials_;
+  }
+
   // GaiaAuthConsumer implementation.
   virtual void OnIssueAuthTokenSuccess(const std::string& service,
                                        const std::string& auth_token) OVERRIDE;

chrome/browser/sync/credential_cache_service_win.h

@@ -39,8 +39,7 @@ namespace syncer {
 // sync using credentials that were cached due to signing in on the other
 // (alternate) mode.
 class CredentialCacheService : public ProfileKeyedService,
-                               public content::NotificationObserver,
-                               public PrefStore::Observer {
+                               public content::NotificationObserver {
  public:
   explicit CredentialCacheService(Profile* profile);
   virtual ~CredentialCacheService();
@@ -48,15 +47,26 @@ class CredentialCacheService : public ProfileKeyedService,
   // ProfileKeyedService implementation.
   virtual void Shutdown() OVERRIDE;
 
-  // PrefStore::Observer implementation.
-  virtual void OnInitializationCompleted(bool succeeded) OVERRIDE;
-  virtual void OnPrefValueChanged(const std::string& key) OVERRIDE;
-
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // Loads cached sync credentials from the alternate profile and applies them
+  // to the local profile if the load was successful.
+  void ReadCachedCredentialsFromAlternateProfile();
+
+  // Populates a new local credential cache file if the user is already signed
+  // in to the local profile, and there is no existing local credential cache.
+  // Used in scenarios where a user upgraded from an older version of Chrome
+  // that didn't support credential caching. This method is a no-op if local
+  // sync prefs have already been written to the local cache.
+  void WriteExistingSyncPrefsToLocalCache();
+
+  // Resets |alternate_store_| and schedules the next read from the alternate
+  // credential cache.
+  void ScheduleNextReadFromAlternateCredentialCache();
+
  protected:
   // Returns true if the credential cache represented by |store| contains a
   // value for |pref_name|.
@@ -111,6 +121,50 @@ class CredentialCacheService : public ProfileKeyedService,
   }
 
  private:
+  // Used to track the initialization of the local credential cache.
+  class LocalStoreObserver
+      : public base::RefCounted<LocalStoreObserver>,
+        public PrefStore::Observer {
+   public:
+    LocalStoreObserver(CredentialCacheService* service,
+                       scoped_refptr<JsonPrefStore> local_store);
+    virtual ~LocalStoreObserver();
+
+    // PrefStore::Observer implementation.
+    virtual void OnInitializationCompleted(bool succeeded) OVERRIDE;
+    virtual void OnPrefValueChanged(const std::string& key) OVERRIDE;
+
+   protected:
+    friend class base::RefCounted<LocalStoreObserver>;
+
+   private:
+    CredentialCacheService* service_;
+    scoped_refptr<JsonPrefStore> local_store_;
+    DISALLOW_COPY_AND_ASSIGN(LocalStoreObserver);
+  };
+
+  // Used to track the initialization of the alternate credential cache.
+  class AlternateStoreObserver
+      : public base::RefCounted<AlternateStoreObserver>,
+        public PrefStore::Observer  {
+   public:
+    AlternateStoreObserver(CredentialCacheService* service,
+                           scoped_refptr<JsonPrefStore> alternate_store);
+    virtual ~AlternateStoreObserver();
+
+    // PrefStore::Observer implementation.
+    virtual void OnInitializationCompleted(bool succeeded) OVERRIDE;
+    virtual void OnPrefValueChanged(const std::string& key) OVERRIDE;
+
+   protected:
+    friend class base::RefCounted<AlternateStoreObserver>;
+
+   private:
+    CredentialCacheService* service_;
+    scoped_refptr<JsonPrefStore> alternate_store_;
+    DISALLOW_COPY_AND_ASSIGN(AlternateStoreObserver);
+  };
+
   // Returns the path to the sync credentials file in the current profile
   // directory.
   FilePath GetCredentialPathInCurrentProfile() const;
@@ -124,20 +178,9 @@ class CredentialCacheService : public ProfileKeyedService,
   // writer must be initialized, and false if not.
   bool ShouldInitializeLocalCredentialCacheWriter() const;
 
-  // Determines if we must look for credentials in the alternate profile, based
-  // on relevant sync preferences, in addition the to conditions in
-  // ShouldInitializeLocalCredentialCacheWriter(). Returns true if we must look
-  // for cached credentials, and false if not.
-  bool ShouldLookForCachedCredentialsInAlternateProfile() const;
-
   // Initializes the JsonPrefStore object for the local profile directory.
   void InitializeLocalCredentialCacheWriter();
 
-  // Initializes the JsonPrefStore object for the alternate profile directory
-  // if |should_initialize| is true. We take a bool* instead of a bool since
-  // this is a callback, and base::Owned needs to clean up the flag.
-  void InitializeAlternateCredentialCacheReader(bool* should_initialize);
-
   // Returns true if there is an empty value for kGoogleServicesUsername in the
   // credential cache for the local profile (indicating that the user first
   // signed in and then signed out). Returns false if there's no value at all
@@ -151,10 +194,6 @@ class CredentialCacheService : public ProfileKeyedService,
   // cannot auto-start.
   void LookForCachedCredentialsInAlternateProfile();
 
-  // Loads cached sync credentials from the alternate profile and calls
-  // ApplyCachedCredentials if the load was successful.
-  void ReadCachedCredentialsFromAlternateProfile();
-
   // Initiates sync sign in using credentials read from the alternate profile by
   // persisting |google_services_username|, |encryption_bootstrap_token|,
   // |keep_everything_synced| and |preferred_types| to the local pref store, and
@@ -211,10 +250,6 @@ class CredentialCacheService : public ProfileKeyedService,
                           const std::string& sid,
                           const std::string& encryption_bootstrap_token);
 
-  // Resets |alternate_store_| and schedules the next read from the alternate
-  // credential cache.
-  void ScheduleNextReadFromAlternateCredentialCache();
-
   // Profile for which credentials are being cached.
   Profile* profile_;
 
@@ -226,10 +261,16 @@ class CredentialCacheService : public ProfileKeyedService,
   // it can be accessed by unit tests.
   scoped_refptr<JsonPrefStore> local_store_;
 
+  // Used to respond to the initialization of |local_store_|.
+  scoped_refptr<LocalStoreObserver> local_store_observer_;
+
   // Used for read operations on the credential cache file in the alternate
   // profile directory. This is separate from the chrome pref store.
   scoped_refptr<JsonPrefStore> alternate_store_;
 
+  // Used to respond to the initialization of |alternate_store_|.
+  scoped_refptr<AlternateStoreObserver> alternate_store_observer_;
+
   // Registrar for notifications from the PrefService.
   PrefChangeRegistrar pref_registrar_;
 

chrome/browser/sync/credential_cache_service_win_unittest.cc

@@ -39,9 +39,6 @@ class CredentialCacheServiceTest : public CredentialCacheService,
     file_message_loop_.RunAllPending();
   }
 
-  // PrefStore::Observer implementation.
-  virtual void OnInitializationCompleted(bool succeeded) OVERRIDE {}
-
  private:
   ScopedTempDir temp_dir_;
   MessageLoop file_message_loop_;