[s13n] Convert AccountInvestigator away from SiginManager

Ideally, it should be possible to use the IdentityTestEnvironment
ctor that takes only the boolean paramater, and constructs the
signin and token handler objects internally. But some tests
tweak the PrefService instance, which is not exposed by
IdentityTestEnvironment APIs.

Bots are true but mac_chromium_rel_gn, which is sick. Adding "notry".

NOTRY=true

BUG=908519

Change-Id: I6b4c9076c61079b45da76960c5269fd5ca8397a8
Reviewed-on: https://chromium-review.googlesource.com/c/1351152
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611106}

chrome/browser/signin/account_investigator_factory.cc

@@ -7,12 +7,12 @@
 #include "base/memory/singleton.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service_factory.h"
 #include "components/signin/core/browser/account_investigator.h"
-#include "components/signin/core/browser/signin_manager.h"
+#include "services/identity/public/cpp/identity_manager.h"
 
 // static
 AccountInvestigatorFactory* AccountInvestigatorFactory::GetInstance() {
@@ -31,7 +31,7 @@ AccountInvestigatorFactory::AccountInvestigatorFactory()
           "AccountInvestigator",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(GaiaCookieManagerServiceFactory::GetInstance());
-  DependsOn(SigninManagerFactory::GetInstance());
+  DependsOn(IdentityManagerFactory::GetInstance());
 }
 
 AccountInvestigatorFactory::~AccountInvestigatorFactory() {}
@@ -41,7 +41,7 @@ KeyedService* AccountInvestigatorFactory::BuildServiceInstanceFor(
   Profile* profile(Profile::FromBrowserContext(context));
   AccountInvestigator* investigator = new AccountInvestigator(
       GaiaCookieManagerServiceFactory::GetForProfile(profile),
-      profile->GetPrefs(), SigninManagerFactory::GetForProfile(profile));
+      profile->GetPrefs(), IdentityManagerFactory::GetForProfile(profile));
   investigator->Initialize();
   return investigator;
 }

components/signin/core/browser/account_investigator.cc

@@ -13,11 +13,11 @@
 #include "base/time/time.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
-#include "components/signin/core/browser/signin_manager_base.h"
 #include "components/signin/core/browser/signin_metrics.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/google_service_auth_error.h"
+#include "services/identity/public/cpp/identity_manager.h"
 
 using base::Time;
 using base::TimeDelta;
@@ -44,10 +44,10 @@ const TimeDelta AccountInvestigator::kPeriodicReportingInterval =
 AccountInvestigator::AccountInvestigator(
     GaiaCookieManagerService* cookie_service,
     PrefService* pref_service,
-    SigninManagerBase* signin_manager)
+    identity::IdentityManager* identity_manager)
     : cookie_service_(cookie_service),
       pref_service_(pref_service),
-      signin_manager_(signin_manager) {}
+      identity_manager_(identity_manager) {}
 
 AccountInvestigator::~AccountInvestigator() {}
 
