[Autofill] Convert PersonalDataManager to use IdentityManager

The conversion is straightforward, getting the email address of the primary account from IdentityManager rather than SigninManager and AccountTrackerService. Straightforward unittest conversions are also made. NOTE: This CL was not tested manually as I don't know how to exercise this functionality. TBR=sdefresne@chromium.org Bug: 809435 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I7e145538b9e8634326a409c812b03a04c0254b05 Reviewed-on: https://chromium-review.googlesource.com/913390 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Mathieu Perreault <mathp@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#539456}
marcosholgado · Feb 27, 2018 · fbb98d8 · fbb98d8
1 parent de4aa61
commit fbb98d8
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 104 deletions.
diff --git a/chrome/browser/autofill/personal_data_manager_factory.cc b/chrome/browser/autofill/personal_data_manager_factory.cc
@@ -8,14 +8,11 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/signin/account_tracker_service_factory.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/web_data_service_factory.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "components/signin/core/browser/account_tracker_service.h"
-#include "components/signin/core/browser/signin_manager.h"
 
 namespace autofill {
 
@@ -35,8 +32,7 @@ PersonalDataManagerFactory::PersonalDataManagerFactory()
     : BrowserContextKeyedServiceFactory(
         "PersonalDataManager",
         BrowserContextDependencyManager::GetInstance()) {
-  DependsOn(AccountTrackerServiceFactory::GetInstance());
-  DependsOn(SigninManagerFactory::GetInstance());
+  DependsOn(IdentityManagerFactory::GetInstance());
   DependsOn(WebDataServiceFactory::GetInstance());
 }
 
@@ -51,8 +47,7 @@ KeyedService* PersonalDataManagerFactory::BuildServiceInstanceFor(
   service->Init(WebDataServiceFactory::GetAutofillWebDataForProfile(
                     profile, ServiceAccessType::EXPLICIT_ACCESS),
                 profile->GetPrefs(),
-                AccountTrackerServiceFactory::GetForProfile(profile),
-                SigninManagerFactory::GetForProfile(profile),
+                IdentityManagerFactory::GetForProfile(profile),
                 profile->IsOffTheRecord());
   return service;
 }

diff --git a/components/autofill/core/browser/autofill_metrics_unittest.cc b/components/autofill/core/browser/autofill_metrics_unittest.cc
@@ -2434,7 +2434,7 @@ TEST_F(AutofillMetricsTest, AutofillIsEnabledAtStartup) {
   base::HistogramTester histogram_tester;
   personal_data_->SetAutofillEnabled(true);
   personal_data_->Init(autofill_client_.GetDatabase(),
-                       autofill_client_.GetPrefs(), nullptr, nullptr, false);
+                       autofill_client_.GetPrefs(), nullptr, false);
   histogram_tester.ExpectUniqueSample("Autofill.IsEnabled.Startup", true, 1);
 }
 
@@ -2443,7 +2443,7 @@ TEST_F(AutofillMetricsTest, AutofillIsDisabledAtStartup) {
   base::HistogramTester histogram_tester;
   personal_data_->SetAutofillEnabled(false);
   personal_data_->Init(autofill_client_.GetDatabase(),
-                       autofill_client_.GetPrefs(), nullptr, nullptr, false);
+                       autofill_client_.GetPrefs(), nullptr, false);
   histogram_tester.ExpectUniqueSample("Autofill.IsEnabled.Startup", false, 1);
 }
 

diff --git a/components/autofill/core/browser/form_data_importer_unittest.cc b/components/autofill/core/browser/form_data_importer_unittest.cc
@@ -117,7 +117,7 @@ class FormDataImporterTestBase {
     personal_data_manager_.reset(new PersonalDataManager("en"));
     personal_data_manager_->Init(
         scoped_refptr<AutofillWebDataService>(autofill_database_service_),
-        prefs_.get(), nullptr, nullptr, is_incognito);
+        prefs_.get(), nullptr, is_incognito);
     personal_data_manager_->AddObserver(&personal_data_observer_);
     personal_data_manager_->OnSyncServiceInitialized(nullptr);
 

diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc
@@ -41,12 +41,10 @@
 #include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/autofill_util.h"
 #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_pref_names.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/version_info/version_info.h"
+#include "services/identity/public/cpp/identity_manager.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_formatter.h"
 
@@ -358,22 +356,20 @@ PersonalDataManager::PersonalDataManager(const std::string& app_locale)
       pending_server_creditcards_query_(0),
       app_locale_(app_locale),
       pref_service_(nullptr),
