Move source of truth for GetPreferred/ChosenDataTypes to SyncUserSettings
Bug: 884159
Change-Id: I1f9e743cfc364aacafe62b13c2ffa2582c49d418
Reviewed-on: https://chromium-review.googlesource.com/c/1382483
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622400}diff --git a/chrome/browser/ui/webui/browsing_history_handler_unittest.cc b/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
index 12c6b31..3a11aec 100644
--- a/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
+++ b/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
@@ -54,9 +54,7 @@
explicit TestSyncService(Profile* profile)
: browser_sync::TestProfileSyncService(
CreateProfileSyncServiceParamsForTest(profile)),
- state_(TransportState::ACTIVE) {
- GetUserSettings()->SetFirstSetupComplete();
- }
+ state_(TransportState::ACTIVE) {}
TransportState GetTransportState() const override { return state_; }
@@ -134,8 +132,11 @@
private:
static std::unique_ptr<KeyedService> BuildFakeSyncService(
content::BrowserContext* context) {
- return std::make_unique<TestSyncService>(
+ auto service = std::make_unique<TestSyncService>(
static_cast<TestingProfile*>(context));
+ service->Initialize();
+ service->GetUserSettings()->SetFirstSetupComplete();
+ return service;
}
static std::unique_ptr<KeyedService> BuildFakeWebHistoryService(
@@ -143,11 +144,11 @@
std::unique_ptr<history::FakeWebHistoryService> service =
std::make_unique<history::FakeWebHistoryService>();
service->SetupFakeResponse(true /* success */, net::HTTP_OK);
- return std::move(service);
+ return service;
}
- TestSyncService* sync_service_;
- history::FakeWebHistoryService* web_history_service_;
+ TestSyncService* sync_service_ = nullptr;
+ history::FakeWebHistoryService* web_history_service_ = nullptr;
std::unique_ptr<content::TestWebUI> web_ui_;
};
diff --git a/components/browser_sync/abstract_profile_sync_service_test.cc b/components/browser_sync/abstract_profile_sync_service_test.cc
index dc922c87..e4372af 100644
--- a/components/browser_sync/abstract_profile_sync_service_test.cc
+++ b/components/browser_sync/abstract_profile_sync_service_test.cc
@@ -174,7 +174,7 @@
EXPECT_CALL(*components, CreateSyncEngine(_, _, _, _))
.WillOnce(Return(ByMove(std::move(engine))));
- sync_service_->GetUserSettings()->SetFirstSetupComplete();
+ sync_service_->sync_prefs()->SetFirstSetupComplete();
}
CreateRootHelper::CreateRootHelper(AbstractProfileSyncServiceTest* test,
diff --git a/components/browser_sync/profile_sync_service.cc b/components/browser_sync/profile_sync_service.cc
index c3c86c7..b6410c16 100644
--- a/components/browser_sync/profile_sync_service.cc
+++ b/components/browser_sync/profile_sync_service.cc
@@ -164,7 +164,6 @@
local_device_(std::move(init_params.local_device_info_provider)),
sync_prefs_(sync_client_->GetPrefService()),
identity_manager_(init_params.identity_manager),
- user_settings_(this, &sync_prefs_),
auth_manager_(std::make_unique<SyncAuthManager>(
&sync_prefs_,
identity_manager_,
@@ -283,6 +282,8 @@
->GetControllerDelegate()
.get()));
+ user_settings_ = std::make_unique<SyncUserSettingsImpl>(this, &sync_prefs_);
+
sync_prefs_.AddSyncPrefObserver(this);
// If sync is disallowed by policy, clean up.
@@ -730,11 +731,11 @@
}
syncer::SyncUserSettings* ProfileSyncService::GetUserSettings() {
- return &user_settings_;
+ return user_settings_.get();
}
const syncer::SyncUserSettings* ProfileSyncService::GetUserSettings() const {
- return &user_settings_;
+ return user_settings_.get();
}
int ProfileSyncService::GetDisableReasons() const {
@@ -745,7 +746,7 @@
DCHECK(IsSyncAllowedByFlag());
int result = DISABLE_REASON_NONE;
- if (!user_settings_.IsSyncAllowedByPlatform()) {
+ if (!user_settings_->IsSyncAllowedByPlatform()) {
result = result | DISABLE_REASON_PLATFORM_OVERRIDE;
}
if (sync_prefs_.IsManaged() || sync_disabled_by_admin_) {
@@ -827,7 +828,7 @@
bool ProfileSyncService::IsFirstSetupComplete() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- return user_settings_.IsFirstSetupComplete();
+ return user_settings_->IsFirstSetupComplete();
}
void ProfileSyncService::UpdateLastSyncedTime() {
@@ -985,7 +986,7 @@
// Auto-start means IsFirstSetupComplete gets set automatically.
if (start_behavior_ == AUTO_START && !IsFirstSetupComplete()) {
// This will trigger a configure if it completes setup.
- user_settings_.SetFirstSetupComplete();
+ user_settings_->SetFirstSetupComplete();
} else if (CanConfigureDataTypes(/*bypass_setup_in_progress_check=*/false)) {
// Datatype downloads on restart are generally due to newly supported
// datatypes (although it's also possible we're picking up where a failed
@@ -1414,15 +1415,7 @@
syncer::ModelTypeSet ProfileSyncService::GetPreferredDataTypes() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- syncer::ModelTypeSet preferred_types =
- Union(sync_prefs_.GetPreferredDataTypes(GetRegisteredDataTypes()),
- syncer::ControlTypes());
- if (IsLocalSyncEnabled()) {
- preferred_types.Remove(syncer::APP_LIST);
- preferred_types.Remove(syncer::USER_CONSENTS);
- preferred_types.Remove(syncer::USER_EVENTS);
- }
- return Union(preferred_types, GetForcedDataTypes());
+ return Union(user_settings_->GetPreferredDataTypes(), GetForcedDataTypes());
}
syncer::ModelTypeSet ProfileSyncService::GetActiveDataTypes() const {
diff --git a/components/browser_sync/profile_sync_service.h b/components/browser_sync/profile_sync_service.h
index 336550ff..8bc2573 100644
--- a/components/browser_sync/profile_sync_service.h
+++ b/components/browser_sync/profile_sync_service.h
@@ -548,7 +548,7 @@
// email address and sign-out upon error.
identity::IdentityManager* const identity_manager_;
- SyncUserSettingsImpl user_settings_;
+ std::unique_ptr<SyncUserSettingsImpl> user_settings_;
// Handles tracking of the authenticated account and acquiring access tokens.
// Only null after Shutdown().
diff --git a/components/browser_sync/profile_sync_service_startup_unittest.cc b/components/browser_sync/profile_sync_service_startup_unittest.cc
index 679a4e2..561fceb 100644
--- a/components/browser_sync/profile_sync_service_startup_unittest.cc
+++ b/components/browser_sync/profile_sync_service_startup_unittest.cc
@@ -343,10 +343,10 @@
TEST_F(ProfileSyncServiceStartupTest, StartNoCredentials) {
// We're already signed in, but don't have a refresh token.
SimulateTestUserSigninWithoutRefreshToken();
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
EXPECT_CALL(*data_type_manager, Configure(_, _));
@@ -366,11 +366,10 @@
TEST_F(ProfileSyncServiceStartupTest, StartInvalidCredentials) {
SimulateTestUserSignin();
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
-
// Tell the engine to stall while downloading control types (simulating an
// auth error).
FakeSyncEngine* fake_engine = SetUpFakeSyncEngine();
@@ -467,9 +466,9 @@
}
TEST_F(ProfileSyncServiceWithoutStandaloneTransportStartupTest, StopSync) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
ON_CALL(*data_type_manager, state())
@@ -486,9 +485,9 @@
}
TEST_F(ProfileSyncServiceWithStandaloneTransportStartupTest, StopSync) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
ON_CALL(*data_type_manager, state())
@@ -511,9 +510,9 @@
}
TEST_F(ProfileSyncServiceWithoutStandaloneTransportStartupTest, DisableSync) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
ON_CALL(*data_type_manager, state())
@@ -530,9 +529,9 @@
}
TEST_F(ProfileSyncServiceWithStandaloneTransportStartupTest, DisableSync) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
ON_CALL(*data_type_manager, state())
@@ -564,9 +563,9 @@
pref_service()->ClearPref(syncer::SyncPrefs::GetPrefNameForDataType(type));
}
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
EXPECT_CALL(*data_type_manager, Configure(_, _));
@@ -587,9 +586,9 @@
sync_prefs()->SetPreferredDataTypes(/*keep_everything_synced=*/false,
syncer::UserTypes(), {syncer::BOOKMARKS});
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
EXPECT_CALL(*data_type_manager, Configure(_, _));
@@ -619,9 +618,9 @@
}
TEST_F(ProfileSyncServiceWithoutStandaloneTransportStartupTest, SwitchManaged) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
EXPECT_CALL(*data_type_manager, Configure(_, _));
@@ -670,9 +669,9 @@
}
TEST_F(ProfileSyncServiceWithStandaloneTransportStartupTest, SwitchManaged) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
EXPECT_CALL(*data_type_manager, Configure(_, _));
@@ -728,9 +727,9 @@
}
TEST_F(ProfileSyncServiceStartupTest, StartFailure) {
+ sync_prefs()->SetFirstSetupComplete();
CreateSyncService(ProfileSyncService::MANUAL_START);
SimulateTestUserSignin();
- sync_service()->GetUserSettings()->SetFirstSetupComplete();
SetUpFakeSyncEngine();
DataTypeManagerMock* data_type_manager = SetUpDataTypeManagerMock();
DataTypeManager::ConfigureStatus status = DataTypeManager::ABORTED;
diff --git a/components/browser_sync/sync_user_settings_impl.cc b/components/browser_sync/sync_user_settings_impl.cc
index 67272f9c..d3b22c3 100644
--- a/components/browser_sync/sync_user_settings_impl.cc
+++ b/components/browser_sync/sync_user_settings_impl.cc
@@ -12,7 +12,9 @@
SyncUserSettingsImpl::SyncUserSettingsImpl(ProfileSyncService* service,
syncer::SyncPrefs* prefs)
- : service_(service), prefs_(prefs) {}
+ : service_(service),
+ prefs_(prefs),
+ registered_types_(service_->GetRegisteredDataTypes()) {}
SyncUserSettingsImpl::~SyncUserSettingsImpl() = default;
@@ -58,15 +60,14 @@
}
syncer::ModelTypeSet SyncUserSettingsImpl::GetChosenDataTypes() const {
- syncer::ModelTypeSet types = service_->GetPreferredDataTypes();
+ syncer::ModelTypeSet types = GetPreferredDataTypes();
types.RetainAll(syncer::UserSelectableTypes());
return types;
}
void SyncUserSettingsImpl::SetChosenDataTypes(bool sync_everything,
syncer::ModelTypeSet types) {
- // TODO(crbug.com/884159): Write to prefs directly (might be tricky because it
- // needs the registered types).
+ // TODO(crbug.com/884159): Write to prefs directly.
service_->OnUserChoseDatatypes(sync_everything, types);
}
@@ -130,4 +131,15 @@
return service_->SetDecryptionPassphrase(passphrase);
}
+syncer::ModelTypeSet SyncUserSettingsImpl::GetPreferredDataTypes() const {
+ syncer::ModelTypeSet types = Union(
+ prefs_->GetPreferredDataTypes(registered_types_), syncer::ControlTypes());
+ if (prefs_->IsLocalSyncEnabled()) {
+ types.Remove(syncer::APP_LIST);
+ types.Remove(syncer::USER_CONSENTS);
+ types.Remove(syncer::USER_EVENTS);
+ }
+ return types;
+}
+
} // namespace browser_sync
diff --git a/components/browser_sync/sync_user_settings_impl.h b/components/browser_sync/sync_user_settings_impl.h
index 59bc7639..096fab112 100644
--- a/components/browser_sync/sync_user_settings_impl.h
+++ b/components/browser_sync/sync_user_settings_impl.h
@@ -51,9 +51,12 @@
void SetEncryptionPassphrase(const std::string& passphrase) override;
bool SetDecryptionPassphrase(const std::string& passphrase) override;
+ syncer::ModelTypeSet GetPreferredDataTypes() const;
+
private:
ProfileSyncService* const service_;
syncer::SyncPrefs* const prefs_;
+ const syncer::ModelTypeSet registered_types_;
// Whether sync is currently allowed on this platform.
bool sync_allowed_by_platform_ = true;
