[unified-consent] Rollback unified consent when feature is disabled

This CL rolls back the unified consent preferences when
the installation is excluded from the experiment. This is
needed in case the unified consent feature has to be disabled
due to some reason and rolled out again later.

During rollback, the following is done:
 - All unified consent prefs are cleared.
 - Sync-everything is turned back on if all datatypes are
 turned on and if the consent bump was supposed to be presented.

Bug: 863937
Change-Id: Icb58fad008069c0b38a1ef952ed19e48f8cd3fe6
Reviewed-on: https://chromium-review.googlesource.com/1165226
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582263}

chrome/browser/unified_consent/unified_consent_service_factory.cc

@@ -46,14 +46,19 @@ void UnifiedConsentServiceFactory::RegisterProfilePrefs(
 KeyedService* UnifiedConsentServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
+  syncer::SyncService* sync_service =
+      ProfileSyncServiceFactory::GetSyncServiceForBrowserContext(profile);
 
-  if (!IsUnifiedConsentEnabled(profile))
+  if (!IsUnifiedConsentEnabled(profile)) {
+    unified_consent::UnifiedConsentService::RollbackIfNeeded(
+        profile->GetPrefs(), sync_service);
     return nullptr;
+  }
 
   return new unified_consent::UnifiedConsentService(
       std::make_unique<ChromeUnifiedConsentServiceClient>(profile->GetPrefs()),
       profile->GetPrefs(), IdentityManagerFactory::GetForProfile(profile),
-      ProfileSyncServiceFactory::GetSyncServiceForBrowserContext(profile));
+      sync_service);
 }
 
 bool UnifiedConsentServiceFactory::ServiceIsNULLWhileTesting() const {

components/unified_consent/unified_consent_service.cc

@@ -5,6 +5,8 @@
 #include "components/unified_consent/unified_consent_service.h"
 
 #include "base/metrics/histogram_macros.h"
+#include "base/scoped_observer.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "components/autofill/core/common/autofill_prefs.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -16,6 +18,61 @@
 #include "components/unified_consent/pref_names.h"
 #include "components/unified_consent/unified_consent_service_client.h"
 
+namespace {
+
+// Used for observing the sync service and finishing the rollback once the sync
+// engine is initialized.
+// Note: This object is suicidal - it will kill itself after it finishes the
+// rollback.
+class RollbackHelper : public syncer::SyncServiceObserver {
+ public:
+  explicit RollbackHelper(syncer::SyncService* sync_service);
+  ~RollbackHelper() override = default;
+
+ private:
+  // syncer::SyncServiceObserver:
+  void OnStateChanged(syncer::SyncService* sync_service) override;
+
+  void DoRollbackIfPossibleAndDie(syncer::SyncService* sync_service);
+
+  ScopedObserver<syncer::SyncService, RollbackHelper> scoped_sync_observer_;
+};
+
+RollbackHelper::RollbackHelper(syncer::SyncService* sync_service)
+    : scoped_sync_observer_(this) {
+  if (sync_service->IsEngineInitialized())
+    DoRollbackIfPossibleAndDie(sync_service);
+  else
+    scoped_sync_observer_.Add(sync_service);
+}
+
+void RollbackHelper::OnStateChanged(syncer::SyncService* sync_service) {
+  if (!sync_service->IsEngineInitialized())
+    return;
+
+  scoped_sync_observer_.RemoveAll();
+  DoRollbackIfPossibleAndDie(sync_service);
+}
+
+void RollbackHelper::DoRollbackIfPossibleAndDie(
+    syncer::SyncService* sync_service) {
+  DCHECK(!scoped_sync_observer_.IsObservingSources());
+
+  if (sync_service->GetPreferredDataTypes().HasAll(
+          syncer::UserSelectableTypes())) {
+    // As part of the migration of a profile to Unified Consent, sync everything
+    // is disabled but sync continues to be enabled for all data types.
+    // Therefore it is desired to restore sync everything when rolling back
+    // unified consent to leave sync in the same state as the one before
+    // migration.
+    sync_service->OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
+  }
+
+  base::SequencedTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
+}
+
+}  // namespace
+
 namespace unified_consent {
 
 UnifiedConsentService::UnifiedConsentService(
@@ -67,6 +124,35 @@ void UnifiedConsentService::RegisterPrefs(
       static_cast<int>(MigrationState::NOT_INITIALIZED));
 }
 
+// static
+void UnifiedConsentService::RollbackIfNeeded(
+    PrefService* user_pref_service,
+    syncer::SyncService* sync_service) {
+  DCHECK(user_pref_service);
+
+  if (user_pref_service->GetInteger(prefs::kUnifiedConsentMigrationState) ==
+      static_cast<int>(MigrationState::NOT_INITIALIZED)) {
+    // If there was no migration yet, nothing has to be rolled back.
+    return;
+  }
+
+  if (user_pref_service->GetInteger(prefs::kUnifiedConsentMigrationState) ==
+          static_cast<int>(
+              MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP) &&
+      sync_service &&
+      sync_service->GetDisableReasons() ==
+          syncer::SyncService::DISABLE_REASON_NONE) {
+    // This will wait until the sync engine is initialized and then enables the
+    // sync-everything pref in case the user is syncing all data types.
+    new RollbackHelper(sync_service);
+  }
+
+  // Clear all unified consent prefs.
+  user_pref_service->ClearPref(prefs::kUrlKeyedAnonymizedDataCollectionEnabled);
+  user_pref_service->ClearPref(prefs::kUnifiedConsentGiven);
+  user_pref_service->ClearPref(prefs::kUnifiedConsentMigrationState);
+}
+
 void UnifiedConsentService::SetUnifiedConsentGiven(bool unified_consent_given) {
   pref_service_->SetBoolean(prefs::kUnifiedConsentGiven, unified_consent_given);
 }

components/unified_consent/unified_consent_service.h

@@ -65,6 +65,11 @@ class UnifiedConsentService : public KeyedService,
   // Register the prefs used by this UnifiedConsentService.
   static void RegisterPrefs(user_prefs::PrefRegistrySyncable* registry);
 
+  // Rolls back changes made during migration. This method does nothing if the
+  // user hasn't migrated to unified consent yet.
+  static void RollbackIfNeeded(PrefService* user_pref_service,
+                               syncer::SyncService* sync_service);
+
   // This updates the consent pref and if |unified_consent_given| is true, all
   // unified consent services are enabled.
   void SetUnifiedConsentGiven(bool unified_consent_given);

components/unified_consent/unified_consent_service_unittest.cc

@@ -23,6 +23,9 @@ namespace {
 
 class TestSyncService : public syncer::FakeSyncService {
  public:
+  explicit TestSyncService(PrefService* pref_service)
+      : pref_service_(pref_service) {}
+
   int GetDisableReasons() const override { return DISABLE_REASON_NONE; }
   TransportState GetTransportState() const override { return state_; }
   bool IsFirstSetupComplete() const override { return true; }
@@ -31,7 +34,11 @@ class TestSyncService : public syncer::FakeSyncService {
   }
   void OnUserChoseDatatypes(bool sync_everything,
                             syncer::ModelTypeSet chosen_types) override {
-    is_syncing_everything_ = sync_everything;
+    syncer::SyncPrefs(pref_service_).SetKeepEverythingSynced(sync_everything);
+    chosen_types_ = chosen_types;
+  }
+  syncer::ModelTypeSet GetPreferredDataTypes() const override {
+    return chosen_types_;
   }
 
   void SetTransportState(TransportState state) { state_ = state; }
@@ -39,15 +46,12 @@ class TestSyncService : public syncer::FakeSyncService {
     if (observer_)
       observer_->OnStateChanged(this);
   }
-  // This is a helper if the value is set through |OnUserChoseDatatypes|, which
-  // is not implemented in |FakeSyncService|. Usually
-  // |sync_prefs_.HasKeepEverythingSynced()| is used.
-  bool IsSyncingEverything() { return is_syncing_everything_; }
 
  private:
   syncer::SyncServiceObserver* observer_ = nullptr;
   TransportState state_ = TransportState::ACTIVE;
-  bool is_syncing_everything_ = false;
+  syncer::ModelTypeSet chosen_types_ = syncer::UserSelectableTypes();
+  PrefService* pref_service_;
 };
 
 const char kSpellCheckDummyEnabled[] = "spell_check_dummy.enabled";
@@ -105,6 +109,8 @@ class FakeUnifiedConsentServiceClient : public UnifiedConsentServiceClient {
 
 class UnifiedConsentServiceTest : public testing::Test {
  public:
+  UnifiedConsentServiceTest() : sync_service_(&pref_service_) {}
+
   // testing::Test:
   void SetUp() override {
     pref_service_.registry()->RegisterBooleanPref(
@@ -115,7 +121,10 @@ class UnifiedConsentServiceTest : public testing::Test {
                                                   false);
   }
 
-  void TearDown() override { consent_service_->Shutdown(); }
+  void TearDown() override {
+    if (consent_service_)
+      consent_service_->Shutdown();
+  }
 
   void CreateConsentService(bool client_services_on_by_default = false) {
     auto client =
@@ -207,9 +216,8 @@ TEST_F(UnifiedConsentServiceTest, EnableUnfiedConsent_SyncNotActive) {
   CreateConsentService();
   identity_test_environment_.SetPrimaryAccount("testaccount");
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_FALSE(sync_service_.IsSyncingEverything());
+  sync_service_.OnUserChoseDatatypes(false, syncer::UserSelectableTypes());
   syncer::SyncPrefs sync_prefs(&pref_service_);
-  sync_prefs.SetKeepEverythingSynced(false);
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_FALSE(consent_service_->IsUnifiedConsentGiven());
 
@@ -225,14 +233,14 @@ TEST_F(UnifiedConsentServiceTest, EnableUnfiedConsent_SyncNotActive) {
   EXPECT_TRUE(consent_service_->IsUnifiedConsentGiven());
 
   // Couldn't sync everything because sync is not active.
-  EXPECT_FALSE(sync_service_.IsSyncingEverything());
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
 
   // Initalize sync engine and therefore activate sync.
   sync_service_.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
   sync_service_.FireStateChanged();
 
   // UnifiedConsentService starts syncing everything.
-  EXPECT_TRUE(sync_service_.IsSyncingEverything());
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
 }
 
 // Test whether unified consent is disabled when any of its dependent services
@@ -292,11 +300,9 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndAllServicesOn) {
 
   // Create inconsistent state.
   identity_test_environment_.SetPrimaryAccount("testaccount");
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
   syncer::SyncPrefs sync_prefs(&pref_service_);
-  sync_prefs.SetKeepEverythingSynced(true);
   EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
-  sync_service_.OnUserChoseDatatypes(true, {});
-  EXPECT_TRUE(sync_service_.IsSyncingEverything());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
 
   CreateConsentService(true /* client services on by default */);
@@ -305,7 +311,7 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndAllServicesOn) {
   // the migration state should be in-progress (i.e. the consent bump should be
   // shown).
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_FALSE(sync_service_.IsSyncingEverything());
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_EQ(
       consent_service_->GetMigrationState(),
       unified_consent::MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP);
@@ -330,11 +336,9 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndServicesOff) {
 
   // Create inconsistent state.
   identity_test_environment_.SetPrimaryAccount("testaccount");
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
   syncer::SyncPrefs sync_prefs(&pref_service_);
-  sync_prefs.SetKeepEverythingSynced(true);
   EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
-  sync_service_.OnUserChoseDatatypes(true, {});
-  EXPECT_TRUE(sync_service_.IsSyncingEverything());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
 
   CreateConsentService();
@@ -343,7 +347,7 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndServicesOff) {
   // the migration state should be completed because not all on-by-default
   // privacy settings were on.
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_FALSE(sync_service_.IsSyncingEverything());
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_EQ(consent_service_->GetMigrationState(),
             unified_consent::MigrationState::COMPLETED);
 
@@ -359,8 +363,8 @@ TEST_F(UnifiedConsentServiceTest, Migration_NotSyncingEverything) {
   base::HistogramTester histogram_tester;
 
   identity_test_environment_.SetPrimaryAccount("testaccount");
+  sync_service_.OnUserChoseDatatypes(false, syncer::UserSelectableTypes());
   syncer::SyncPrefs sync_prefs(&pref_service_);
-  sync_prefs.SetKeepEverythingSynced(false);
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
 
@@ -427,4 +431,71 @@ TEST_F(UnifiedConsentServiceTest, Migration_NotSignedIn) {
 }
 #endif  // !defined(OS_CHROMEOS)
 
+TEST_F(UnifiedConsentServiceTest, Rollback_WasSyncingEverything) {
+  identity_test_environment_.SetPrimaryAccount("testaccount");
+  syncer::SyncPrefs sync_prefs(&pref_service_);
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+
+  // Migrate
+  CreateConsentService(true /* client services on by default */);
+  // Check expectations after migration.
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
+  EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
+  EXPECT_EQ(
+      unified_consent::MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP,
+      consent_service_->GetMigrationState());
+
+  consent_service_->Shutdown();
+  consent_service_.reset();
+
+  // Rollback
+  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_);
+  // Unified consent prefs should be cleared.
+  EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
+  EXPECT_EQ(static_cast<int>(unified_consent::MigrationState::NOT_INITIALIZED),
+            pref_service_.GetInteger(
+                unified_consent::prefs::kUnifiedConsentMigrationState));
+  // Sync everything should be back on.
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+
+  // Run until idle so the RollbackHelper is deleted.
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(UnifiedConsentServiceTest, Rollback_WasNotSyncingEverything) {
+  identity_test_environment_.SetPrimaryAccount("testaccount");
+  syncer::SyncPrefs sync_prefs(&pref_service_);
+  syncer::ModelTypeSet chosen_data_types = syncer::UserSelectableTypes();
+  chosen_data_types.Remove(syncer::BOOKMARKS);
+  sync_service_.OnUserChoseDatatypes(false, chosen_data_types);
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
+  EXPECT_FALSE(sync_service_.GetPreferredDataTypes().HasAll(
+      syncer::UserSelectableTypes()));
+
+  // Migrate
+  CreateConsentService();
+  // Check expectations after migration.
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
+  EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
+  EXPECT_EQ(unified_consent::MigrationState::COMPLETED,
+            consent_service_->GetMigrationState());
+
+  consent_service_->Shutdown();
+  consent_service_.reset();
+
+  // Rollback
+  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_);
+  // Unified consent prefs should be cleared.
+  EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
+  EXPECT_EQ(static_cast<int>(unified_consent::MigrationState::NOT_INITIALIZED),
+            pref_service_.GetInteger(
+                unified_consent::prefs::kUnifiedConsentMigrationState));
+  // Sync everything should be off because not all user types were on.
+  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
+
+  // Run until idle so the RollbackHelper is deleted.
+  base::RunLoop().RunUntilIdle();
+}
+
 }  // namespace unified_consent