-      account_tracker_(nullptr),
+      identity_manager_(nullptr),
       is_off_the_record_(false),
       has_logged_stored_profile_metrics_(false),
       has_logged_stored_credit_card_metrics_(false) {}
 
 void PersonalDataManager::Init(scoped_refptr<AutofillWebDataService> database,
                                PrefService* pref_service,
-                               AccountTrackerService* account_tracker,
-                               SigninManagerBase* signin_manager,
+                               identity::IdentityManager* identity_manager,
                                bool is_off_the_record) {
   CountryNames::SetLocaleString(app_locale_);
 
   database_ = database;
   SetPrefService(pref_service);
-  account_tracker_ = account_tracker;
-  signin_manager_ = signin_manager;
+  identity_manager_ = identity_manager;
   is_off_the_record_ = is_off_the_record;
 
   if (!is_off_the_record_)
@@ -1992,9 +1988,8 @@ std::string PersonalDataManager::MergeServerAddressesIntoProfiles(
 
     // Wallet addresses don't have an email address, use the one from the
     // currently signed-in account.
-    std::string account_id = signin_manager_->GetAuthenticatedAccountId();
     base::string16 email =
-        base::UTF8ToUTF16(account_tracker_->GetAccountInfo(account_id).email);
+        base::UTF8ToUTF16(identity_manager_->GetPrimaryAccountInfo().email);
     if (!email.empty())
       existing_profiles->back().SetRawInfo(EMAIL_ADDRESS, email);
 

diff --git a/components/autofill/core/browser/personal_data_manager.h b/components/autofill/core/browser/personal_data_manager.h
@@ -29,11 +29,9 @@
 #include "components/prefs/pref_member.h"
 #include "components/webdata/common/web_data_service_consumer.h"
 
-class AccountTrackerService;
 class Browser;
 class PrefService;
 class RemoveAutofillTester;
-class SigninManagerBase;
 
 namespace autofill {
 class AutofillInteractiveTest;
@@ -46,6 +44,10 @@ void SetProfiles(int, std::vector<autofill::AutofillProfile>*);
 void SetCreditCards(int, std::vector<autofill::CreditCard>*);
 }  // namespace autofill_helper
 
+namespace identity {
+class IdentityManager;
+}
+
 namespace syncer {
 class SyncService;
 }  // namespace syncer
@@ -72,8 +74,7 @@ class PersonalDataManager : public KeyedService,
   // context.
   void Init(scoped_refptr<AutofillWebDataService> database,
             PrefService* pref_service,
-            AccountTrackerService* account_tracker,
-            SigninManagerBase* signin_manager,
+            identity::IdentityManager* identity_manager,
             bool is_off_the_record);
 
   // Called once the sync service is known to be instantiated. Note that it may
@@ -424,14 +425,6 @@ class PersonalDataManager : public KeyedService,
     database_ = database;
   }
 
-  void set_account_tracker(AccountTrackerService* account_tracker) {
-    account_tracker_ = account_tracker;
-  }
-
-  void set_signin_manager(SigninManagerBase* signin_manager) {
-    signin_manager_ = signin_manager;
-  }
-
   // The backing database that this PersonalDataManager uses.
   scoped_refptr<AutofillWebDataService> database_;
 
@@ -581,12 +574,8 @@ class PersonalDataManager : public KeyedService,
   // The PrefService that this instance uses. Must outlive this instance.
   PrefService* pref_service_;
 
-  // The AccountTrackerService that this instance uses. Must outlive this
-  // instance.
-  AccountTrackerService* account_tracker_;
-
-  // The signin manager that this instance uses. Must outlive this instance.
-  SigninManagerBase* signin_manager_;
+  // The identity manager that this instance uses. Must outlive this instance.
+  identity::IdentityManager* identity_manager_;
 
   // Whether the user is currently operating in an off-the-record context.
   // Default value is false.

diff --git a/components/autofill/core/browser/personal_data_manager_unittest.cc b/components/autofill/core/browser/personal_data_manager_unittest.cc
@@ -45,13 +45,10 @@
 #include "components/autofill/core/common/form_data.h"
 #include "components/os_crypt/os_crypt_mocker.h"
 #include "components/prefs/pref_service.h"
-#include "components/signin/core/browser/account_tracker_service.h"
-#include "components/signin/core/browser/fake_signin_manager.h"
-#include "components/signin/core/browser/signin_pref_names.h"
-#include "components/signin/core/browser/test_signin_client.h"
 #include "components/variations/variations_params_manager.h"
 #include "components/webdata/common/web_data_service_base.h"
 #include "components/webdata/common/web_database_service.h"
+#include "services/identity/public/cpp/identity_test_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -125,8 +122,7 @@ class PersonalDataManagerTestBase {
     personal_data_.reset(new PersonalDataManager("en"));
     personal_data_->Init(
         scoped_refptr<AutofillWebDataService>(autofill_database_service_),
-        prefs_.get(), account_tracker_.get(), signin_manager_.get(),
-        is_incognito);
+        prefs_.get(), identity_test_env_.identity_manager(), is_incognito);
     personal_data_->AddObserver(&personal_data_observer_);
     personal_data_->OnSyncServiceInitialized(nullptr);
 
