[unified-consent] Check eligibility for consent bump on every startup

The eligibility for showing the consent bump is checked on every
startup during the creation of the UnifiedConsentService.

Two metrics for the consent bump suppress reason are added:
 - User turned off a sync data type
 - User turned off a on-by-default privacy setting

Bug: 872435
Change-Id: I6187e88bebce1516aec72fd369281c34b89039be
Reviewed-on: https://chromium-review.googlesource.com/1188478
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585846}

components/unified_consent/unified_consent_service.cc

@@ -135,6 +135,9 @@ UnifiedConsentService::UnifiedConsentService(
   if (GetMigrationState() == MigrationState::kNotInitialized)
     MigrateProfileToUnifiedConsent();
 
+  // Check if this profile is still eligible for the consent bump.
+  CheckConsentBumpEligibility();
+
   service_client_->AddObserver(this);
   identity_manager_->AddObserver(this);
   sync_service_->AddObserver(this);
@@ -233,6 +236,8 @@ void UnifiedConsentService::RecordConsentBumpSuppressReason(
     case ConsentBumpSuppressReason::kPrivacySettingOff:
     case ConsentBumpSuppressReason::kSettingsOptIn:
     case ConsentBumpSuppressReason::kUserSignedOut:
+    case ConsentBumpSuppressReason::kUserTurnedSyncDatatypeOff:
+    case ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff:
       pref_service_->SetBoolean(prefs::kShouldShowUnifiedConsentBump, false);
       break;
     case ConsentBumpSuppressReason::kSyncPaused:
@@ -490,4 +495,23 @@ void UnifiedConsentService::RecordSettingsHistogram() {
     RecordSettingsHistogramSample(SettingsHistogramValue::kNone);
 }
 
+void UnifiedConsentService::CheckConsentBumpEligibility() {
+  // Only check eligility if the user was eligible before.
+  if (!ShouldShowConsentBump())
+    return;
+
+  syncer::ModelTypeSet user_types_without_user_events =
+      syncer::UserSelectableTypes();
+  user_types_without_user_events.Remove(syncer::USER_EVENTS);
+
+  if (!sync_service_->GetPreferredDataTypes().HasAll(
+          user_types_without_user_events)) {
+    RecordConsentBumpSuppressReason(
+        ConsentBumpSuppressReason::kUserTurnedSyncDatatypeOff);
+  } else if (!AreAllOnByDefaultPrivacySettingsOn()) {
+    RecordConsentBumpSuppressReason(
+        ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff);
+  }
+}
+
 }  //  namespace unified_consent

components/unified_consent/unified_consent_service.h

@@ -40,15 +40,27 @@ enum class MigrationState : int {
 // Used in histograms. Do not change existing values, append new values at the
 // end.
 enum class ConsentBumpSuppressReason {
+  // There is no suppress reason. The consent bump was shown.
   kNone,
+  // The user wasn't signed in during the migration.
   kNotSignedIn,
+  // The user wasn't syncing everything during the migration.
   kSyncEverythingOff,
+  // The user didn't have all on-by-default privacy settings enabled during
+  // migration.
   kPrivacySettingOff,
   kSettingsOptIn,
+  // The user was eligible for seeing the consent bump, but then signed out.
   kUserSignedOut,
   kSyncPaused,
+  // The user was eligible for seeing the consent bump, but turned an individual
+  // sync data type off.
+  kUserTurnedSyncDatatypeOff,
+  // The user was eligible for seeing the consent bump, but turned an
+  // on-by-default privacy setting off.
+  kUserTurnedPrivacySettingOff,
 
-  kMaxValue = kSyncPaused
+  kMaxValue = kUserTurnedPrivacySettingOff
 };
 
 // Google services that can be toggled in user settings.
@@ -153,6 +165,12 @@ class UnifiedConsentService : public KeyedService,
   // kNone is recorded when none of the other buckets are recorded.
   void RecordSettingsHistogram();
 
+  // This method is called on startup to check the eligibility criteria for
+  // showing the consent bump. The check is only done when the profile was
+  // eligible before. If the user is not eligible anymore, the
+  // kShouldShowUnifiedConsentBump pref is set to false.
+  void CheckConsentBumpEligibility();
+
   std::unique_ptr<UnifiedConsentServiceClient> service_client_;
   PrefService* pref_service_;
   identity::IdentityManager* identity_manager_;

components/unified_consent/unified_consent_service_unittest.cc

@@ -362,7 +362,7 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndAllServicesOn) {
       syncer::SyncService::TransportState::PENDING_DESIRED_CONFIGURATION);
   EXPECT_FALSE(sync_service_.IsSyncActive());
 
-  CreateConsentService(true /* client services on by default */);
+  CreateConsentService(true /* client_services_on_by_default */);
   EXPECT_TRUE(AreAllNonPersonalizedServicesEnabled());
   // After the creation of the consent service, the profile started to migrate
   // (but waiting for sync init) and |ShouldShowConsentBump| should return true.
@@ -522,7 +522,7 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasSyncingEverything) {
   EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
 
   // Migrate
-  CreateConsentService(true /* client services on by default */);
+  CreateConsentService(true /* client_services_on_by_default */);
   // Check expectations after migration.
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
@@ -636,4 +636,99 @@ TEST_F(UnifiedConsentServiceTest, SettingsHistogram_NoUnifiedConsentGiven) {
       SettingsHistogramValue::kSpellCheck, 1);
 }
 
