Migrate the iOS SyncSetupService to use SyncUserSettings

SyncUserSettings is a new class that encapsulates all the
user-configurable knobs for Sync. It replaces a bunch of setters
and getters directly on the SyncService.

Bug: 884159
Change-Id: I219dc4d37ecd4a9eac1a7fb40f50b6b0bc2cb88d
Reviewed-on: https://chromium-review.googlesource.com/c/1331402Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607577}

ios/chrome/browser/signin/authentication_service_unittest.mm

@@ -93,8 +93,7 @@ std::unique_ptr<KeyedService> BuildMockSyncSetupService(
   ios::ChromeBrowserState* browser_state =
       ios::ChromeBrowserState::FromBrowserState(context);
   return std::make_unique<SyncSetupServiceMock>(
-      ProfileSyncServiceFactory::GetForBrowserState(browser_state),
-      browser_state->GetPrefs());
+      ProfileSyncServiceFactory::GetForBrowserState(browser_state));
 }
 
 }  // namespace

ios/chrome/browser/sync/sync_setup_service.cc

@@ -8,16 +8,11 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
-#include "components/prefs/pref_service.h"
 #include "components/sync/base/stop_source.h"
-#include "components/sync/base/sync_prefs.h"
 #include "components/sync/driver/sync_service.h"
-#include "components/sync/protocol/sync_protocol_error.h"
+#include "components/sync/driver/sync_user_settings.h"
 #include "components/unified_consent/feature.h"
 #include "google_apis/gaia/google_service_auth_error.h"
-#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#include "ios/chrome/browser/pref_names.h"
-#include "net/base/network_change_notifier.h"
 
 namespace {
 // The set of user-selectable datatypes. This must be in the same order as
@@ -29,11 +24,9 @@ syncer::ModelType kDataTypes[] = {
 };
 }  // namespace
 