@@ -142,8 +138,7 @@ class PersonalDataManagerTestBase {
   }
 
   void EnableWalletCardImport() {
-    signin_manager_->SetAuthenticatedAccountInfo("12345",
-                                                 "syncuser@example.com");
+    identity_test_env_.MakePrimaryAccountAvailable("syncuser@example.com");
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kEnableOfferStoreUnmaskedWalletCards);
   }
@@ -252,9 +247,7 @@ class PersonalDataManagerTestBase {
   base::ScopedTempDir temp_dir_;
   base::MessageLoopForUI message_loop_;
   std::unique_ptr<PrefService> prefs_;
-  std::unique_ptr<AccountTrackerService> account_tracker_;
-  std::unique_ptr<FakeSigninManagerBase> signin_manager_;
-  std::unique_ptr<TestSigninClient> signin_client_;
+  identity::IdentityTestEnvironment identity_test_env_;
   scoped_refptr<AutofillWebDataService> autofill_database_service_;
   scoped_refptr<WebDatabaseService> web_database_;
   AutofillTable* autofill_table_;  // weak ref
@@ -275,14 +268,6 @@ class PersonalDataManagerTest : public PersonalDataManagerTestBase,
         new WebDatabaseService(path, base::ThreadTaskRunnerHandle::Get(),
                                base::ThreadTaskRunnerHandle::Get());
 
-    // Set up account tracker.
-    signin_client_.reset(new TestSigninClient(prefs_.get()));
-    account_tracker_.reset(new AccountTrackerService());
-    account_tracker_->Initialize(signin_client_.get());
-    signin_manager_.reset(new FakeSigninManagerBase(signin_client_.get(),
-                                                    account_tracker_.get()));
-    signin_manager_->Initialize(prefs_.get());
-
     // Hacky: hold onto a pointer but pass ownership.
     autofill_table_ = new AutofillTable;
     web_database_->AddTable(std::unique_ptr<WebDatabaseTable>(autofill_table_));
@@ -306,13 +291,6 @@ class PersonalDataManagerTest : public PersonalDataManagerTestBase,
   void TearDown() override {
     // Order of destruction is important as AutofillManager relies on
     // PersonalDataManager to be around when it gets destroyed.
-    signin_manager_->Shutdown();
-    signin_manager_.reset();
-
-    account_tracker_->Shutdown();
-    account_tracker_.reset();
-    signin_client_.reset();
-
     test::ReenableSystemServices();
     OSCryptMocker::TearDown();
   }
@@ -2960,14 +2938,6 @@ class SaveImportedProfileTest
         new WebDatabaseService(path, base::ThreadTaskRunnerHandle::Get(),
                                base::ThreadTaskRunnerHandle::Get());
 
-    // Set up account tracker.
-    signin_client_.reset(new TestSigninClient(prefs_.get()));
-    account_tracker_.reset(new AccountTrackerService());
-    account_tracker_->Initialize(signin_client_.get());
-    signin_manager_.reset(new FakeSigninManagerBase(signin_client_.get(),
-                                                    account_tracker_.get()));
-    signin_manager_->Initialize(prefs_.get());
-
     // Hacky: hold onto a pointer but pass ownership.
     autofill_table_ = new AutofillTable;
     web_database_->AddTable(std::unique_ptr<WebDatabaseTable>(autofill_table_));
@@ -2989,15 +2959,6 @@ class SaveImportedProfileTest
   }
 
   void TearDown() override {
-    // Order of destruction is important as AutofillManager relies on
-    // PersonalDataManager to be around when it gets destroyed.
-    signin_manager_->Shutdown();
-    signin_manager_.reset();
-
-    account_tracker_->Shutdown();
-    account_tracker_.reset();
-    signin_client_.reset();
-
     test::DisableSystemServices(prefs_.get());
     OSCryptMocker::TearDown();
   }

diff --git a/components/browser_sync/profile_sync_service_autofill_unittest.cc b/components/browser_sync/profile_sync_service_autofill_unittest.cc
@@ -433,10 +433,9 @@ class ProfileSyncServiceAutofillTest
     EXPECT_CALL(personal_data_manager(), LoadProfiles());
     EXPECT_CALL(personal_data_manager(), LoadCreditCards());
 
