Commit 5a785c38 authored by Thomas Tangl's avatar Thomas Tangl Committed by Commit Bot

[unified-consent] Suppress consent bump when user has custom passphrase

When a custom passphrase is recognized for a user that is
eligible for the consent bump otherwise, the consent bump
will be suppressed.

Note: Checking whether a user has a custom passphrase takes
some time, because this information is only available when
the sync engine is initialized.

Bug: 888756
Change-Id: Ie661769cb65cfd2b432ed433ad416e944a979fce
Reviewed-on: https://chromium-review.googlesource.com/c/1261441
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597079}
parent 3feb1854
...@@ -51,8 +51,10 @@ enum class ConsentBumpSuppressReason { ...@@ -51,8 +51,10 @@ enum class ConsentBumpSuppressReason {
// The user was eligible for seeing the consent bump, but turned an // The user was eligible for seeing the consent bump, but turned an
// on-by-default privacy setting off. // on-by-default privacy setting off.
kUserTurnedPrivacySettingOff, kUserTurnedPrivacySettingOff,
// The user has a custom passphrase tied to their sync account.
kCustomPassphrase,
kMaxValue = kUserTurnedPrivacySettingOff kMaxValue = kCustomPassphrase
}; };
// Google services that can be toggled in user settings. // Google services that can be toggled in user settings.
......
...@@ -228,6 +228,7 @@ void UnifiedConsentService::RecordConsentBumpSuppressReason( ...@@ -228,6 +228,7 @@ void UnifiedConsentService::RecordConsentBumpSuppressReason(
case metrics::ConsentBumpSuppressReason::kUserSignedOut: case metrics::ConsentBumpSuppressReason::kUserSignedOut:
case metrics::ConsentBumpSuppressReason::kUserTurnedSyncDatatypeOff: case metrics::ConsentBumpSuppressReason::kUserTurnedSyncDatatypeOff:
case metrics::ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff: case metrics::ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff:
case metrics::ConsentBumpSuppressReason::kCustomPassphrase:
pref_service_->SetBoolean(prefs::kShouldShowUnifiedConsentBump, false); pref_service_->SetBoolean(prefs::kShouldShowUnifiedConsentBump, false);
break; break;
case metrics::ConsentBumpSuppressReason::kSyncPaused: case metrics::ConsentBumpSuppressReason::kSyncPaused:
...@@ -294,11 +295,18 @@ void UnifiedConsentService::OnStateChanged(syncer::SyncService* sync) { ...@@ -294,11 +295,18 @@ void UnifiedConsentService::OnStateChanged(syncer::SyncService* sync) {
if (GetMigrationState() == MigrationState::kInProgressWaitForSyncInit) if (GetMigrationState() == MigrationState::kInProgressWaitForSyncInit)
UpdateSettingsForMigration(); UpdateSettingsForMigration();
if (sync_service_->IsUsingSecondaryPassphrase() && IsUnifiedConsentGiven()) { if (sync_service_->IsUsingSecondaryPassphrase()) {
// Force off unified consent given when the user sets a custom passphrase. if (ShouldShowConsentBump()) {
SetUnifiedConsentGiven(false); // Do not show the consent bump when the user has a custom passphrase.
RecordUnifiedConsentRevoked( RecordConsentBumpSuppressReason(
metrics::UnifiedConsentRevokeReason::kCustomPassphrase); metrics::ConsentBumpSuppressReason::kCustomPassphrase);
}
if (IsUnifiedConsentGiven()) {
// Force off unified consent given when the user sets a custom passphrase.
SetUnifiedConsentGiven(false);
RecordUnifiedConsentRevoked(
metrics::UnifiedConsentRevokeReason::kCustomPassphrase);
}
} }
syncer::SyncPrefs sync_prefs(pref_service_); syncer::SyncPrefs sync_prefs(pref_service_);
......
...@@ -781,4 +781,35 @@ TEST_F(UnifiedConsentServiceTest, ...@@ -781,4 +781,35 @@ TEST_F(UnifiedConsentServiceTest,
metrics::ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff, 1); metrics::ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff, 1);
} }
TEST_F(UnifiedConsentServiceTest, ConsentBump_SuppressedWithCustomPassphrase) {
base::HistogramTester histogram_tester;
// Setup sync account with custom passphrase, such that it would be eligible
// for the consent bump without custom passphrase.
identity_test_environment_.SetPrimaryAccount("testaccount");
sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
sync_service_.SetIsUsingPassphrase(true);
syncer::SyncPrefs sync_prefs(&pref_service_);
EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
sync_service_.SetTransportState(
syncer::SyncService::TransportState::INITIALIZING);
EXPECT_FALSE(sync_service_.IsEngineInitialized());
// Before sync is initialized, the user is eligible for seeing the consent
// bump.
CreateConsentService(true /* client_services_on_by_default */);
EXPECT_TRUE(AreAllNonPersonalizedServicesEnabled());
EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
// When sync is initialized, it fires the observer in the consent service.
// This will suppress the consent bump.
sync_service_.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
EXPECT_TRUE(sync_service_.IsEngineInitialized());
sync_service_.FireStateChanged();
EXPECT_FALSE(consent_service_->ShouldShowConsentBump());
histogram_tester.ExpectUniqueSample(
"UnifiedConsent.ConsentBump.SuppressReason",
metrics::ConsentBumpSuppressReason::kCustomPassphrase, 1);
}
} // namespace unified_consent } // namespace unified_consent
...@@ -49397,6 +49397,7 @@ Full version information for the fingerprint enum values: ...@@ -49397,6 +49397,7 @@ Full version information for the fingerprint enum values:
<int value="6" label="Sync was paused"/> <int value="6" label="Sync was paused"/>
<int value="7" label="User turned off sync data type"/> <int value="7" label="User turned off sync data type"/>
<int value="8" label="User turned off on-by-default privacy setting"/> <int value="8" label="User turned off on-by-default privacy setting"/>
<int value="9" label="User has a custom passphrase for their sync account"/>
</enum> </enum>
<enum name="UnifiedConsentRevokeReason"> <enum name="UnifiedConsentRevokeReason">
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