+TEST_F(UnifiedConsentServiceTest, ConsentBump_EligibleOnSecondStartup) {
+  base::HistogramTester histogram_tester;
+
+  identity_test_environment_.SetPrimaryAccount("testaccount");
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
+  syncer::SyncPrefs sync_prefs(&pref_service_);
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+
+  // First time creation of the service migrates the profile and initializes the
+  // consent bump pref.
+  CreateConsentService(true /* client_services_on_by_default */);
+  EXPECT_TRUE(AreAllNonPersonalizedServicesEnabled());
+  EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
+  histogram_tester.ExpectTotalCount("UnifiedConsent.ConsentBump.SuppressReason",
+                                    0);
+
+  // Simulate shutdown.
+  consent_service_->Shutdown();
+  consent_service_.reset();
+
+  // After the second startup, the user should still be eligible.
+  CreateConsentService(true /* client_services_on_by_default */);
+  EXPECT_TRUE(AreAllNonPersonalizedServicesEnabled());
+  EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
+  histogram_tester.ExpectTotalCount("UnifiedConsent.ConsentBump.SuppressReason",
+                                    0);
+}
+
+TEST_F(UnifiedConsentServiceTest,
+       ConsentBump_NotEligibleOnSecondStartup_DisabledSyncDatatype) {
+  base::HistogramTester histogram_tester;
+
+  identity_test_environment_.SetPrimaryAccount("testaccount");
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
+  syncer::SyncPrefs sync_prefs(&pref_service_);
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+
+  // First time creation of the service migrates the profile and initializes the
+  // consent bump pref.
+  CreateConsentService(true /* client_services_on_by_default */);
+  EXPECT_TRUE(AreAllNonPersonalizedServicesEnabled());
+  EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
+  histogram_tester.ExpectTotalCount("UnifiedConsent.ConsentBump.SuppressReason",
+                                    0);
+
+  // Simulate shutdown.
+  consent_service_->Shutdown();
+  consent_service_.reset();
+
+  // User disables BOOKMARKS.
+  auto data_types = sync_service_.GetPreferredDataTypes();
+  data_types.RetainAll(syncer::UserSelectableTypes());
+  data_types.Remove(syncer::BOOKMARKS);
+  sync_service_.OnUserChoseDatatypes(false, data_types);
+
+  // After the second startup, the user should not be eligible anymore.
+  CreateConsentService(true /* client_services_on_by_default */);
+  EXPECT_FALSE(consent_service_->ShouldShowConsentBump());
+  histogram_tester.ExpectBucketCount(
+      "UnifiedConsent.ConsentBump.SuppressReason",
+      unified_consent::ConsentBumpSuppressReason::kUserTurnedSyncDatatypeOff,
+      1);
+}
+
+TEST_F(UnifiedConsentServiceTest,
+       ConsentBump_NotEligibleOnSecondStartup_DisabledPrivacySetting) {
+  base::HistogramTester histogram_tester;
+
+  identity_test_environment_.SetPrimaryAccount("testaccount");
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
+  syncer::SyncPrefs sync_prefs(&pref_service_);
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+
+  // First time creation of the service migrates the profile and initializes the
+  // consent bump pref.
+  CreateConsentService(true /* client_services_on_by_default */);
+  EXPECT_TRUE(AreAllNonPersonalizedServicesEnabled());
+  EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
+  histogram_tester.ExpectTotalCount("UnifiedConsent.ConsentBump.SuppressReason",
+                                    0);
+
+  // Simulate shutdown.
+  consent_service_->Shutdown();
+  consent_service_.reset();
+
+  // Privacy settings are disabled. After the second startup, the user should
+  // not be eligible anymore.
+  CreateConsentService(false /* client_services_on_by_default */);
+  EXPECT_FALSE(consent_service_->ShouldShowConsentBump());
+  histogram_tester.ExpectBucketCount(
+      "UnifiedConsent.ConsentBump.SuppressReason",
+      unified_consent::ConsentBumpSuppressReason::kUserTurnedPrivacySettingOff,
+      1);
+}
+
 }  // namespace unified_consent

tools/metrics/histograms/enums.xml

@@ -48119,12 +48119,15 @@ Full version information for the fingerprint enum values:
 
 <enum name="UnifiedConsentBumpSuppressReason">
   <int value="0" label="None (consent bump was shown)"/>
-  <int value="1" label="User wasn't signed in"/>
-  <int value="2" label="User wasn't syncing everything"/>
-  <int value="3" label="On-by-default privacy setting was off"/>
+  <int value="1" label="User wasn't signed in during migration"/>
+  <int value="2" label="User wasn't syncing everything during migration"/>
+  <int value="3"
+      label="On-by-default privacy setting was off during migration"/>
   <int value="4" label="User opted into Unity in settings"/>
   <int value="5" label="User signed out"/>
   <int value="6" label="Sync was paused"/>
+  <int value="7" label="User turned off sync data type"/>
+  <int value="8" label="User turned off on-by-default privacy setting"/>
 </enum>
 
 <enum name="UnifiedConsentSyncAndGoogleServicesSettings">