Sync: Plumb IdentityManager through to ProfileSyncService

This CL plumbs the IdentityManager through to PSS, via SigninManagerWrapper.
There are no behavioral changes, in particular the IdentityManager isn't
used at all yet. Uses will follow soon (https://crrev.com/c/983914).

This does require a small test change to make the newly-instantiated
IdentityManager happy (without that change, it complains that its view of
the primary account isn't consistent with SigninManager's).

Bug: 825190
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I8b313f5572339a2c1319725fa4012b4a41365e08
Reviewed-on: https://chromium-review.googlesource.com/995414
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548419}

chrome/browser/sync/profile_sync_service_factory.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/signin/about_signin_internals_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/spellchecker/spellcheck_factory.h"
@@ -137,6 +138,7 @@ ProfileSyncServiceFactory::ProfileSyncServiceFactory()
   DependsOn(ThemeServiceFactory::GetInstance());
 #endif  // !defined(OS_ANDROID)
   DependsOn(HistoryServiceFactory::GetInstance());
+  DependsOn(IdentityManagerFactory::GetInstance());
   DependsOn(invalidation::ProfileInvalidationProviderFactory::GetInstance());
   DependsOn(PasswordStoreFactory::GetInstance());
   DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
@@ -218,10 +220,6 @@ KeyedService* ProfileSyncServiceFactory::BuildServiceInstanceFor(
 #endif  // defined(OS_WIN)
 
   if (!local_sync_backend_enabled) {
-    SigninManagerBase* signin = SigninManagerFactory::GetForProfile(profile);
-    SigninClient* signin_client =
-        ChromeSigninClientFactory::GetForProfile(profile);
-
     // Always create the GCMProfileService instance such that we can listen to
     // the profile notifications and purge the GCM store when the profile is
     // being signed out.
@@ -232,13 +230,15 @@ KeyedService* ProfileSyncServiceFactory::BuildServiceInstanceFor(
     AboutSigninInternalsFactory::GetForProfile(profile);
 
     init_params.signin_wrapper =
-        std::make_unique<SupervisedUserSigninManagerWrapper>(profile, signin);
+        std::make_unique<SupervisedUserSigninManagerWrapper>(
+            profile, IdentityManagerFactory::GetForProfile(profile),
+            SigninManagerFactory::GetForProfile(profile));
     // Note: base::Unretained(signin_client) is safe because the SigninClient is
     // guaranteed to outlive the PSS, per a DependsOn() above (and because PSS
     // clears the callback in its Shutdown()).
-    init_params.signin_scoped_device_id_callback =
-        base::BindRepeating(&SigninClient::GetSigninScopedDeviceId,
-                            base::Unretained(signin_client));
+    init_params.signin_scoped_device_id_callback = base::BindRepeating(
+        &SigninClient::GetSigninScopedDeviceId,
+        base::Unretained(ChromeSigninClientFactory::GetForProfile(profile)));
     init_params.oauth2_token_service =
         ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
     init_params.gaia_cookie_manager_service =

chrome/browser/sync/profile_sync_test_util.cc

@@ -11,6 +11,7 @@
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/chrome_sync_client.h"
@@ -45,6 +46,7 @@ ProfileSyncService::InitParams CreateProfileSyncServiceParamsForTest(
   ProfileSyncService::InitParams init_params;
 
   init_params.signin_wrapper = std::make_unique<SigninManagerWrapper>(
+      IdentityManagerFactory::GetForProfile(profile),
       SigninManagerFactory::GetForProfile(profile));
   init_params.signin_scoped_device_id_callback =
       base::BindRepeating([]() { return std::string(); });

chrome/browser/sync/supervised_user_signin_manager_wrapper.cc

@@ -15,8 +15,10 @@
 
 SupervisedUserSigninManagerWrapper::SupervisedUserSigninManagerWrapper(
     Profile* profile,
-    SigninManagerBase* original)
-    : SigninManagerWrapper(original), profile_(profile) {}
+    identity::IdentityManager* identity_manager,
+    SigninManagerBase* signin_manager)
+    : SigninManagerWrapper(identity_manager, signin_manager),
+      profile_(profile) {}
 
 SupervisedUserSigninManagerWrapper::~SupervisedUserSigninManagerWrapper() {}
 

chrome/browser/sync/supervised_user_signin_manager_wrapper.h

@@ -13,6 +13,10 @@
 class Profile;
 class SigninManagerBase;
 
+namespace identity {
+class IdentityManager;
+}
+
 // Some chrome cloud services support supervised users as well as normally
 // authenticated users that sign in through SigninManager.  To facilitate
 // getting the "effective" username and account identifiers, services can
@@ -20,8 +24,10 @@ class SigninManagerBase;
 // information when appropriate.
 class SupervisedUserSigninManagerWrapper : public SigninManagerWrapper {
  public:
-  SupervisedUserSigninManagerWrapper(Profile* profile,
-                                     SigninManagerBase* original);
+  SupervisedUserSigninManagerWrapper(
+      Profile* profile,
+      identity::IdentityManager* identity_manager,
+      SigninManagerBase* signin_manager);
   ~SupervisedUserSigninManagerWrapper() override;
 
   // SigninManagerWrapper implementation

components/browser_sync/BUILD.gn

@@ -43,6 +43,7 @@ static_library("browser_sync") {
     "//components/version_info:generate_version_info",
     "//google_apis",
     "//net",
+    "//services/identity/public/cpp",
   ]
 }
 
@@ -126,6 +127,7 @@ static_library("test_support") {
     "//components/sync_sessions:test_support",
     "//google_apis",
     "//net:test_support",
+    "//services/identity/public/cpp",
     "//testing/gmock",
   ]
 }

components/browser_sync/DEPS

@@ -23,4 +23,5 @@ include_rules = [
   "+components/webdata_services",
   "+google_apis",
   "+net",
+  "+services/identity/public/cpp",
 ]

components/browser_sync/profile_sync_service.cc

@@ -338,13 +338,13 @@ void ProfileSyncService::RegisterAuthNotifications() {
   DCHECK(thread_checker_.CalledOnValidThread());
   oauth2_token_service_->AddObserver(this);
   if (signin_)
-    signin_->GetOriginal()->AddObserver(this);
+    signin_->GetSigninManager()->AddObserver(this);
 }
 
 void ProfileSyncService::UnregisterAuthNotifications() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (signin_)
-    signin_->GetOriginal()->RemoveObserver(this);
+    signin_->GetSigninManager()->RemoveObserver(this);
   if (oauth2_token_service_)
     oauth2_token_service_->RemoveObserver(this);
 }
@@ -1116,7 +1116,7 @@ void ProfileSyncService::OnActionableError(const SyncProtocolError& error) {
       // On every platform except ChromeOS, sign out the user after a dashboard
       // clear.
       if (!IsLocalSyncEnabled()) {
-        SigninManager::FromSigninManagerBase(signin_->GetOriginal())
+        SigninManager::FromSigninManagerBase(signin_->GetSigninManager())
             ->SignOut(signin_metrics::SERVER_FORCED_DISABLE,
                       signin_metrics::SignoutDelete::IGNORE_METRIC);
       }

components/browser_sync/profile_sync_service_unittest.cc

@@ -191,10 +191,14 @@ class ProfileSyncServiceTest : public ::testing::Test {
     std::string account_id =
         account_tracker()->SeedAccountInfo(kGaiaId, kEmail);
     auth_service()->UpdateCredentials(account_id, "oauth2_login_token");
+#if defined(OS_CHROMEOS)
+    signin_manager()->SignIn(account_id);
+#else
+    signin_manager()->SignIn(kGaiaId, kEmail, "password");
+#endif
   }
 
   void CreateService(ProfileSyncService::StartBehavior behavior) {
-    signin_manager()->SetAuthenticatedAccountInfo(kGaiaId, kEmail);
     component_factory_ = profile_sync_service_bundle_.component_factory();
     ProfileSyncServiceBundle::SyncClientBuilder builder(
         &profile_sync_service_bundle_);
@@ -318,9 +322,9 @@ class ProfileSyncServiceTest : public ::testing::Test {
   }
 
 #if defined(OS_CHROMEOS)
-  SigninManagerBase* signin_manager()
+  FakeSigninManagerBase* signin_manager()
 #else
-  SigninManager* signin_manager()
+  FakeSigninManager* signin_manager()
 #endif
   // Opening brace is outside of macro to avoid confusing lint.
   {

components/browser_sync/profile_sync_test_util.cc

@@ -233,6 +233,7 @@ ProfileSyncServiceBundle::ProfileSyncServiceBundle()
                       &account_tracker_,
                       nullptr),
 #endif
+      identity_manager_(&signin_manager_, &auth_service_),
       url_request_context_(new net::TestURLRequestContextGetter(
           base::ThreadTaskRunnerHandle::Get())) {
   RegisterPrefsForProfileSyncService(pref_service_.registry());
@@ -250,8 +251,8 @@ ProfileSyncService::InitParams ProfileSyncServiceBundle::CreateBasicInitParams(
 
   init_params.start_behavior = start_behavior;
   init_params.sync_client = std::move(sync_client);
-  init_params.signin_wrapper =
-      std::make_unique<SigninManagerWrapper>(signin_manager());
+  init_params.signin_wrapper = std::make_unique<SigninManagerWrapper>(
+      identity_manager(), signin_manager());
   init_params.signin_scoped_device_id_callback =
       base::BindRepeating([]() { return std::string(); });
   init_params.oauth2_token_service = auth_service();

components/browser_sync/profile_sync_test_util.h

@@ -21,6 +21,7 @@
 #include "components/sync/driver/sync_api_component_factory_mock.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/sync_sessions/mock_sync_sessions_client.h"
+#include "services/identity/public/cpp/identity_manager.h"
 
 namespace history {
 class HistoryService;
@@ -132,6 +133,8 @@ class ProfileSyncServiceBundle {
 
   FakeSigninManagerType* signin_manager() { return &signin_manager_; }
 
+  identity::IdentityManager* identity_manager() { return &identity_manager_; }
+
   AccountTrackerService* account_tracker() { return &account_tracker_; }
 
   syncer::SyncApiComponentFactoryMock* component_factory() {
@@ -160,6 +163,7 @@ class ProfileSyncServiceBundle {
   AccountTrackerService account_tracker_;
   FakeSigninManagerType signin_manager_;
   FakeProfileOAuth2TokenService auth_service_;
+  identity::IdentityManager identity_manager_;
   syncer::SyncApiComponentFactoryMock component_factory_;
   testing::NiceMock<sync_sessions::MockSyncSessionsClient>
       sync_sessions_client_;

components/sync/BUILD.gn

@@ -561,6 +561,7 @@ jumbo_static_library("sync") {
     "//components/version_info",
     "//crypto",
     "//google_apis",
+    "//services/identity/public/cpp",
     "//sql",
     "//third_party/cacheinvalidation",
     "//third_party/crc32c",

components/sync/driver/DEPS

@@ -19,4 +19,5 @@ include_rules = [
   "+google/cacheinvalidation",
   "+net",
   "+policy",
+  "+services/identity/public/cpp",
 ]

components/sync/driver/signin_manager_wrapper.cc

@@ -7,21 +7,27 @@
 #include "components/signin/core/browser/signin_manager_base.h"
 #include "google_apis/gaia/gaia_constants.h"
 
-SigninManagerWrapper::SigninManagerWrapper(SigninManagerBase* original)
-    : original_(original) {}
+SigninManagerWrapper::SigninManagerWrapper(
+    identity::IdentityManager* identity_manager,
+    SigninManagerBase* signin_manager)
+    : identity_manager_(identity_manager), signin_manager_(signin_manager) {}
 
 SigninManagerWrapper::~SigninManagerWrapper() {}
 
-SigninManagerBase* SigninManagerWrapper::GetOriginal() {
-  return original_;
+identity::IdentityManager* SigninManagerWrapper::GetIdentityManager() {
+  return identity_manager_;
+}
+
+SigninManagerBase* SigninManagerWrapper::GetSigninManager() {
+  return signin_manager_;
 }
 
 std::string SigninManagerWrapper::GetEffectiveUsername() const {
-  return original_->GetAuthenticatedAccountInfo().email;
+  return signin_manager_->GetAuthenticatedAccountInfo().email;
 }
 
 std::string SigninManagerWrapper::GetAccountIdToUse() const {
-  return original_->GetAuthenticatedAccountId();
+  return signin_manager_->GetAuthenticatedAccountId();
 }
 
 std::string SigninManagerWrapper::GetSyncScopeToUse() const {

components/sync/driver/signin_manager_wrapper.h

@@ -11,13 +11,18 @@
 
 class SigninManagerBase;
 
+namespace identity {
+class IdentityManager;
+}
+
 // Wraps SigninManager so subclasses can support different ways of getting
 // account information if necessary. Currently exists for supervised users;
 // the subclass SupervisedUserSigninManagerWrapper may be merged back into
 // this class once supervised users are componentized.
 class SigninManagerWrapper {
  public:
-  explicit SigninManagerWrapper(SigninManagerBase* original);
+  explicit SigninManagerWrapper(identity::IdentityManager* identity_manager,
+                                SigninManagerBase* signin_manager);
   virtual ~SigninManagerWrapper();
 
   // Get the email address to use for this account.
@@ -29,11 +34,15 @@ class SigninManagerWrapper {
   // Get the OAuth2 scope to use for this account.
   virtual std::string GetSyncScopeToUse() const;
 
+  // Return the original IdentityManager object that was passed in.
+  identity::IdentityManager* GetIdentityManager();
+
   // Return the original SigninManagerBase object that was passed in.
-  SigninManagerBase* GetOriginal();
+  SigninManagerBase* GetSigninManager();
 
  private:
-  SigninManagerBase* original_;
+  identity::IdentityManager* identity_manager_;
+  SigninManagerBase* signin_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(SigninManagerWrapper);
 };

components/sync/driver/sync_service_base.cc

@@ -85,7 +85,7 @@ bool SyncServiceBase::HasObserver(const SyncServiceObserver* observer) const {
 
 AccountInfo SyncServiceBase::GetAuthenticatedAccountInfo() const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return signin_ ? signin_->GetOriginal()->GetAuthenticatedAccountInfo()
+  return signin_ ? signin_->GetSigninManager()->GetAuthenticatedAccountInfo()
                  : AccountInfo();
 }
 

ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.cc

@@ -29,6 +29,7 @@
 #include "ios/chrome/browser/services/gcm/ios_chrome_gcm_profile_service_factory.h"
 #include "ios/chrome/browser/sessions/ios_chrome_tab_restore_service_factory.h"
 #include "ios/chrome/browser/signin/about_signin_internals_factory.h"
+#include "ios/chrome/browser/signin/identity_manager_factory.h"
 #include "ios/chrome/browser/signin/oauth2_token_service_factory.h"
 #include "ios/chrome/browser/signin/signin_client_factory.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
@@ -105,6 +106,7 @@ IOSChromeProfileSyncServiceFactory::IOSChromeProfileSyncServiceFactory()
   DependsOn(ios::SigninManagerFactory::GetInstance());
   DependsOn(ios::TemplateURLServiceFactory::GetInstance());
   DependsOn(ios::WebDataServiceFactory::GetInstance());
+  DependsOn(IdentityManagerFactory::GetInstance());
   DependsOn(IOSChromeGCMProfileServiceFactory::GetInstance());
   DependsOn(IOSChromePasswordStoreFactory::GetInstance());
   DependsOn(IOSChromeProfileInvalidationProviderFactory::GetInstance());
@@ -121,11 +123,6 @@ IOSChromeProfileSyncServiceFactory::BuildServiceInstanceFor(
   ios::ChromeBrowserState* browser_state =
       ios::ChromeBrowserState::FromBrowserState(context);
 
-  SigninManagerBase* signin =
-      ios::SigninManagerFactory::GetForBrowserState(browser_state);
-  SigninClient* signin_client =
-      SigninClientFactory::GetForBrowserState(browser_state);
-
   // Always create the GCMProfileService instance such that we can listen to
   // the profile notifications and purge the GCM store when the profile is
   // being signed out.
@@ -136,9 +133,12 @@ IOSChromeProfileSyncServiceFactory::BuildServiceInstanceFor(
   ios::AboutSigninInternalsFactory::GetForBrowserState(browser_state);
 
   ProfileSyncService::InitParams init_params;
-  init_params.signin_wrapper = std::make_unique<SigninManagerWrapper>(signin);
+  init_params.signin_wrapper = std::make_unique<SigninManagerWrapper>(
+      IdentityManagerFactory::GetForBrowserState(browser_state),
+      ios::SigninManagerFactory::GetForBrowserState(browser_state));
   init_params.signin_scoped_device_id_callback = base::BindRepeating(
-      &SigninClient::GetSigninScopedDeviceId, base::Unretained(signin_client));
+      &SigninClient::GetSigninScopedDeviceId,
+      base::Unretained(SigninClientFactory::GetForBrowserState(browser_state)));
   init_params.oauth2_token_service =
       OAuth2TokenServiceFactory::GetForBrowserState(browser_state);
   init_params.start_behavior = ProfileSyncService::MANUAL_START;

ios/chrome/browser/sync/ios_chrome_profile_sync_test_util.cc

@@ -15,6 +15,7 @@
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/sync/driver/signin_manager_wrapper.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#include "ios/chrome/browser/signin/identity_manager_factory.h"
 #include "ios/chrome/browser/signin/oauth2_token_service_factory.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 #include "ios/chrome/browser/sync/ios_chrome_sync_client.h"
@@ -27,6 +28,7 @@ CreateProfileSyncServiceParamsForTest(
   browser_sync::ProfileSyncService::InitParams init_params;
 
   init_params.signin_wrapper = std::make_unique<SigninManagerWrapper>(
+      IdentityManagerFactory::GetForBrowserState(browser_state),
       ios::SigninManagerFactory::GetForBrowserState(browser_state));
   init_params.signin_scoped_device_id_callback =
       base::BindRepeating([]() { return std::string(); });