Migrate off CoreAccountId for gaia IDs

This affects the experimental codepath used to retrieve and store
encryption keys. The keys are associated to a user as determined by
identity servers, and using CoreAccountId for such IDs does not work
well on all platforms (namely, ChromeOS).

Bug: 1000146
Change-Id: Iaaab6f1867ac99c026e3474822e86ca5e80f5404
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863803
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706424}

components/sync/driver/profile_sync_service.cc

@@ -867,8 +867,7 @@ void ProfileSyncService::OnEngineInitialized(
           initial_types, debug_info_listener, &data_type_controllers_,
           user_settings_.get(), engine_.get(), this);
 
-  crypto_.SetSyncEngine(GetAuthenticatedAccountInfo().account_id,
-                        engine_.get());
+  crypto_.SetSyncEngine(GetAuthenticatedAccountInfo(), engine_.get());
 
   // Auto-start means IsFirstSetupComplete gets set automatically.
   if (start_behavior_ == AUTO_START &&

components/sync/driver/sync_service_crypto.cc

@@ -33,12 +33,12 @@ class EmptyTrustedVaultClient : public TrustedVaultClient {
 
   // TrustedVaultClient implementatio.
   void FetchKeys(
-      const CoreAccountId& account_id,
+      const std::string& gaia_id,
       base::OnceCallback<void(const std::vector<std::string>&)> cb) override {
     std::move(cb).Run({});
   }
 
-  void StoreKeys(const CoreAccountId& account_id,
+  void StoreKeys(const std::string& gaia_id,
                  const std::vector<std::string>& keys) override {}
 };
 
@@ -317,11 +317,11 @@ bool SyncServiceCrypto::SetDecryptionPassphrase(const std::string& passphrase) {
 }
 
 void SyncServiceCrypto::AddTrustedVaultDecryptionKeys(
-    const CoreAccountId& account_id,
+    const std::string& gaia_id,
     const std::vector<std::string>& keys) {
-  trusted_vault_client_->StoreKeys(account_id, keys);
+  trusted_vault_client_->StoreKeys(gaia_id, keys);
 
-  if (state_.engine && state_.account_id == account_id) {
+  if (state_.engine && state_.account_info.gaia == gaia_id) {
     state_.engine->AddTrustedVaultDecryptionKeys(keys, base::DoNothing());
   }
 }
@@ -398,7 +398,7 @@ void SyncServiceCrypto::OnTrustedVaultKeyRequired() {
   state_.required_user_action = RequiredUserAction::kFetchingTrustedVaultKeys;
 
   trusted_vault_client_->FetchKeys(
-      state_.account_id,
+      state_.account_info.gaia,
       base::BindOnce(&SyncServiceCrypto::TrustedVaultKeysFetched,
                      weak_factory_.GetWeakPtr()));
 }
@@ -477,10 +477,10 @@ void SyncServiceCrypto::OnPassphraseTypeChanged(PassphraseType type,
   notify_observers_.Run();
 }
 
-void SyncServiceCrypto::SetSyncEngine(const CoreAccountId& account_id,
+void SyncServiceCrypto::SetSyncEngine(const CoreAccountInfo& account_info,
                                       SyncEngine* engine) {
   DCHECK(engine);
-  state_.account_id = account_id;
+  state_.account_info = account_info;
   state_.engine = engine;
 }
 

components/sync/driver/sync_service_crypto.h

@@ -12,11 +12,11 @@
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
+#include "components/signin/public/identity_manager/account_info.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/engine/configure_reason.h"
 #include "components/sync/engine/sync_encryption_handler.h"
 #include "components/sync/engine/sync_engine.h"
-#include "google_apis/gaia/core_account_id.h"
 
 namespace syncer {
 
@@ -49,7 +49,7 @@ class SyncServiceCrypto : public SyncEncryptionHandler::Observer {
   bool IsEncryptEverythingEnabled() const;
   void SetEncryptionPassphrase(const std::string& passphrase);
   bool SetDecryptionPassphrase(const std::string& passphrase);
-  void AddTrustedVaultDecryptionKeys(const CoreAccountId& account_id,
+  void AddTrustedVaultDecryptionKeys(const std::string& gaia_id,
                                      const std::vector<std::string>& keys);
 
   // Returns the actual passphrase type being used for encryption.
@@ -77,7 +77,7 @@ class SyncServiceCrypto : public SyncEncryptionHandler::Observer {
                                base::Time passphrase_time) override;
 
   // Used to provide the engine when it is initialized.
-  void SetSyncEngine(const CoreAccountId& account_id, SyncEngine* engine);
+  void SetSyncEngine(const CoreAccountInfo& account_info, SyncEngine* engine);
 
   // Creates a proxy observer object that will post calls to this thread.
   std::unique_ptr<SyncEncryptionHandler::Observer> GetEncryptionObserverProxy();
@@ -126,7 +126,7 @@ class SyncServiceCrypto : public SyncEncryptionHandler::Observer {
     SyncEngine* engine = nullptr;
 
     // Populated when the engine is initialized.
-    CoreAccountId account_id;
+    CoreAccountInfo account_info;
 
     RequiredUserAction required_user_action = RequiredUserAction::kNone;
 

components/sync/driver/sync_service_crypto_unittest.cc

@@ -39,6 +39,12 @@ sync_pb::EncryptedData MakeEncryptedData(
   return encrypted;
 }
 
+CoreAccountInfo MakeAccountInfoWithGaia(const std::string& gaia) {
+  CoreAccountInfo result;
+  result.gaia = gaia;
+  return result;
+}
+
 class MockCryptoSyncPrefs : public CryptoSyncPrefs {
  public:
   MockCryptoSyncPrefs() = default;
@@ -57,10 +63,10 @@ class MockTrustedVaultClient : public TrustedVaultClient {
 
   MOCK_METHOD2(
       FetchKeys,
-      void(const CoreAccountId& account_id,
+      void(const std::string& gaia_id,
            base::OnceCallback<void(const std::vector<std::string>&)> cb));
   MOCK_METHOD2(StoreKeys,
-               void(const CoreAccountId& account_id,
+               void(const std::string& gaia_id,
                     const std::vector<std::string>& keys));
 };
 
@@ -95,7 +101,7 @@ class SyncServiceCryptoTest : public testing::Test {
 TEST_F(SyncServiceCryptoTest, ShouldExposePassphraseRequired) {
   const std::string kTestPassphrase = "somepassphrase";
 
-  crypto_.SetSyncEngine(CoreAccountId(), &engine_);
+  crypto_.SetSyncEngine(CoreAccountInfo(), &engine_);
   ASSERT_FALSE(crypto_.IsPassphraseRequired());
 
   // Mimic the engine determining that a passphrase is required.
@@ -126,45 +132,48 @@ TEST_F(SyncServiceCryptoTest, ShouldExposePassphraseRequired) {
 
 TEST_F(SyncServiceCryptoTest,
        ShouldStoreTrustedVaultKeysBeforeEngineInitialization) {
-  const CoreAccountId kAccount = CoreAccountId("account1");
+  const std::string kAccount = "account1";
   const std::vector<std::string> kKeys = {"key1"};
-  EXPECT_CALL(trusted_vault_client_, StoreKeys(kAccount, kKeys));
+  EXPECT_CALL(trusted_vault_client_, StoreKeys("account1", kKeys));
   crypto_.AddTrustedVaultDecryptionKeys(kAccount, kKeys);
 }
 
 TEST_F(SyncServiceCryptoTest,
        ShouldStoreTrustedVaultKeysAfterEngineInitialization) {
-  const CoreAccountId kSyncingAccount = CoreAccountId("syncingaccount");
-  const CoreAccountId kOtherAccount = CoreAccountId("otheraccount");
+  const CoreAccountInfo kSyncingAccount =
+      MakeAccountInfoWithGaia("syncingaccount");
+  const CoreAccountInfo kOtherAccount = MakeAccountInfoWithGaia("otheraccount");
   const std::vector<std::string> kSyncingAccountKeys = {"key1"};
   const std::vector<std::string> kOtherAccountKeys = {"key2"};
 
   crypto_.SetSyncEngine(kSyncingAccount, &engine_);
 
   EXPECT_CALL(trusted_vault_client_,
-              StoreKeys(kOtherAccount, kOtherAccountKeys));
+              StoreKeys(kOtherAccount.gaia, kOtherAccountKeys));
   EXPECT_CALL(trusted_vault_client_,
-              StoreKeys(kSyncingAccount, kSyncingAccountKeys));
+              StoreKeys(kSyncingAccount.gaia, kSyncingAccountKeys));
 
   // Only the sync-ing account should be propagated to the engine.
   EXPECT_CALL(engine_, AddTrustedVaultDecryptionKeys(kOtherAccountKeys, _))
       .Times(0);
   EXPECT_CALL(engine_, AddTrustedVaultDecryptionKeys(kSyncingAccountKeys, _));
-  crypto_.AddTrustedVaultDecryptionKeys(kOtherAccount, kOtherAccountKeys);
-  crypto_.AddTrustedVaultDecryptionKeys(kSyncingAccount, kSyncingAccountKeys);
+  crypto_.AddTrustedVaultDecryptionKeys(kOtherAccount.gaia, kOtherAccountKeys);
+  crypto_.AddTrustedVaultDecryptionKeys(kSyncingAccount.gaia,
+                                        kSyncingAccountKeys);
 }
 
 TEST_F(SyncServiceCryptoTest, ShouldReadValidTrustedVaultKeysFromClient) {
-  const CoreAccountId kSyncingAccount = CoreAccountId("syncingaccount");
+  const CoreAccountInfo kSyncingAccount =
+      MakeAccountInfoWithGaia("syncingaccount");
   const std::vector<std::string> kFetchedKeys = {"key1"};
 
   EXPECT_CALL(reconfigure_cb_, Run(_)).Times(0);
   ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequired());
 
   base::OnceCallback<void(const std::vector<std::string>&)> fetch_keys_cb;
-  EXPECT_CALL(trusted_vault_client_, FetchKeys(kSyncingAccount, _))
+  EXPECT_CALL(trusted_vault_client_, FetchKeys(kSyncingAccount.gaia, _))
       .WillOnce(
-          [&](const CoreAccountId& account_id,
+          [&](const std::string& gaia_id,
               base::OnceCallback<void(const std::vector<std::string>&)> cb) {
             fetch_keys_cb = std::move(cb);
           });
@@ -198,15 +207,16 @@ TEST_F(SyncServiceCryptoTest, ShouldReadValidTrustedVaultKeysFromClient) {
 }
 
 TEST_F(SyncServiceCryptoTest, ShouldReadInvalidTrustedVaultKeysFromClient) {
-  const CoreAccountId kSyncingAccount = CoreAccountId("syncingaccount");
+  const CoreAccountInfo kSyncingAccount =
+      MakeAccountInfoWithGaia("syncingaccount");
   const std::vector<std::string> kFetchedKeys = {"key1"};
 
   ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequired());
 
   base::OnceCallback<void(const std::vector<std::string>&)> fetch_keys_cb;
-  EXPECT_CALL(trusted_vault_client_, FetchKeys(kSyncingAccount, _))
+  EXPECT_CALL(trusted_vault_client_, FetchKeys(kSyncingAccount.gaia, _))
       .WillOnce(
-          [&](const CoreAccountId& account_id,
+          [&](const std::string& gaia_id,
               base::OnceCallback<void(const std::vector<std::string>&)> cb) {
             fetch_keys_cb = std::move(cb);
           });

components/sync/driver/sync_user_settings.h

@@ -15,8 +15,6 @@
 #include "components/sync/base/user_selectable_type.h"
 #include "components/sync/driver/data_type_encryption_handler.h"
 
-struct CoreAccountId;
-
 namespace syncer {
 
 // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser
@@ -120,7 +118,7 @@ class SyncUserSettings : public syncer::DataTypeEncryptionHandler {
   // TRUSTED_VAULT_PASSPHRASE: it provides new decryption keys that could
   // allow decrypting pending Nigori keys.
   virtual void AddTrustedVaultDecryptionKeys(
-      const CoreAccountId& account_id,
+      const std::string& gaia_id,
       const std::vector<std::string>& keys) = 0;
 };
 

components/sync/driver/sync_user_settings_impl.cc

@@ -171,10 +171,10 @@ bool SyncUserSettingsImpl::SetDecryptionPassphrase(
 }
 
 void SyncUserSettingsImpl::AddTrustedVaultDecryptionKeys(
-    const CoreAccountId& account_id,
+    const std::string& gaia_id,
     const std::vector<std::string>& keys) {
   DVLOG(1) << "Adding trusted vault decryption keys.";
-  crypto_->AddTrustedVaultDecryptionKeys(account_id, keys);
+  crypto_->AddTrustedVaultDecryptionKeys(gaia_id, keys);
 }
 
 void SyncUserSettingsImpl::SetSyncRequestedIfNotSetExplicitly() {

components/sync/driver/sync_user_settings_impl.h

@@ -14,8 +14,6 @@
 #include "components/sync/driver/sync_type_preference_provider.h"
 #include "components/sync/driver/sync_user_settings.h"
 
-struct CoreAccountId;
-
 namespace syncer {
 
 class SyncPrefs;
@@ -65,7 +63,7 @@ class SyncUserSettingsImpl : public SyncUserSettings {
   void SetEncryptionPassphrase(const std::string& passphrase) override;
   bool SetDecryptionPassphrase(const std::string& passphrase) override;
   void AddTrustedVaultDecryptionKeys(
-      const CoreAccountId& account_id,
+      const std::string& gaia_id,
       const std::vector<std::string>& keys) override;
 
   void SetSyncRequestedIfNotSetExplicitly();

components/sync/driver/sync_user_settings_mock.h

@@ -9,7 +9,6 @@
 #include <vector>
 
 #include "components/sync/driver/sync_user_settings.h"
-#include "google_apis/gaia/core_account_id.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace syncer {
@@ -49,7 +48,7 @@ class SyncUserSettingsMock : public SyncUserSettings {
   MOCK_METHOD1(SetEncryptionPassphrase, void(const std::string&));
   MOCK_METHOD1(SetDecryptionPassphrase, bool(const std::string&));
   MOCK_METHOD2(AddTrustedVaultDecryptionKeys,
-               void(const CoreAccountId&, const std::vector<std::string>&));
+               void(const std::string&, const std::vector<std::string>&));
 };
 
 }  // namespace syncer

components/sync/driver/test_sync_user_settings.cc

@@ -153,7 +153,7 @@ bool TestSyncUserSettings::SetDecryptionPassphrase(
 }
 
 void TestSyncUserSettings::AddTrustedVaultDecryptionKeys(
-    const CoreAccountId& account_id,
+    const std::string& gaia_id,
     const std::vector<std::string>& keys) {}
 
 void TestSyncUserSettings::SetFirstSetupComplete() {

components/sync/driver/test_sync_user_settings.h

@@ -10,8 +10,6 @@
 
 #include "components/sync/driver/sync_user_settings.h"
 
-struct CoreAccountId;
-
 namespace syncer {
 
 class TestSyncService;
@@ -54,7 +52,7 @@ class TestSyncUserSettings : public SyncUserSettings {
   void SetEncryptionPassphrase(const std::string& passphrase) override;
   bool SetDecryptionPassphrase(const std::string& passphrase) override;
   void AddTrustedVaultDecryptionKeys(
-      const CoreAccountId& account_id,
+      const std::string& gaia_id,
       const std::vector<std::string>& keys) override;
 
   void SetFirstSetupComplete();

components/sync/driver/trusted_vault_client.h

@@ -11,8 +11,6 @@
 #include "base/callback_forward.h"
 #include "base/macros.h"
 
-struct CoreAccountId;
-
 namespace syncer {
 
 // Interface that allows platform-specific logic related to accessing locally
@@ -26,14 +24,14 @@ class TrustedVaultClient {
   // Implementations are expected to NOT prompt the user for actions. |cb| is
   // called on completion with known keys or an empty list if none known.
   virtual void FetchKeys(
-      const CoreAccountId& account_id,
+      const std::string& gaia_id,
       base::OnceCallback<void(const std::vector<std::string>&)> cb) = 0;
 
   // Allows implementations to store encryption keys fetched by other means such
   // as Web interactions. Implementations are free to completely ignore these
   // keys, so callers may not assume that later calls to FetchKeys() would
   // necessarily return the keys passed here.
-  virtual void StoreKeys(const CoreAccountId& account_id,
+  virtual void StoreKeys(const std::string& gaia_id,
                          const std::vector<std::string>& keys) = 0;
 
  private: