Commit 26fa7866 authored by vitaliii's avatar vitaliii Committed by Commit Bot

[Sync::Consent] Don't link UserEventService if separate datatype is used.

Previously ConsentAuditor did get UserEventService even when the
separate consents datatype was used. In this CL, the UserEventService is
just not requested from ConsentAuditor factory at all in this case (on
all platforms). Also DCHECKs in ConsentAuditor are adjusted to check only
the correct dependency based on the feature.

Bug: 851438
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I756e595b826e084d1b2f9905d2827cae1160fedd
Reviewed-on: https://chromium-review.googlesource.com/1124333Reviewed-by: default avatarMarkus Heintz <markusheintz@chromium.org>
Commit-Queue: vitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572580}
parent ba94cbfe
...@@ -51,7 +51,8 @@ KeyedService* ConsentAuditorFactory::BuildServiceInstanceFor( ...@@ -51,7 +51,8 @@ KeyedService* ConsentAuditorFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const { content::BrowserContext* context) const {
Profile* profile = static_cast<Profile*>(context); Profile* profile = static_cast<Profile*>(context);
std::unique_ptr<syncer::ConsentSyncBridge> bridge; std::unique_ptr<syncer::ConsentSyncBridge> consent_sync_bridge;
syncer::UserEventService* user_event_service = nullptr;
if (base::FeatureList::IsEnabled(switches::kSyncUserConsentSeparateType)) { if (base::FeatureList::IsEnabled(switches::kSyncUserConsentSeparateType)) {
syncer::OnceModelTypeStoreFactory store_factory = syncer::OnceModelTypeStoreFactory store_factory =
browser_sync::ProfileSyncService::GetModelTypeStoreFactory( browser_sync::ProfileSyncService::GetModelTypeStoreFactory(
...@@ -61,13 +62,15 @@ KeyedService* ConsentAuditorFactory::BuildServiceInstanceFor( ...@@ -61,13 +62,15 @@ KeyedService* ConsentAuditorFactory::BuildServiceInstanceFor(
syncer::USER_CONSENTS, syncer::USER_CONSENTS,
base::BindRepeating(&syncer::ReportUnrecoverableError, base::BindRepeating(&syncer::ReportUnrecoverableError,
chrome::GetChannel())); chrome::GetChannel()));
bridge = std::make_unique<syncer::ConsentSyncBridgeImpl>( consent_sync_bridge = std::make_unique<syncer::ConsentSyncBridgeImpl>(
std::move(store_factory), std::move(change_processor)); std::move(store_factory), std::move(change_processor));
} else {
user_event_service =
browser_sync::UserEventServiceFactory::GetForProfile(profile);
} }
// TODO(vitaliii): Don't create UserEventService when it won't be used.
return new consent_auditor::ConsentAuditor( return new consent_auditor::ConsentAuditor(
profile->GetPrefs(), std::move(bridge), profile->GetPrefs(), std::move(consent_sync_bridge), user_event_service,
browser_sync::UserEventServiceFactory::GetForProfile(profile),
// The browser version and locale do not change runtime, so we can pass // The browser version and locale do not change runtime, so we can pass
// them directly. // them directly.
version_info::GetVersionNumber(), version_info::GetVersionNumber(),
......
...@@ -90,11 +90,12 @@ ConsentAuditor::ConsentAuditor( ...@@ -90,11 +90,12 @@ ConsentAuditor::ConsentAuditor(
user_event_service_(user_event_service), user_event_service_(user_event_service),
app_version_(app_version), app_version_(app_version),
app_locale_(app_locale) { app_locale_(app_locale) {
DCHECK(!IsSeparateConsentTypeEnabled() || consent_sync_bridge_); if (IsSeparateConsentTypeEnabled()) {
DCHECK(consent_sync_bridge_);
} else {
DCHECK(user_event_service_);
}
DCHECK(pref_service_); DCHECK(pref_service_);
// TODO(vitaliii): Don't require user_event_service when the separate datatype
// is enabled.
DCHECK(user_event_service_);
} }
ConsentAuditor::~ConsentAuditor() {} ConsentAuditor::~ConsentAuditor() {}
......
...@@ -57,12 +57,9 @@ std::unique_ptr<KeyedService> ConsentAuditorFactory::BuildServiceInstanceFor( ...@@ -57,12 +57,9 @@ std::unique_ptr<KeyedService> ConsentAuditorFactory::BuildServiceInstanceFor(
web::BrowserState* browser_state) const { web::BrowserState* browser_state) const {
ios::ChromeBrowserState* ios_browser_state = ios::ChromeBrowserState* ios_browser_state =
ios::ChromeBrowserState::FromBrowserState(browser_state); ios::ChromeBrowserState::FromBrowserState(browser_state);
// TODO(crbug.com/851438): Don't create user event service at all if it is not
// needed.
syncer::UserEventService* const user_event_service =
IOSUserEventServiceFactory::GetForBrowserState(ios_browser_state);
std::unique_ptr<syncer::ConsentSyncBridge> bridge; std::unique_ptr<syncer::ConsentSyncBridge> consent_sync_bridge;
syncer::UserEventService* user_event_service = nullptr;
if (base::FeatureList::IsEnabled(switches::kSyncUserConsentSeparateType)) { if (base::FeatureList::IsEnabled(switches::kSyncUserConsentSeparateType)) {
syncer::OnceModelTypeStoreFactory store_factory = syncer::OnceModelTypeStoreFactory store_factory =
browser_sync::ProfileSyncService::GetModelTypeStoreFactory( browser_sync::ProfileSyncService::GetModelTypeStoreFactory(
...@@ -72,12 +69,16 @@ std::unique_ptr<KeyedService> ConsentAuditorFactory::BuildServiceInstanceFor( ...@@ -72,12 +69,16 @@ std::unique_ptr<KeyedService> ConsentAuditorFactory::BuildServiceInstanceFor(
syncer::USER_CONSENTS, syncer::USER_CONSENTS,
base::BindRepeating(&syncer::ReportUnrecoverableError, base::BindRepeating(&syncer::ReportUnrecoverableError,
::GetChannel())); ::GetChannel()));
bridge = std::make_unique<syncer::ConsentSyncBridgeImpl>( consent_sync_bridge = std::make_unique<syncer::ConsentSyncBridgeImpl>(
std::move(store_factory), std::move(change_processor)); std::move(store_factory), std::move(change_processor));
} else {
user_event_service =
IOSUserEventServiceFactory::GetForBrowserState(ios_browser_state);
} }
return std::make_unique<consent_auditor::ConsentAuditor>( return std::make_unique<consent_auditor::ConsentAuditor>(
ios_browser_state->GetPrefs(), std::move(bridge), user_event_service, ios_browser_state->GetPrefs(), std::move(consent_sync_bridge),
user_event_service,
// The browser version and locale do not change runtime, so we can pass // The browser version and locale do not change runtime, so we can pass
// them directly. // them directly.
version_info::GetVersionNumber(), version_info::GetVersionNumber(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment