Eliminate SigninManager as public interface

After recent changes, SigninManager exposes no public API surfaces
over SigninManagerBase. This CL eliminates nearly all usage of
signin_manager.h and in particular eliminates the no-longer needed
SigninManager::FromSigninManagerBase() method.

The remaining usage is for sites that *construct* SigninManager
instances. These sites will need to remain as such until SigninManager
is actually merged into SigninManagerBase.

This is a step in eliminating the inheritance relationship between
SigninManager and SigninManagerBase, as motivated in this design doc:

https://docs.google.com/document/d/15y-Db27BV08vrIyelHB-3CwiAfDYh-FigNKGXrqSWto/edit#heading=h.8jjdy95t6p7x

and described concretely here:

https://docs.google.com/document/d/15y-Db27BV08vrIyelHB-3CwiAfDYh-FigNKGXrqSWto/edit#heading=h.mbkrv9nkb93w

TBR=rohitrao@chromium.org

Bug: 952766, 883648
Change-Id: Icb114b11dc2c74d8b49cfd77baba33b707a4157e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565316
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665898}

chrome/browser/android/signin/signin_manager_android.cc

@@ -155,9 +155,8 @@ void SigninManagerAndroid::OnSignInCompleted(
 
   // TODO(crbug.com/889902): Migrate to IdentityManager once there's an
   // API mapping for SigninManager::SignIn().
-  SigninManager::FromSigninManagerBase(
-      IdentityManagerFactory::GetForProfile(profile_)->GetSigninManager())
-      ->SignIn(base::android::ConvertJavaStringToUTF8(env, username));
+  IdentityManagerFactory::GetForProfile(profile_)->GetSigninManager()->SignIn(
+      base::android::ConvertJavaStringToUTF8(env, username));
 }
 
 void SigninManagerAndroid::SignOut(JNIEnv* env,

chrome/browser/signin/identity_manager_factory.cc

@@ -38,24 +38,16 @@
 
 namespace {
 
-#if !defined(OS_CHROMEOS)
-using ConcreteSigninManager = SigninManager;
-#else
-using ConcreteSigninManager = SigninManagerBase;
-#endif
-
 // Helper function returning a newly constructed PrimaryAccountMutator for
 // |profile|.  May return null if mutation of the signed-in state is not
 // supported on the current platform.
 std::unique_ptr<identity::PrimaryAccountMutator> BuildPrimaryAccountMutator(
     Profile* profile,
     AccountTrackerService* account_tracker_service,
-    ConcreteSigninManager* signin_manager) {
+    SigninManagerBase* signin_manager) {
 #if !defined(OS_CHROMEOS)
   return std::make_unique<identity::PrimaryAccountMutatorImpl>(
-      account_tracker_service,
-      SigninManager::FromSigninManagerBase(signin_manager),
-      profile->GetPrefs());
+      account_tracker_service, signin_manager, profile->GetPrefs());
 #else
   return nullptr;
 #endif
@@ -78,19 +70,19 @@ std::unique_ptr<identity::AccountsMutator> BuildAccountsMutator(
 #endif
 }
 
-std::unique_ptr<ConcreteSigninManager> BuildSigninManager(
+std::unique_ptr<SigninManagerBase> BuildSigninManager(
     Profile* profile,
     AccountTrackerService* account_tracker_service,
     ProfileOAuth2TokenService* token_service) {
-  std::unique_ptr<ConcreteSigninManager> signin_manager;
+  std::unique_ptr<SigninManagerBase> signin_manager;
   SigninClient* client =
       ChromeSigninClientFactory::GetInstance()->GetForProfile(profile);
 #if defined(OS_CHROMEOS)
-  signin_manager = std::make_unique<ConcreteSigninManager>(
+  signin_manager = std::make_unique<SigninManagerBase>(
       client, token_service, account_tracker_service,
       AccountConsistencyModeManager::GetMethodForProfile(profile));
 #else
-  signin_manager = std::make_unique<ConcreteSigninManager>(
+  signin_manager = std::make_unique<SigninManager>(
       client, token_service, account_tracker_service,
       AccountConsistencyModeManager::GetMethodForProfile(profile));
 #endif
@@ -187,7 +179,7 @@ KeyedService* IdentityManagerFactory::BuildServiceInstanceFor(
   auto gaia_cookie_manager_service = std::make_unique<GaiaCookieManagerService>(
       token_service.get(), ChromeSigninClientFactory::GetForProfile(profile));
 
-  std::unique_ptr<ConcreteSigninManager> signin_manager = BuildSigninManager(
+  std::unique_ptr<SigninManagerBase> signin_manager = BuildSigninManager(
       profile, account_tracker_service.get(), token_service.get());
 
   std::unique_ptr<identity::PrimaryAccountMutator> primary_account_mutator =

components/signin/core/browser/account_tracker_service.cc

@@ -24,7 +24,7 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/signin/core/browser/account_info_util.h"
-#include "components/signin/core/browser/signin_manager.h"
+#include "components/signin/core/browser/signin_manager_base.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 
 #if defined(OS_ANDROID)

components/signin/core/browser/signin_manager.cc

@@ -104,12 +104,6 @@ void SigninManager::OnSigninAllowedPrefChanged() {
   }
 }
 
-// static
-SigninManager* SigninManager::FromSigninManagerBase(
-    SigninManagerBase* manager) {
-  return static_cast<SigninManager*>(manager);
-}
-
 bool SigninManager::IsAllowedUsername(const std::string& username) const {
   const PrefService* local_state = local_state_pref_registrar_.prefs();
   if (!local_state)

components/signin/core/browser/signin_manager.h

@@ -52,11 +52,6 @@ class SigninManager : public SigninManagerBase,
                 signin::AccountConsistencyMethod account_consistency);
   ~SigninManager() override;
 
-  // Returns |manager| as a SigninManager instance. Relies on the fact that on
-  // platforms where signin_manager.* is built, all SigninManagerBase instances
-  // are actually SigninManager instances.
-  static SigninManager* FromSigninManagerBase(SigninManagerBase* manager);
-
   // On platforms where SigninManager is responsible for dealing with
   // invalid username policy updates, we need to check this during
   // initialization and sign the user out.

components/signin/core/browser/signin_manager_base.h

@@ -72,8 +72,8 @@ class SigninManagerBase {
 // invariant that any SigninManagerBase object can be cast to a
 // SigninManager object when not on ChromeOS. Make the constructor private
 // and add SigninManager as a friend to support this.
-// TODO(883648): Eliminate the need to downcast SigninManagerBase to
-// SigninManager and then eliminate this as well.
+// TODO(952766): Eliminate this once SigninManager and SigninManagerBase are
+// merged.
 #if !defined(OS_CHROMEOS)
  private:
 #endif

ios/chrome/browser/signin/identity_manager_factory.cc

@@ -60,7 +60,7 @@ std::unique_ptr<AccountFetcherService> BuildAccountFetcherService(
   return account_fetcher_service;
 }
 
-std::unique_ptr<SigninManager> BuildSigninManager(
+std::unique_ptr<SigninManagerBase> BuildSigninManager(
     ios::ChromeBrowserState* chrome_browser_state,
     AccountTrackerService* account_tracker_service,
     ProfileOAuth2TokenService* token_service) {
@@ -139,7 +139,7 @@ std::unique_ptr<KeyedService> IdentityManagerFactory::BuildServiceInstanceFor(
       token_service.get(),
       SigninClientFactory::GetForBrowserState(browser_state));
 
-  std::unique_ptr<SigninManager> signin_manager = BuildSigninManager(
+  std::unique_ptr<SigninManagerBase> signin_manager = BuildSigninManager(
       browser_state, account_tracker_service.get(), token_service.get());
 
   auto primary_account_mutator =

ios/web_view/internal/signin/web_view_identity_manager_factory.mm

@@ -71,7 +71,7 @@ std::unique_ptr<AccountFetcherService> BuildAccountFetcherService(
   return account_fetcher_service;
 }
 
-std::unique_ptr<SigninManager> BuildSigninManager(
+std::unique_ptr<SigninManagerBase> BuildSigninManager(
     WebViewBrowserState* browser_state,
     AccountTrackerService* account_tracker_service,
     ProfileOAuth2TokenService* token_service) {
@@ -141,7 +141,7 @@ WebViewIdentityManagerFactory::BuildServiceInstanceFor(
       token_service.get(),
       WebViewSigninClientFactory::GetForBrowserState(browser_state));
 
-  std::unique_ptr<SigninManager> signin_manager = BuildSigninManager(
+  std::unique_ptr<SigninManagerBase> signin_manager = BuildSigninManager(
       browser_state, account_tracker_service.get(), token_service.get());
 
   auto primary_account_mutator =

services/identity/public/cpp/identity_manager.h

@@ -22,10 +22,6 @@
 #include "services/identity/public/cpp/accounts_in_cookie_jar_info.h"
 #include "services/identity/public/cpp/scope_set.h"
 
-#if !defined(OS_CHROMEOS)
-#include "components/signin/core/browser/signin_manager.h"
-#endif
-
 #if defined(OS_ANDROID)
 #include "base/android/jni_android.h"
 #endif

services/identity/public/cpp/identity_test_environment.cc

@@ -198,13 +198,12 @@ IdentityTestEnvironment::BuildIdentityManagerForTests(
       signin_client, token_service.get(), account_tracker_service.get(),
       std::make_unique<image_fetcher::FakeImageDecoder>());
 
-#if defined(OS_CHROMEOS)
   std::unique_ptr<SigninManagerBase> signin_manager =
+#if defined(OS_CHROMEOS)
       std::make_unique<SigninManagerBase>(signin_client, token_service.get(),
                                           account_tracker_service.get(),
                                           account_consistency);
 #else
-  std::unique_ptr<SigninManagerBase> signin_manager =
       std::make_unique<SigninManager>(signin_client, token_service.get(),
                                       account_tracker_service.get(),
                                       account_consistency);
@@ -220,8 +219,7 @@ IdentityTestEnvironment::BuildIdentityManagerForTests(
 
 #if !defined(OS_CHROMEOS)
   primary_account_mutator = std::make_unique<PrimaryAccountMutatorImpl>(
-      account_tracker_service.get(),
-      static_cast<SigninManager*>(signin_manager.get()), pref_service);
+      account_tracker_service.get(), signin_manager.get(), pref_service);
 #endif
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)

services/identity/public/cpp/identity_test_utils.cc

@@ -101,9 +101,7 @@ CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
   // test-only API.
   identity_manager->LegacySetPrimaryAccount(gaia_id, email);
 #else
-  SigninManager* real_signin_manager =
-      SigninManager::FromSigninManagerBase(signin_manager);
-  real_signin_manager->SignIn(email);
+  signin_manager->SignIn(email);
 #endif
 
   DCHECK(signin_manager->IsAuthenticated());
@@ -163,8 +161,7 @@ void ClearPrimaryAccount(IdentityManager* identity_manager,
   TestIdentityManagerObserver signout_observer(identity_manager);
   signout_observer.SetOnPrimaryAccountClearedCallback(run_loop.QuitClosure());
 
-  SigninManager* real_signin_manager = SigninManager::FromSigninManagerBase(
-      identity_manager->GetSigninManager());
+  SigninManagerBase* signin_manager = identity_manager->GetSigninManager();
   signin_metrics::ProfileSignout signout_source_metric =
       signin_metrics::SIGNOUT_TEST;
   signin_metrics::SignoutDelete signout_delete_metric =
@@ -172,15 +169,14 @@ void ClearPrimaryAccount(IdentityManager* identity_manager,
 
   switch (policy) {
     case ClearPrimaryAccountPolicy::DEFAULT:
-      real_signin_manager->SignOut(signout_source_metric,
-                                   signout_delete_metric);
+      signin_manager->SignOut(signout_source_metric, signout_delete_metric);
       break;
     case ClearPrimaryAccountPolicy::KEEP_ALL_ACCOUNTS:
-      real_signin_manager->SignOutAndKeepAllAccounts(signout_source_metric,
+      signin_manager->SignOutAndKeepAllAccounts(signout_source_metric,
                                                 signout_delete_metric);
       break;
     case ClearPrimaryAccountPolicy::REMOVE_ALL_ACCOUNTS:
-      real_signin_manager->SignOutAndRemoveAllAccounts(signout_source_metric,
+      signin_manager->SignOutAndRemoveAllAccounts(signout_source_metric,
                                                   signout_delete_metric);
       break;
   }

services/identity/public/cpp/primary_account_mutator_impl.cc

@@ -8,14 +8,14 @@
 
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/account_tracker_service.h"
-#include "components/signin/core/browser/signin_manager.h"
+#include "components/signin/core/browser/signin_manager_base.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 
 namespace identity {
 
 PrimaryAccountMutatorImpl::PrimaryAccountMutatorImpl(
     AccountTrackerService* account_tracker,
-    SigninManager* signin_manager,
+    SigninManagerBase* signin_manager,
     PrefService* pref_service)
     : account_tracker_(account_tracker),
       signin_manager_(signin_manager),

services/identity/public/cpp/primary_account_mutator_impl.h

@@ -9,7 +9,7 @@
 
 class AccountTrackerService;
 class PrefService;
-class SigninManager;
+class SigninManagerBase;
 
 namespace identity {
 
@@ -18,7 +18,7 @@ namespace identity {
 class PrimaryAccountMutatorImpl : public PrimaryAccountMutator {
  public:
   PrimaryAccountMutatorImpl(AccountTrackerService* account_tracker,
-                            SigninManager* signin_manager,
+                            SigninManagerBase* signin_manager,
                             PrefService* pref_service);
   ~PrimaryAccountMutatorImpl() override;
 
@@ -33,7 +33,7 @@ class PrimaryAccountMutatorImpl : public PrimaryAccountMutator {
   // Pointers to the services used by the PrimaryAccountMutatorImpl. They
   // *must* outlive this instance.
   AccountTrackerService* account_tracker_ = nullptr;
-  SigninManager* signin_manager_ = nullptr;
+  SigninManagerBase* signin_manager_ = nullptr;
   PrefService* pref_service_ = nullptr;
 };