@@ -60,7 +60,7 @@ void AccountInvestigator::RegisterPrefs(PrefRegistrySimple* registry) {
 
 void AccountInvestigator::Initialize() {
   cookie_service_->AddObserver(this);
-  previously_authenticated_ = signin_manager_->IsAuthenticated();
+  previously_authenticated_ = identity_manager_->HasPrimaryAccount();
 
   Time previous = Time::FromDoubleT(
       pref_service_->GetDouble(prefs::kGaiaCookiePeriodicReportTime));
@@ -102,7 +102,7 @@ void AccountInvestigator::OnGaiaAccountsInCookieUpdated(
   const std::string old_hash(pref_service_->GetString(prefs::kGaiaCookieHash));
   const std::string new_hash(
       HashAccounts(signed_in_accounts, signed_out_accounts));
-  const bool currently_authenticated = signin_manager_->IsAuthenticated();
+  const bool currently_authenticated = identity_manager_->HasPrimaryAccount();
   if (old_hash != new_hash) {
     SharedCookieJarReport(signed_in_accounts, signed_out_accounts, Time::Now(),
                           ReportingType::ON_CHANGE);
@@ -239,7 +239,7 @@ void AccountInvestigator::SharedCookieJarReport(
                                      signed_out_count,
                                      signed_in_count + signed_out_count, type);
 
-  if (signin_manager_->IsAuthenticated()) {
+  if (identity_manager_->HasPrimaryAccount()) {
     SignedInAccountRelationReport(signed_in_accounts, signed_out_accounts,
                                   type);
   }
@@ -255,7 +255,7 @@ void AccountInvestigator::SignedInAccountRelationReport(
     const std::vector<ListedAccount>& signed_out_accounts,
     ReportingType type) {
   signin_metrics::LogAccountRelation(
-      DiscernRelation(signin_manager_->GetAuthenticatedAccountInfo(),
+      DiscernRelation(identity_manager_->GetPrimaryAccountInfo(),
                       signed_in_accounts, signed_out_accounts),
       type);
 }

components/signin/core/browser/account_investigator.h

@@ -17,12 +17,15 @@ struct AccountInfo;
 class GaiaCookieManagerService;
 class PrefRegistrySimple;
 class PrefService;
-class SigninManagerBase;
 
 namespace base {
 class Time;
 }  // namespace base
 
+namespace identity {
+class IdentityManager;
+}
+
 namespace signin_metrics {
 enum class AccountRelation;
 enum class ReportingType;
@@ -42,7 +45,7 @@ class AccountInvestigator : public KeyedService,
 
   AccountInvestigator(GaiaCookieManagerService* cookie_service,
                       PrefService* pref_service,
-                      SigninManagerBase* signin_manager);
+                      identity::IdentityManager* identity_manager);
   ~AccountInvestigator() override;
 
   static void RegisterPrefs(PrefRegistrySimple* registry);
@@ -113,7 +116,7 @@ class AccountInvestigator : public KeyedService,
 
   GaiaCookieManagerService* cookie_service_;
   PrefService* pref_service_;
-  SigninManagerBase* signin_manager_;
+  identity::IdentityManager* identity_manager_;
 
   // Handles invoking our periodic logic at the right time. As part of our
   // handling of this call we reset the timer for the next loop.

components/signin/core/browser/account_investigator_unittest.cc

@@ -16,12 +16,15 @@
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/fake_gaia_cookie_manager_service.h"
 #include "components/signin/core/browser/fake_signin_manager.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_metrics.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "google_apis/gaia/fake_oauth2_token_service_delegate.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/google_service_auth_error.h"
+#include "services/identity/public/cpp/identity_test_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::HistogramTester;
@@ -35,15 +38,20 @@ class AccountInvestigatorTest : public testing::Test {
  protected:
   AccountInvestigatorTest()
       : signin_client_(&prefs_),
+        token_service_(&prefs_,
+                       std::make_unique<FakeOAuth2TokenServiceDelegate>()),
         signin_manager_(&signin_client_,
-                        nullptr,
+                        &token_service_,
                         &account_tracker_service_,
                         nullptr),
-        gaia_cookie_manager_service_(nullptr,
-                                     &signin_client_),
+        gaia_cookie_manager_service_(&token_service_, &signin_client_),
+        identity_test_env_(&account_tracker_service_,
+                           &token_service_,
+                           &signin_manager_,
+                           &gaia_cookie_manager_service_),
         investigator_(&gaia_cookie_manager_service_,
                       &prefs_,
-                      &signin_manager_) {
+                      identity_test_env_.identity_manager()) {
     AccountTrackerService::RegisterPrefs(prefs_.registry());
     AccountInvestigator::RegisterPrefs(prefs_.registry());
     SigninManagerBase::RegisterProfilePrefs(prefs_.registry());
@@ -53,6 +61,9 @@ class AccountInvestigatorTest : public testing::Test {
   ~AccountInvestigatorTest() override { investigator_.Shutdown(); }
 
   FakeSigninManager* signin_manager() { return &signin_manager_; }
+  identity::IdentityTestEnvironment* identity_test_env() {
+    return &identity_test_env_;
+  }
   PrefService* pref_service() { return &prefs_; }
   AccountInvestigator* investigator() { return &investigator_; }
   FakeGaiaCookieManagerService* cookie_manager_service() {
@@ -155,8 +166,10 @@ class AccountInvestigatorTest : public testing::Test {
   sync_preferences::TestingPrefServiceSyncable prefs_;
   AccountTrackerService account_tracker_service_;
   TestSigninClient signin_client_;
+  FakeProfileOAuth2TokenService token_service_;
   FakeSigninManager signin_manager_;
   FakeGaiaCookieManagerService gaia_cookie_manager_service_;
+  identity::IdentityTestEnvironment identity_test_env_;
   AccountInvestigator investigator_;
   std::map<ReportingType, std::string> suffix_ = {
       {ReportingType::PERIODIC, "_Periodic"},
@@ -177,14 +190,19 @@ AccountInfo Info(const std::string& id) {
   return info;
 }
 
+// NOTE: IdentityTestEnvironment uses a prefix for generating gaia IDs:
+// "gaia_id_for_". For this reason, the tests prefix expected account IDs
+// used so that there is a match.
 const std::vector<ListedAccount> no_accounts{};
-const std::vector<ListedAccount> just_one{Account("1")};
-const std::vector<ListedAccount> just_two{Account("2")};
-const std::vector<ListedAccount> both{Account("1"), Account("2")};
-const std::vector<ListedAccount> both_reversed{Account("2"), Account("1")};
+const std::vector<ListedAccount> just_one{Account("gaia_id_for_1_mail.com")};
+const std::vector<ListedAccount> just_two{Account("gaia_id_for_2_mail.com")};
+const std::vector<ListedAccount> both{Account("gaia_id_for_1_mail.com"),
+                                      Account("gaia_id_for_2_mail.com")};
+const std::vector<ListedAccount> both_reversed{
+    Account("gaia_id_for_2_mail.com"), Account("gaia_id_for_1_mail.com")};
 
-const AccountInfo one(Info("1"));
-const AccountInfo three(Info("3"));
+const AccountInfo one(Info("gaia_id_for_1_mail.com"));
+const AccountInfo three(Info("gaia_id_for_3_mail.com"));
 
 TEST_F(AccountInvestigatorTest, CalculatePeriodicDelay) {
   const Time epoch;
@@ -239,7 +257,7 @@ TEST_F(AccountInvestigatorTest, DiscernRelation) {
 TEST_F(AccountInvestigatorTest, SignedInAccountRelationReport) {
   ExpectRelationReport(just_one, no_accounts, ReportingType::PERIODIC,
                        AccountRelation::WITH_SIGNED_IN_NO_MATCH);
-  signin_manager()->SignIn("1", "a", "A");
+  identity_test_env()->SetPrimaryAccount("1@mail.com");
   ExpectRelationReport(just_one, no_accounts, ReportingType::PERIODIC,
                        AccountRelation::SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT);
   ExpectRelationReport(just_two, no_accounts, ReportingType::ON_CHANGE,
@@ -255,7 +273,7 @@ TEST_F(AccountInvestigatorTest, SharedCookieJarReportEmpty) {
 }
 
 TEST_F(AccountInvestigatorTest, SharedCookieJarReportWithAccount) {
-  signin_manager()->SignIn("1", "a", "A");
+  identity_test_env()->SetPrimaryAccount("1@mail.com");
   base::Time now = base::Time::Now();
   pref_service()->SetDouble(prefs::kGaiaCookieChangedTime, now.ToDoubleT());
   const AccountRelation expected_relation(
@@ -290,7 +308,7 @@ TEST_F(AccountInvestigatorTest, OnGaiaAccountsInCookieUpdatedSigninOnly) {
       no_accounts, no_accounts, GoogleServiceAuthError::AuthErrorNone());
 
   const HistogramTester histogram_tester;
-  signin_manager()->SignIn("1", "a", "A");
+  identity_test_env()->SetPrimaryAccount("1@mail.com");
   pref_service()->SetString(prefs::kGaiaCookieHash,
                             Hash(just_one, no_accounts));
   investigator()->OnGaiaAccountsInCookieUpdated(
@@ -303,7 +321,7 @@ TEST_F(AccountInvestigatorTest, OnGaiaAccountsInCookieUpdatedSigninOnly) {
 TEST_F(AccountInvestigatorTest,
        OnGaiaAccountsInCookieUpdatedSigninSignOutOfContent) {
   const HistogramTester histogram_tester;
-  signin_manager()->SignIn("1", "a", "A");
+  identity_test_env()->SetPrimaryAccount("1@mail.com");
   investigator()->OnGaiaAccountsInCookieUpdated(
       just_one, no_accounts, GoogleServiceAuthError::AuthErrorNone());
   ExpectRelationReport(ReportingType::ON_CHANGE, histogram_tester,
@@ -332,7 +350,7 @@ TEST_F(AccountInvestigatorTest, Initialize) {
 }
 
 TEST_F(AccountInvestigatorTest, InitializeSignedIn) {
-  signin_manager()->SignIn("1", "a", "A");
+  identity_test_env()->SetPrimaryAccount("1@mail.com");
   EXPECT_FALSE(*previously_authenticated());
 
   investigator()->Initialize();