-    personal_data_manager_->Init(
-        web_data_service_, profile_sync_service_bundle()->pref_service(),
-        profile_sync_service_bundle()->account_tracker(),
-        profile_sync_service_bundle()->signin_manager(), false);
+    personal_data_manager_->Init(web_data_service_,
+                                 profile_sync_service_bundle()->pref_service(),
+                                 nullptr, false);
 
     web_data_service_->StartSyncableService();
 

diff --git a/ios/chrome/browser/autofill/personal_data_manager_factory.cc b/ios/chrome/browser/autofill/personal_data_manager_factory.cc
@@ -15,8 +15,7 @@
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/browser_state_otr_helper.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
-#include "ios/chrome/browser/signin/signin_manager_factory.h"
+#include "ios/chrome/browser/signin/identity_manager_factory.h"
 #include "ios/chrome/browser/web_data_service_factory.h"
 
 namespace autofill {
@@ -37,8 +36,7 @@ PersonalDataManagerFactory::PersonalDataManagerFactory()
     : BrowserStateKeyedServiceFactory(
           "PersonalDataManager",
           BrowserStateDependencyManager::GetInstance()) {
-  DependsOn(ios::AccountTrackerServiceFactory::GetInstance());
-  DependsOn(ios::SigninManagerFactory::GetInstance());
+  DependsOn(IdentityManagerFactory::GetInstance());
   DependsOn(ios::WebDataServiceFactory::GetInstance());
 }
 
@@ -51,14 +49,12 @@ PersonalDataManagerFactory::BuildServiceInstanceFor(
       ios::ChromeBrowserState::FromBrowserState(context);
   std::unique_ptr<PersonalDataManager> service(
       new PersonalDataManager(GetApplicationContext()->GetApplicationLocale()));
-  service->Init(
-      ios::WebDataServiceFactory::GetAutofillWebDataForBrowserState(
-          chrome_browser_state, ServiceAccessType::EXPLICIT_ACCESS),
-      chrome_browser_state->GetPrefs(),
-      ios::AccountTrackerServiceFactory::GetForBrowserState(
-          chrome_browser_state),
-      ios::SigninManagerFactory::GetForBrowserState(chrome_browser_state),
-      chrome_browser_state->IsOffTheRecord());
+  service->Init(ios::WebDataServiceFactory::GetAutofillWebDataForBrowserState(
+                    chrome_browser_state, ServiceAccessType::EXPLICIT_ACCESS),
+                chrome_browser_state->GetPrefs(),
+                IdentityManagerFactory::GetInstance()->GetForBrowserState(
+                    chrome_browser_state),
+                chrome_browser_state->IsOffTheRecord());
   return service;
 }
 

diff --git a/ios/web_view/internal/autofill/web_view_personal_data_manager_factory.cc b/ios/web_view/internal/autofill/web_view_personal_data_manager_factory.cc
@@ -13,8 +13,7 @@
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "ios/web_view/internal/app/application_context.h"
-#include "ios/web_view/internal/signin/web_view_account_tracker_service_factory.h"
-#include "ios/web_view/internal/signin/web_view_signin_manager_factory.h"
+#include "ios/web_view/internal/signin/web_view_identity_manager_factory.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
 #include "ios/web_view/internal/webdata_services/web_view_web_data_service_wrapper_factory.h"
 
@@ -38,8 +37,7 @@ WebViewPersonalDataManagerFactory::WebViewPersonalDataManagerFactory()
     : BrowserStateKeyedServiceFactory(
           "PersonalDataManager",
           BrowserStateDependencyManager::GetInstance()) {
-  DependsOn(WebViewAccountTrackerServiceFactory::GetInstance());
-  DependsOn(WebViewSigninManagerFactory::GetInstance());
+  DependsOn(WebViewIdentityManagerFactory::GetInstance());
   DependsOn(WebViewWebDataServiceWrapperFactory::GetInstance());
 }
 
@@ -57,8 +55,8 @@ WebViewPersonalDataManagerFactory::BuildServiceInstanceFor(
       WebViewWebDataServiceWrapperFactory::GetAutofillWebDataForBrowserState(
           browser_state, ServiceAccessType::EXPLICIT_ACCESS),
       browser_state->GetPrefs(),
-      WebViewAccountTrackerServiceFactory::GetForBrowserState(browser_state),
-      WebViewSigninManagerFactory::GetForBrowserState(browser_state),
+      WebViewIdentityManagerFactory::GetInstance()->GetForBrowserState(
+          browser_state),
       browser_state->IsOffTheRecord());
   return service;
 }