Skip to content

GitLab

T
tangled
Commit ca820fa4 authored by Mihai Sardarescu's avatar Mihai Sardarescu Committed by Commit Bot
Browse files
Options

URL-keyed personalized data collection is enabled when sync is enabled. 

The Activity & Interactions toggle that used to control user events was
removed from settings and thus URL-keyed and Gaia-keyed user events
are enabled iff history sync is active. This CL changes the
personalized version of UrlKeyedDataCollectionConsentHelper to check
whether history sync is enabled instead of checking the user events.

Bug: 906031
Change-Id: I693474f21ced13c7a4bee634e9a92adcf7bb0cf9
Reviewed-on: https://chromium-review.googlesource.com/c/1352150Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Reviewed-by: default avatarThomas Tangl <tangltom@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611136}
parent e98161eb
Hide whitespace changes
Inline Side-by-side
Showing with 23 additions and 51 deletions
components/unified_consent/url_keyed_data_collection_consent_helper.cc
View file @ ca820fa4
......@@ -163,16 +163,9 @@ UrlKeyedDataCollectionConsentHelper::NewAnonymizedDataCollectionConsentHelper(
std::unique_ptr<UrlKeyedDataCollectionConsentHelper>
UrlKeyedDataCollectionConsentHelper::NewPersonalizedDataCollectionConsentHelper(
syncer::SyncService* sync_service) {
if (IsUnifiedConsentFeatureEnabled()) {
return std::make_unique<SyncBasedUrlKeyedDataCollectionConsentHelper>(
sync_service, std::set<syncer::ModelType>(
{syncer::ModelType::HISTORY_DELETE_DIRECTIVES,
syncer::ModelType::USER_EVENTS}));
} else {
return std::make_unique<SyncBasedUrlKeyedDataCollectionConsentHelper>(
sync_service, std::set<syncer::ModelType>(
{syncer::ModelType::HISTORY_DELETE_DIRECTIVES}));
}
return std::make_unique<SyncBasedUrlKeyedDataCollectionConsentHelper>(
sync_service, std::set<syncer::ModelType>(
{syncer::ModelType::HISTORY_DELETE_DIRECTIVES}));
}
void UrlKeyedDataCollectionConsentHelper::AddObserver(Observer* observer) {
......
components/unified_consent/url_keyed_data_collection_consent_helper.h
View file @ ca820fa4
......@@ -38,10 +38,10 @@ class UrlKeyedDataCollectionConsentHelper {
// |prefs::kUrlKeyedAnonymizedDataCollectionEnabled| is set to true.
//
// 2. If |is_unified_consent_enabled| is false, then the instance is backed by
// the sync service. Url-keyed data collection is enabled if sync is active
// and if sync history is enabled.
// the sync service. Url-keyed data collection is enabled if history sync
// has an active upload state.
//
// Note: |pref_service| must outlive the retuned instance.
// Note: |pref_service| must outlive the returned instance.
static std::unique_ptr<UrlKeyedDataCollectionConsentHelper>
NewAnonymizedDataCollectionConsentHelper(PrefService* pref_service,
syncer::SyncService* sync_service);
......@@ -51,11 +51,8 @@ class UrlKeyedDataCollectionConsentHelper {
// the client needs to check whether the user has granted consent for
// URL-keyed data collection keyed by their Google account.
//
// Implementation-wise we distinguish the following cases:
// 1. If |is_unified_consent_enabled| is true then URL-keyed data collection
// is enabled if sync is active and if sync event logger is enabled.
// 2. If |is_unified_consent_enabled| is false then URL-keyed data collection
// is enabled if sync is active and if sync history is enabled.
// Implementation-wise URL-keyed data collection is enabled if history sync
// has an active upload state.
static std::unique_ptr<UrlKeyedDataCollectionConsentHelper>
NewPersonalizedDataCollectionConsentHelper(syncer::SyncService* sync_service);
......
components/unified_consent/url_keyed_data_collection_consent_helper_unittest.cc
View file @ ca820fa4
......@@ -55,12 +55,12 @@ class UrlKeyedDataCollectionConsentHelperTest
void OnUrlKeyedDataCollectionConsentStateChanged(
UrlKeyedDataCollectionConsentHelper* consent_helper) override {
state_changed_notifications.push_back(consent_helper->IsEnabled());
state_changed_notifications_.push_back(consent_helper->IsEnabled());
}
protected:
sync_preferences::TestingPrefServiceSyncable pref_service_;
std::vector<bool> state_changed_notifications;
std::vector<bool> state_changed_notifications_;
TestSyncService sync_service_;
};
......@@ -74,20 +74,20 @@ TEST_F(UrlKeyedDataCollectionConsentHelperTest,
&sync_service_);
helper->AddObserver(this);
EXPECT_FALSE(helper->IsEnabled());
EXPECT_TRUE(state_changed_notifications.empty());
EXPECT_TRUE(state_changed_notifications_.empty());
pref_service_.SetBoolean(prefs::kUrlKeyedAnonymizedDataCollectionEnabled,
true);
EXPECT_TRUE(helper->IsEnabled());
ASSERT_EQ(1U, state_changed_notifications.size());
EXPECT_TRUE(state_changed_notifications[0]);
ASSERT_EQ(1U, state_changed_notifications_.size());
EXPECT_TRUE(state_changed_notifications_[0]);
state_changed_notifications.clear();
state_changed_notifications_.clear();
pref_service_.SetBoolean(prefs::kUrlKeyedAnonymizedDataCollectionEnabled,
false);
EXPECT_FALSE(helper->IsEnabled());
ASSERT_EQ(1U, state_changed_notifications.size());
EXPECT_FALSE(state_changed_notifications[0]);
ASSERT_EQ(1U, state_changed_notifications_.size());
EXPECT_FALSE(state_changed_notifications_[0]);
helper->RemoveObserver(this);
}
......@@ -101,12 +101,12 @@ TEST_F(UrlKeyedDataCollectionConsentHelperTest,
&sync_service_);
helper->AddObserver(this);
EXPECT_FALSE(helper->IsEnabled());
EXPECT_TRUE(state_changed_notifications.empty());
EXPECT_TRUE(state_changed_notifications_.empty());
sync_service_.AddActiveDataType(syncer::ModelType::HISTORY_DELETE_DIRECTIVES);
sync_service_.FireOnStateChangeOnAllObservers();
EXPECT_TRUE(helper->IsEnabled());
EXPECT_EQ(1U, state_changed_notifications.size());
EXPECT_EQ(1U, state_changed_notifications_.size());
helper->RemoveObserver(this);
}
......@@ -122,7 +122,7 @@ TEST_F(UrlKeyedDataCollectionConsentHelperTest,
}
TEST_F(UrlKeyedDataCollectionConsentHelperTest,
PersonalizeddDataCollection_UnifiedConsentEnabled) {
PersonalizedDataCollection_UnifiedConsentEnabled) {
ScopedUnifiedConsent scoped_unified_consent(
UnifiedConsentFeatureState::kEnabledNoBump);
std::unique_ptr<UrlKeyedDataCollectionConsentHelper> helper =
......@@ -130,30 +130,12 @@ TEST_F(UrlKeyedDataCollectionConsentHelperTest,
NewPersonalizedDataCollectionConsentHelper(&sync_service_);
helper->AddObserver(this);
EXPECT_FALSE(helper->IsEnabled());
EXPECT_TRUE(state_changed_notifications.empty());
EXPECT_TRUE(state_changed_notifications_.empty());
// Peronalized data collection is disabled when only USER_EVENTS are enabled.
sync_service_.AddActiveDataType(syncer::ModelType::USER_EVENTS);
sync_service_.FireOnStateChangeOnAllObservers();
EXPECT_FALSE(helper->IsEnabled());
EXPECT_TRUE(state_changed_notifications.empty());
// Peronalized data collection is disabled when only HISTORY_DELETE_DIRECTIVES
// are enabled.
sync_service_.SetActiveDataTypes({});
sync_service_.AddActiveDataType(syncer::ModelType::HISTORY_DELETE_DIRECTIVES);
sync_service_.FireOnStateChangeOnAllObservers();
EXPECT_FALSE(helper->IsEnabled());
EXPECT_TRUE(state_changed_notifications.empty());
// Personalized data collection is enabled iff USER_EVENTS and
// HISTORY_DELETE_DIRECTIVES are enabled.
sync_service_.SetActiveDataTypes({});
sync_service_.AddActiveDataType(syncer::ModelType::HISTORY_DELETE_DIRECTIVES);
sync_service_.AddActiveDataType(syncer::ModelType::USER_EVENTS);
sync_service_.FireOnStateChangeOnAllObservers();
EXPECT_TRUE(helper->IsEnabled());
EXPECT_EQ(1U, state_changed_notifications.size());
EXPECT_EQ(1U, state_changed_notifications_.size());
helper->RemoveObserver(this);
}
......@@ -166,12 +148,12 @@ TEST_F(UrlKeyedDataCollectionConsentHelperTest,
NewPersonalizedDataCollectionConsentHelper(&sync_service_);
helper->AddObserver(this);
EXPECT_FALSE(helper->IsEnabled());
EXPECT_TRUE(state_changed_notifications.empty());
EXPECT_TRUE(state_changed_notifications_.empty());
sync_service_.AddActiveDataType(syncer::ModelType::HISTORY_DELETE_DIRECTIVES);
sync_service_.FireOnStateChangeOnAllObservers();
EXPECT_TRUE(helper->IsEnabled());
EXPECT_EQ(1U, state_changed_notifications.size());
EXPECT_EQ(1U, state_changed_notifications_.size());
helper->RemoveObserver(this);
}
......
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