Refactor WebViewSyncClient for testing

- Also removed some unnecessary setup code

Change-Id: I65f46565e6c2c1f21da4e8db22b876125c2d6c07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151089Reviewed-by: Hiroshi Ichikawa <ichikawa@chromium.org>
Commit-Queue: John Wu <jzw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759514}

ios/web_view/BUILD.gn

@@ -285,8 +285,6 @@ source_set("web_view_sources") {
     "//components/strings:components_strings_grit",
     "//components/sync",
     "//components/sync_device_info",
-    "//components/sync_sessions",
-    "//components/sync_user_events",
     "//components/translate/core/browser",
     "//components/translate/core/common",
     "//components/translate/ios/browser",
@@ -416,6 +414,7 @@ test("ios_web_view_unittests") {
     "internal/signin/cwv_identity_unittest.mm",
     "internal/signin/web_view_gaia_auth_fetcher_unittest.mm",
     "internal/sync/cwv_sync_controller_unittest.mm",
+    "internal/sync/web_view_sync_client_unittest.mm",
     "internal/translate/cwv_translation_controller_unittest.mm",
     "internal/translate/cwv_translation_language_unittest.mm",
     "internal/translate/cwv_translation_policy_unittest.mm",
@@ -434,11 +433,13 @@ test("ios_web_view_unittests") {
     "//components/autofill/ios/browser:test_support",
     "//components/autofill/ios/form_util:test_support",
     "//components/browser_sync:test_support",
+    "//components/invalidation/impl:test_support",
     "//components/password_manager/core/browser:test_support",
     "//components/prefs:test_support",
     "//components/signin/public/base:test_support",
     "//components/signin/public/identity_manager:test_support",
     "//components/sync:test_support",
+    "//components/sync_device_info:test_support",
     "//ios/web/common:web_view_creation_util",
     "//ios/web/public/deprecated:test_doubles",
     "//ios/web/public/js_messaging",

ios/web_view/internal/DEPS

@@ -28,8 +28,6 @@ include_rules = [
   "+components/strings/grit",
   "+components/sync",
   "+components/sync_device_info",
-  "+components/sync_sessions",
-  "+components/sync_user_events",
   "+components/translate/core",
   "+components/translate/ios",
   "+components/language/ios",

ios/web_view/internal/passwords/web_view_password_store_factory.h

@@ -31,11 +31,6 @@ class WebViewPasswordStoreFactory
 
   static WebViewPasswordStoreFactory* GetInstance();
 
-  // Called by the PasswordDataTypeController whenever there is a possibility
-  // that syncing passwords has just started or ended for |browser_state|.
-  static void OnPasswordsSyncedStatePotentiallyChanged(
-      WebViewBrowserState* browser_state);
-
  private:
   friend class base::NoDestructor<WebViewPasswordStoreFactory>;
 

ios/web_view/internal/passwords/web_view_password_store_factory.mm

@@ -16,9 +16,6 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
-#include "components/password_manager/core/browser/android_affiliation/affiliated_match_helper.h"
-#include "components/password_manager/core/browser/android_affiliation/affiliation_service.h"
-#include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_store_default.h"
 #include "components/password_manager/core/browser/password_store_factory_util.h"
@@ -56,20 +53,6 @@ WebViewPasswordStoreFactory* WebViewPasswordStoreFactory::GetInstance() {
   return instance.get();
 }
 
-// static
-void WebViewPasswordStoreFactory::OnPasswordsSyncedStatePotentiallyChanged(
-    WebViewBrowserState* browser_state) {
-  scoped_refptr<password_manager::PasswordStore> password_store =
-      GetForBrowserState(browser_state, ServiceAccessType::EXPLICIT_ACCESS);
-  syncer::SyncService* sync_service =
-      WebViewProfileSyncServiceFactory::GetForBrowserState(browser_state);
-  password_manager::ToggleAffiliationBasedMatchingBasedOnPasswordSyncedState(
-      password_store.get(), sync_service,
-      browser_state->GetSharedURLLoaderFactory(),
-      ApplicationContext::GetInstance()->GetNetworkConnectionTracker(),
-      browser_state->GetStatePath());
-}
-
 WebViewPasswordStoreFactory::WebViewPasswordStoreFactory()
     : RefcountedBrowserStateKeyedServiceFactory(
           "PasswordStore",

ios/web_view/internal/sync/web_view_profile_sync_service_factory.mm

@@ -90,7 +90,7 @@ WebViewProfileSyncServiceFactory::BuildServiceInstanceFor(
   syncer::ProfileSyncService::InitParams init_params;
   init_params.identity_manager = identity_manager;
   init_params.start_behavior = syncer::ProfileSyncService::MANUAL_START;
-  init_params.sync_client = std::make_unique<WebViewSyncClient>(browser_state);
+  init_params.sync_client = WebViewSyncClient::Create(browser_state);
   init_params.url_loader_factory = browser_state->GetSharedURLLoaderFactory();
   // ios/web_view has no need to update network time.
   init_params.network_time_update_callback = base::DoNothing();

ios/web_view/internal/sync/web_view_sync_client.h

@@ -10,31 +10,29 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
+#include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/browser_sync/browser_sync_client.h"
-
-namespace autofill {
-class AutofillWebDataService;
-}  // namespace autofill
-
-namespace browser_sync {
-class ProfileSyncComponentsFactoryImpl;
-}  // namespace browser_sync
-
-namespace password_manager {
-class PasswordStore;
-}  // namespace password_manager
-
-namespace syncer {
-class SyncService;
-}  // namespace syncer
+#include "components/browser_sync/profile_sync_components_factory_impl.h"
+#include "components/password_manager/core/browser/password_store.h"
+#include "ios/web_view/internal/web_view_browser_state.h"
 
 namespace ios_web_view {
 
-class WebViewBrowserState;
-
 class WebViewSyncClient : public browser_sync::BrowserSyncClient {
  public:
-  explicit WebViewSyncClient(WebViewBrowserState* browser_state);
+  static std::unique_ptr<WebViewSyncClient> Create(
+      WebViewBrowserState* browser_state);
+
+  explicit WebViewSyncClient(
+      autofill::AutofillWebDataService* profile_web_data_service,
+      autofill::AutofillWebDataService* account_web_data_service,
+      password_manager::PasswordStore* profile_password_store,
+      password_manager::PasswordStore* account_password_store,
+      PrefService* pref_service,
+      signin::IdentityManager* identity_manager,
+      syncer::ModelTypeStoreService* model_type_store_service,
+      syncer::DeviceInfoSyncService* device_info_sync_service,
+      invalidation::InvalidationService* invalidation_service);
   ~WebViewSyncClient() override;
 
   // BrowserSyncClient implementation.
@@ -66,17 +64,20 @@ class WebViewSyncClient : public browser_sync::BrowserSyncClient {
   syncer::SyncTypePreferenceProvider* GetPreferenceProvider() override;
 
  private:
-  WebViewBrowserState* browser_state_ = nullptr;
-  scoped_refptr<autofill::AutofillWebDataService> profile_web_data_service_;
-  scoped_refptr<autofill::AutofillWebDataService> account_web_data_service_;
-  scoped_refptr<password_manager::PasswordStore> password_store_;
-  scoped_refptr<password_manager::PasswordStore> account_password_store_;
+  autofill::AutofillWebDataService* profile_web_data_service_;
+  autofill::AutofillWebDataService* account_web_data_service_;
+  password_manager::PasswordStore* profile_password_store_;
+  password_manager::PasswordStore* account_password_store_;
+  PrefService* pref_service_;
+  signin::IdentityManager* identity_manager_;
+  syncer::ModelTypeStoreService* model_type_store_service_;
+  syncer::DeviceInfoSyncService* device_info_sync_service_;
+  invalidation::InvalidationService* invalidation_service_;
 
   // TODO(crbug.com/915154): Revert to SyncApiComponentFactory once common
   // controller creation is moved elsewhere.
   std::unique_ptr<browser_sync::ProfileSyncComponentsFactoryImpl>
       component_factory_;
-  scoped_refptr<base::SingleThreadTaskRunner> db_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(WebViewSyncClient);
 };

ios/web_view/internal/sync/web_view_sync_client.mm

@@ -11,18 +11,14 @@
 #include "base/logging.h"
 #include "base/task/post_task.h"
 #include "components/autofill/core/browser/webdata/autofill_profile_sync_bridge.h"
-#include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/autofill/core/common/autofill_features.h"
-#include "components/browser_sync/profile_sync_components_factory_impl.h"
 #include "components/history/core/common/pref_names.h"
 #include "components/invalidation/impl/profile_invalidation_provider.h"
 #include "components/keyed_service/core/service_access_type.h"
-#include "components/password_manager/core/browser/password_store.h"
 #include "components/sync/driver/data_type_controller.h"
 #include "components/sync/driver/sync_api_component_factory.h"
 #include "components/sync/driver/sync_util.h"
 #include "components/sync/engine/passive_model_worker.h"
-#include "components/sync_user_events/user_event_service.h"
 #include "components/version_info/version_info.h"
 #include "components/version_info/version_string.h"
 #include "ios/web/public/thread/web_task_traits.h"
@@ -34,7 +30,6 @@
 #import "ios/web_view/internal/sync/web_view_device_info_sync_service_factory.h"
 #import "ios/web_view/internal/sync/web_view_model_type_store_service_factory.h"
 #import "ios/web_view/internal/sync/web_view_profile_invalidation_provider_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"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -55,35 +50,58 @@ syncer::ModelTypeSet GetDisabledTypes() {
 }
 }  // namespace
 
-WebViewSyncClient::WebViewSyncClient(WebViewBrowserState* browser_state)
-    : browser_state_(browser_state) {
-  profile_web_data_service_ =
+// static
+std::unique_ptr<WebViewSyncClient> WebViewSyncClient::Create(
+    WebViewBrowserState* browser_state) {
+  return std::make_unique<WebViewSyncClient>(
       WebViewWebDataServiceWrapperFactory::GetAutofillWebDataForBrowserState(
-          browser_state_, ServiceAccessType::IMPLICIT_ACCESS);
-  account_web_data_service_ =
-      base::FeatureList::IsEnabled(
-          autofill::features::kAutofillEnableAccountWalletStorage)
-          ? WebViewWebDataServiceWrapperFactory::GetAutofillWebDataForAccount(
-                browser_state_, ServiceAccessType::IMPLICIT_ACCESS)
-          : nullptr;
-
-  db_thread_ = profile_web_data_service_
-                   ? profile_web_data_service_->GetDBTaskRunner()
-                   : nullptr;
-
-  password_store_ = WebViewPasswordStoreFactory::GetForBrowserState(
-      browser_state_, ServiceAccessType::IMPLICIT_ACCESS);
-  account_password_store_ =
+          browser_state, ServiceAccessType::IMPLICIT_ACCESS)
+          .get(),
+      WebViewWebDataServiceWrapperFactory::GetAutofillWebDataForAccount(
+          browser_state, ServiceAccessType::IMPLICIT_ACCESS)
+          .get(),
+      WebViewPasswordStoreFactory::GetForBrowserState(
+          browser_state, ServiceAccessType::IMPLICIT_ACCESS)
+          .get(),
       WebViewAccountPasswordStoreFactory::GetForBrowserState(
-          browser_state_, ServiceAccessType::IMPLICIT_ACCESS);
-
+          browser_state, ServiceAccessType::IMPLICIT_ACCESS)
+          .get(),
+      browser_state->GetPrefs(),
+      WebViewIdentityManagerFactory::GetForBrowserState(browser_state),
+      WebViewModelTypeStoreServiceFactory::GetForBrowserState(browser_state),
+      WebViewDeviceInfoSyncServiceFactory::GetForBrowserState(browser_state),
+      WebViewProfileInvalidationProviderFactory::GetForBrowserState(
+          browser_state)
+          ->GetInvalidationService());
+}
+
+WebViewSyncClient::WebViewSyncClient(
+    autofill::AutofillWebDataService* profile_web_data_service,
+    autofill::AutofillWebDataService* account_web_data_service,
+    password_manager::PasswordStore* profile_password_store,
+    password_manager::PasswordStore* account_password_store,
+    PrefService* pref_service,
+    signin::IdentityManager* identity_manager,
+    syncer::ModelTypeStoreService* model_type_store_service,
+    syncer::DeviceInfoSyncService* device_info_sync_service,
+    invalidation::InvalidationService* invalidation_service)
+    : profile_web_data_service_(profile_web_data_service),
+      account_web_data_service_(account_web_data_service),
+      profile_password_store_(profile_password_store),
+      account_password_store_(account_password_store),
+      pref_service_(pref_service),
+      identity_manager_(identity_manager),
+      model_type_store_service_(model_type_store_service),
+      device_info_sync_service_(device_info_sync_service),
+      invalidation_service_(invalidation_service) {
   component_factory_ =
       std::make_unique<browser_sync::ProfileSyncComponentsFactoryImpl>(
           this, version_info::Channel::STABLE,
           prefs::kSavingBrowserHistoryDisabled,
-          base::CreateSingleThreadTaskRunner({web::WebThread::UI}), db_thread_,
-          profile_web_data_service_, account_web_data_service_, password_store_,
-          account_password_store_,
+          base::CreateSingleThreadTaskRunner({web::WebThread::UI}),
+          profile_web_data_service_->GetDBTaskRunner(),
+          profile_web_data_service_, account_web_data_service_,
+          profile_password_store_, account_password_store_,
           /*bookmark_sync_service=*/nullptr);
 }
 
@@ -91,12 +109,12 @@ WebViewSyncClient::~WebViewSyncClient() {}
 
 PrefService* WebViewSyncClient::GetPrefService() {
   DCHECK_CURRENTLY_ON(web::WebThread::UI);
-  return browser_state_->GetPrefs();
+  return pref_service_;
 }
 
 signin::IdentityManager* WebViewSyncClient::GetIdentityManager() {
   DCHECK_CURRENTLY_ON(web::WebThread::UI);
-  return WebViewIdentityManagerFactory::GetForBrowserState(browser_state_);
+  return identity_manager_;
 }
 
 base::FilePath WebViewSyncClient::GetLocalSyncBackendFolder() {
@@ -104,13 +122,11 @@ base::FilePath WebViewSyncClient::GetLocalSyncBackendFolder() {
 }
 
 syncer::ModelTypeStoreService* WebViewSyncClient::GetModelTypeStoreService() {
-  return WebViewModelTypeStoreServiceFactory::GetForBrowserState(
-      browser_state_);
+  return model_type_store_service_;
 }
 
 syncer::DeviceInfoSyncService* WebViewSyncClient::GetDeviceInfoSyncService() {
-  return WebViewDeviceInfoSyncServiceFactory::GetForBrowserState(
-      browser_state_);
+  return device_info_sync_service_;
 }
 
 bookmarks::BookmarkModel* WebViewSyncClient::GetBookmarkModel() {
@@ -135,9 +151,7 @@ WebViewSyncClient::GetSendTabToSelfSyncService() {
 }
 
 base::RepeatingClosure WebViewSyncClient::GetPasswordStateChangedCallback() {
-  return base::BindRepeating(
-      &WebViewPasswordStoreFactory::OnPasswordsSyncedStatePotentiallyChanged,
-      base::Unretained(browser_state_));
+  return base::DoNothing();
 }
 
 syncer::DataTypeController::TypeVector
@@ -152,13 +166,7 @@ BookmarkUndoService* WebViewSyncClient::GetBookmarkUndoService() {
 }
 
 invalidation::InvalidationService* WebViewSyncClient::GetInvalidationService() {
-  invalidation::ProfileInvalidationProvider* provider =
-      WebViewProfileInvalidationProviderFactory::GetForBrowserState(
-          browser_state_);
-  if (provider) {
-    return provider->GetInvalidationService();
-  }
-  return nullptr;
+  return invalidation_service_;
 }
 
 syncer::TrustedVaultClient* WebViewSyncClient::GetTrustedVaultClient() {

ios/web_view/internal/sync/web_view_sync_client_unittest.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/web_view/internal/sync/web_view_sync_client.h"
+
+#include <memory>
+
+#include "base/memory/scoped_refptr.h"
+#include "components/autofill/core/browser/webdata/mock_autofill_webdata_service.h"
+#include "components/history/core/common/pref_names.h"
+#include "components/invalidation/impl/fake_invalidation_service.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
+#include "components/sync/base/model_type.h"
+#include "components/sync/driver/data_type_controller.h"
+#include "components/sync/driver/test_sync_service.h"
+#include "components/sync/model/test_model_type_store_service.h"
+#include "components/sync_device_info/fake_device_info_sync_service.h"
+#include "ios/web/public/test/web_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#import "testing/gtest_mac.h"
+#include "testing/platform_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace ios_web_view {
+
+class WebViewSyncClientTest : public PlatformTest {
+ protected:
+  WebViewSyncClientTest()
+      : profile_web_data_service_(
+            base::MakeRefCounted<autofill::MockAutofillWebDataService>()),
+        account_web_data_service_(
+            base::MakeRefCounted<autofill::MockAutofillWebDataService>()),
+        profile_password_store_(
+            base::MakeRefCounted<password_manager::TestPasswordStore>()),
+        account_password_store_(
+            base::MakeRefCounted<password_manager::TestPasswordStore>(
+                /*is_account_store=*/true)),
+        client_(profile_web_data_service_.get(),
+                account_web_data_service_.get(),
+                profile_password_store_.get(),
+                account_password_store_.get(),
+                &pref_service_,
+                identity_test_environment_.identity_manager(),
+                &model_type_store_service_,
+                &device_info_sync_service_,
+                &invalidation_service_) {
+    pref_service_.registry()->RegisterBooleanPref(
+        prefs::kSavingBrowserHistoryDisabled, true);
+    profile_password_store_->Init(&pref_service_, base::DoNothing());
+    account_password_store_->Init(&pref_service_, base::DoNothing());
+  }
+
+  ~WebViewSyncClientTest() override {
+    profile_password_store_->ShutdownOnUIThread();
+    account_password_store_->ShutdownOnUIThread();
+  }
+
+  web::WebTaskEnvironment task_environment_;
+  scoped_refptr<autofill::MockAutofillWebDataService> profile_web_data_service_;
+  scoped_refptr<autofill::MockAutofillWebDataService> account_web_data_service_;
+  scoped_refptr<password_manager::TestPasswordStore> profile_password_store_;
+  scoped_refptr<password_manager::TestPasswordStore> account_password_store_;
+  TestingPrefServiceSimple pref_service_;
+  signin::IdentityTestEnvironment identity_test_environment_;
+  syncer::TestModelTypeStoreService model_type_store_service_;
+  syncer::FakeDeviceInfoSyncService device_info_sync_service_;
+  invalidation::FakeInvalidationService invalidation_service_;
+  WebViewSyncClient client_;
+};
+
+// Verify enabled data types.
+TEST_F(WebViewSyncClientTest, CreateDataTypeControllers) {
+  syncer::TestSyncService sync_service;
+  syncer::DataTypeController::TypeVector data_type_controllers =
+      client_.CreateDataTypeControllers(&sync_service);
+  syncer::ModelTypeSet allowed_types = syncer::ModelTypeSet(
+      syncer::DEVICE_INFO, syncer::AUTOFILL, syncer::AUTOFILL_PROFILE,
+      syncer::AUTOFILL_WALLET_DATA, syncer::AUTOFILL_WALLET_METADATA,
+      syncer::PASSWORDS);
+  for (const auto& data_type_controller : data_type_controllers) {
+    ASSERT_TRUE(allowed_types.Has(data_type_controller->type()));
+    allowed_types.Remove(data_type_controller->type());
+  }
+  EXPECT_TRUE(allowed_types.Empty());
+}
+
+}  // namespace ios_web_view

ios/web_view/internal/web_view_browser_state.mm

@@ -25,7 +25,6 @@
 #include "components/sync/base/pref_names.h"
 #include "components/sync/base/sync_prefs.h"
 #include "components/sync_device_info/device_info_prefs.h"
-#include "components/sync_sessions/session_sync_prefs.h"
 #include "components/translate/core/browser/translate_pref_names.h"
 #include "components/translate/core/browser/translate_prefs.h"
 #include "ios/web/public/thread/web_task_traits.h"
@@ -168,7 +167,6 @@ void WebViewBrowserState::RegisterPrefs(
   translate::TranslatePrefs::RegisterProfilePrefs(pref_registry);
   autofill::prefs::RegisterProfilePrefs(pref_registry);
   password_manager::PasswordManager::RegisterProfilePrefs(pref_registry);
-  sync_sessions::SessionSyncPrefs::RegisterProfilePrefs(pref_registry);
   syncer::SyncPrefs::RegisterProfilePrefs(pref_registry);
   syncer::DeviceInfoPrefs::RegisterProfilePrefs(pref_registry);