-SyncSetupService::SyncSetupService(syncer::SyncService* sync_service,
-                                   PrefService* prefs)
-    : sync_service_(sync_service), prefs_(prefs) {
+SyncSetupService::SyncSetupService(syncer::SyncService* sync_service)
+    : sync_service_(sync_service) {
   DCHECK(sync_service_);
-  DCHECK(prefs_);
   for (unsigned int i = 0; i < base::size(kDataTypes); ++i) {
     if (kDataTypes[i] == syncer::USER_EVENTS &&
         !unified_consent::IsUnifiedConsentFeatureEnabled())
@@ -74,7 +67,8 @@ void SyncSetupService::SetDataTypeEnabled(syncer::ModelType datatype,
   types.RetainAll(user_selectable_types_);
   if (enabled && !IsSyncEnabled())
     SetSyncEnabledWithoutChangingDatatypes(true);
-  sync_service_->OnUserChoseDatatypes(IsSyncingAllDataTypes(), types);
+  sync_service_->GetUserSettings()->SetChosenDataTypes(IsSyncingAllDataTypes(),
+                                                       types);
   if (GetPreferredDataTypes().Empty())
     SetSyncEnabled(false);
 }
@@ -102,8 +96,7 @@ bool SyncSetupService::UserActionIsRequiredToHaveSyncWork() {
 }
 
 bool SyncSetupService::IsSyncingAllDataTypes() const {
-  syncer::SyncPrefs sync_prefs(prefs_);
-  return sync_prefs.HasKeepEverythingSynced();
+  return sync_service_->GetUserSettings()->IsSyncEverythingEnabled();
 }
 
 void SyncSetupService::SetSyncingAllDataTypes(bool sync_all) {
@@ -111,7 +104,7 @@ void SyncSetupService::SetSyncingAllDataTypes(bool sync_all) {
     sync_blocker_ = sync_service_->GetSetupInProgressHandle();
   if (sync_all && !IsSyncEnabled())
     SetSyncEnabled(true);
-  sync_service_->OnUserChoseDatatypes(
+  sync_service_->GetUserSettings()->SetChosenDataTypes(
       sync_all,
       Intersection(GetPreferredDataTypes(), syncer::UserSelectableTypes()));
 }
@@ -162,7 +155,7 @@ SyncSetupService::SyncServiceState SyncSetupService::GetSyncServiceState() {
   }
   if (sync_service_->HasUnrecoverableError())
     return kSyncServiceUnrecoverableError;
-  if (sync_service_->IsPassphraseRequiredForDecryption())
+  if (sync_service_->GetUserSettings()->IsPassphraseRequiredForDecryption())
     return kSyncServiceNeedsPassphrase;
   return kNoSyncServiceError;
 }
@@ -173,13 +166,13 @@ bool SyncSetupService::HasFinishedInitialSetup() {
   //   OR
   //   2. User is not signed in or has disabled sync.
   return !sync_service_->CanSyncFeatureStart() ||
-         sync_service_->IsFirstSetupComplete();
+         sync_service_->GetUserSettings()->IsFirstSetupComplete();
 }
 
 void SyncSetupService::PrepareForFirstSyncSetup() {
   // |PrepareForFirstSyncSetup| should always be called while the user is signed
   // out. At that time, sync setup is not completed.
-  DCHECK(!sync_service_->IsFirstSetupComplete());
+  DCHECK(!sync_service_->GetUserSettings()->IsFirstSetupComplete());
   if (!sync_blocker_)
     sync_blocker_ = sync_service_->GetSetupInProgressHandle();
 }
@@ -189,7 +182,7 @@ void SyncSetupService::CommitChanges() {
     // Turn on the sync setup completed flag only if the user did not turn sync
     // off.
     if (sync_service_->CanSyncFeatureStart()) {
-      sync_service_->SetFirstSetupComplete();
+      sync_service_->GetUserSettings()->SetFirstSetupComplete();
     }
   }
 
@@ -204,11 +197,9 @@ void SyncSetupService::SetSyncEnabledWithoutChangingDatatypes(
     bool sync_enabled) {
   if (!sync_blocker_)
     sync_blocker_ = sync_service_->GetSetupInProgressHandle();
-  if (sync_enabled) {
-    sync_service_->RequestStart();
-  } else {
+  if (!sync_enabled) {
     UMA_HISTOGRAM_ENUMERATION("Sync.StopSource", syncer::CHROME_SYNC_SETTINGS,
                               syncer::STOP_SOURCE_LIMIT);
-    sync_service_->RequestStop(syncer::SyncService::KEEP_DATA);
   }
+  sync_service_->GetUserSettings()->SetSyncRequested(sync_enabled);
 }

ios/chrome/browser/sync/sync_setup_service.h

@@ -5,16 +5,11 @@
 #ifndef IOS_CHROME_BROWSER_SYNC_SYNC_SETUP_SERVICE_H_
 #define IOS_CHROME_BROWSER_SYNC_SYNC_SETUP_SERVICE_H_
 
-#include <map>
 #include <memory>
 
 #include "base/macros.h"
-#include "base/strings/string16.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/sync/base/model_type.h"
-#include "components/sync/base/syncer_error.h"
-
-class PrefService;
 
 namespace syncer {
 class SyncService;
@@ -49,7 +44,7 @@ class SyncSetupService : public KeyedService {
     kNumberOfSyncableDatatypes
   };
 
-  SyncSetupService(syncer::SyncService* sync_service, PrefService* prefs);
+  SyncSetupService(syncer::SyncService* sync_service);
   ~SyncSetupService() override;
 
   // Returns the |syncer::ModelType| associated to the given
@@ -111,7 +106,6 @@ class SyncSetupService : public KeyedService {
   void SetSyncEnabledWithoutChangingDatatypes(bool sync_enabled);
 
   syncer::SyncService* const sync_service_;
-  PrefService* const prefs_;
   syncer::ModelTypeSet user_selectable_types_;
 
   // Prevents Sync from running until configuration is complete.

ios/chrome/browser/sync/sync_setup_service_factory.cc

@@ -45,6 +45,5 @@ std::unique_ptr<KeyedService> SyncSetupServiceFactory::BuildServiceInstanceFor(
   ios::ChromeBrowserState* browser_state =
       ios::ChromeBrowserState::FromBrowserState(context);
   return std::make_unique<SyncSetupService>(
-      ProfileSyncServiceFactory::GetForBrowserState(browser_state),
-      browser_state->GetPrefs());
+      ProfileSyncServiceFactory::GetForBrowserState(browser_state));
 }

ios/chrome/browser/sync/sync_setup_service_mock.cc

@@ -4,9 +4,12 @@
 
 #include "ios/chrome/browser/sync/sync_setup_service_mock.h"
 
+SyncSetupServiceMock::SyncSetupServiceMock(syncer::SyncService* sync_service)
+    : SyncSetupService(sync_service) {}
+
 SyncSetupServiceMock::SyncSetupServiceMock(syncer::SyncService* sync_service,
                                            PrefService* prefs)
-    : SyncSetupService(sync_service, prefs) {}
+    : SyncSetupService(sync_service) {}
 
 SyncSetupServiceMock::~SyncSetupServiceMock() {
 }

ios/chrome/browser/sync/sync_setup_service_mock.h

@@ -9,9 +9,14 @@
 
 #include "testing/gmock/include/gmock/gmock.h"
 
+class PrefService;
+
 // Mock for the class that allows configuring sync on iOS.
 class SyncSetupServiceMock : public SyncSetupService {
  public:
+  SyncSetupServiceMock(syncer::SyncService* sync_service);
+  // TODO(crbug.com/884159): ¦prefs¦ is not required; get rid of this
+  // constructor once no tests use it anymore.
   SyncSetupServiceMock(syncer::SyncService* sync_service, PrefService* prefs);
   ~SyncSetupServiceMock();
 

ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator_unittest.mm

@@ -43,8 +43,7 @@ std::unique_ptr<KeyedService> CreateSyncSetupService(
       ios::ChromeBrowserState::FromBrowserState(context);
   syncer::SyncService* sync_service =
       ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state);
-  return std::make_unique<SyncSetupServiceMock>(
-      sync_service, chrome_browser_state->GetPrefs());
+  return std::make_unique<SyncSetupServiceMock>(sync_service);
 }
 
 class ProfileSyncServiceMockForRecentTabsTableCoordinator

ios/chrome/browser/ui/settings/sync_encryption_passphrase_collection_view_controller_unittest.mm

@@ -49,8 +49,7 @@ class SyncEncryptionPassphraseCollectionViewControllerTest
         ios::ChromeBrowserState::FromBrowserState(context);
     syncer::SyncService* sync_service =
         ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state);
-    return std::make_unique<SyncSetupServiceMock>(
-        sync_service, chrome_browser_state->GetPrefs());
+    return std::make_unique<SyncSetupServiceMock>(sync_service);
   }
 
   void TurnSyncPassphraseErrorOn() {

ios/chrome/browser/ui/settings/sync_settings_collection_view_controller_unittest.mm

@@ -52,9 +52,8 @@ using testing::Return;
 
 class SyncSetupServiceMockThatFails : public SyncSetupServiceMock {
  public:
-  SyncSetupServiceMockThatFails(browser_sync::ProfileSyncService* sync_service,
-                                PrefService* prefs)
-      : SyncSetupServiceMock(sync_service, prefs) {}
+  SyncSetupServiceMockThatFails(browser_sync::ProfileSyncService* sync_service)
+      : SyncSetupServiceMock(sync_service) {}
   bool IsSyncEnabled() const override { return sync_enabled_; }
   void SetSyncEnabled(bool sync_enabled) override {}
   bool IsSyncingAllDataTypes() const override { return sync_all_; }
@@ -78,9 +77,8 @@ bool SyncSetupServiceMockThatFails::sync_all_ = true;
 class SyncSetupServiceMockThatSucceeds : public SyncSetupServiceMockThatFails {
  public:
   SyncSetupServiceMockThatSucceeds(
-      browser_sync::ProfileSyncService* sync_service,
-      PrefService* prefs)
-      : SyncSetupServiceMockThatFails(sync_service, prefs) {}
+      browser_sync::ProfileSyncService* sync_service)
+      : SyncSetupServiceMockThatFails(sync_service) {}
   void SetSyncEnabled(bool sync_enabled) override {
     sync_enabled_ = sync_enabled;
   }
@@ -99,8 +97,7 @@ class SyncSettingsCollectionViewControllerTest
         ios::ChromeBrowserState::FromBrowserState(context);
     browser_sync::ProfileSyncService* sync_service =
         ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state);
-    return std::make_unique<NiceMock<SyncSetupServiceMock>>(
-        sync_service, chrome_browser_state->GetPrefs());
+    return std::make_unique<NiceMock<SyncSetupServiceMock>>(sync_service);
   }
 
   static std::unique_ptr<KeyedService> CreateSucceedingSyncSetupService(
@@ -110,7 +107,7 @@ class SyncSettingsCollectionViewControllerTest
     browser_sync::ProfileSyncService* sync_service =
         ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state);
     return std::make_unique<NiceMock<SyncSetupServiceMockThatSucceeds>>(
-        sync_service, chrome_browser_state->GetPrefs());
+        sync_service);
   }
 
   static std::unique_ptr<KeyedService> CreateFailingSyncSetupService(
@@ -120,7 +117,7 @@ class SyncSettingsCollectionViewControllerTest
     browser_sync::ProfileSyncService* sync_service =
         ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state);
     return std::make_unique<NiceMock<SyncSetupServiceMockThatFails>>(
-        sync_service, chrome_browser_state->GetPrefs());
+        sync_service);
   }
 
   static std::unique_ptr<KeyedService> CreateProfileSyncService(