Revert "[unified-consent] Refactor migration code and add settings updates"

This reverts commit 28ac1ad7.

Reason for revert: Looks like this broke components_unittests on iOS:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/ios-simulator/36547

Original change's description:
> [unified-consent] Refactor migration code and add settings updates
> 
> The migration code in the UnifiedConsentService is refactored:
>  - The migration state IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP is
>  extracted to it's own pref kShouldShowUnifiedConsentBump.
>  - The migration state kInProgressWaitForSyncInit is introduced
>  to be able to update the settings for migration when sync is
>  initialized.
> 
> Additional changes:
>  - ShouldShowConsentBump only returns true if the consent bump
>  feature is enabled.
>  - The rollback enables sync-everything now also when the user is
>  not syncing USER_EVENTS (which is disabled during the migration).
> 
> Users that previously had the unified consent feature enabled and
> for which ShouldShowConsentBump=true, will not be shown the consent
> bump anymore after this CL is landed. This will only affect a few
> users on Canary and Dev.
> 
> Bug: 863932
> Change-Id: I211805f9059cd26056ed81d01ff19470b8caed3c
> Reviewed-on: https://chromium-review.googlesource.com/1172690
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Thomas Tangl <tangltom@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584341}

TBR=ellyjones@chromium.org,droger@chromium.org,tangltom@chromium.org

Change-Id: Ic50e5f7e4ac7d04be1c170b3811fd594980c71c3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 863932
Reviewed-on: https://chromium-review.googlesource.com/1180682Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584350}

chrome/browser/ui/webui/signin/login_ui_service.cc

@@ -116,7 +116,8 @@ class ConsentBumpActivator : public BrowserListObserver,
     unified_consent::UnifiedConsentService* consent_service =
         UnifiedConsentServiceFactory::GetForProfile(profile_);
 
-    consent_service->MarkConsentBumpShown();
+    consent_service->MarkMigrationComplete(
+        unified_consent::ConsentBumpSuppressReason::kNone);
 
     switch (result) {
       case LoginUIService::CONFIGURE_SYNC_FIRST:

components/unified_consent/pref_names.cc

@@ -7,11 +7,6 @@
 namespace unified_consent {
 namespace prefs {
 
-// Boolean indicating whether all criteria is met for the consent bump to be
-// shown.
-const char kShouldShowUnifiedConsentBump[] =
-    "unified_consent.consent_bump.should_show";
-
 // Boolean that is true when the user opted into unified consent.
 const char kUnifiedConsentGiven[] = "unified_consent_given";
 

components/unified_consent/pref_names.h

@@ -8,7 +8,6 @@
 namespace unified_consent {
 namespace prefs {
 
-extern const char kShouldShowUnifiedConsentBump[];
 extern const char kUnifiedConsentGiven[];
 extern const char kUnifiedConsentMigrationState[];
 extern const char kUrlKeyedAnonymizedDataCollectionEnabled[];

components/unified_consent/unified_consent_service.cc

@@ -58,17 +58,13 @@ void RollbackHelper::DoRollbackIfPossibleAndDie(
     syncer::SyncService* sync_service) {
   DCHECK(!scoped_sync_observer_.IsObservingSources());
 
-  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)) {
+          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 except
-    // USER_EVENTS. 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.
+    // 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());
   }
 
@@ -93,7 +89,7 @@ UnifiedConsentService::UnifiedConsentService(
   DCHECK(identity_manager_);
   DCHECK(sync_service_);
 
-  if (GetMigrationState() == MigrationState::kNotInitialized)
+  if (GetMigrationState() == MigrationState::NOT_INITIALIZED)
     MigrateProfileToUnifiedConsent();
 
   service_client_->AddObserver(this);
@@ -125,8 +121,7 @@ void UnifiedConsentService::RegisterPrefs(
   registry->RegisterBooleanPref(prefs::kUnifiedConsentGiven, false);
   registry->RegisterIntegerPref(
       prefs::kUnifiedConsentMigrationState,
-      static_cast<int>(MigrationState::kNotInitialized));
-  registry->RegisterBooleanPref(prefs::kShouldShowUnifiedConsentBump, false);
+      static_cast<int>(MigrationState::NOT_INITIALIZED));
 }
 
 // static
@@ -136,12 +131,14 @@ void UnifiedConsentService::RollbackIfNeeded(
   DCHECK(user_pref_service);
 
   if (user_pref_service->GetInteger(prefs::kUnifiedConsentMigrationState) ==
-      static_cast<int>(MigrationState::kNotInitialized)) {
+      static_cast<int>(MigrationState::NOT_INITIALIZED)) {
     // If there was no migration yet, nothing has to be rolled back.
     return;
   }
 
-  if (user_pref_service->GetBoolean(prefs::kShouldShowUnifiedConsentBump) &&
+  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) {
@@ -154,7 +151,6 @@ void UnifiedConsentService::RollbackIfNeeded(
   user_pref_service->ClearPref(prefs::kUrlKeyedAnonymizedDataCollectionEnabled);
   user_pref_service->ClearPref(prefs::kUnifiedConsentGiven);
   user_pref_service->ClearPref(prefs::kUnifiedConsentMigrationState);
-  user_pref_service->ClearPref(prefs::kShouldShowUnifiedConsentBump);
 }
 
 void UnifiedConsentService::SetUnifiedConsentGiven(bool unified_consent_given) {
@@ -165,41 +161,36 @@ bool UnifiedConsentService::IsUnifiedConsentGiven() {
   return pref_service_->GetBoolean(prefs::kUnifiedConsentGiven);
 }
 
+MigrationState UnifiedConsentService::GetMigrationState() {
+  int migration_state_int =
+      pref_service_->GetInteger(prefs::kUnifiedConsentMigrationState);
+  DCHECK_LE(static_cast<int>(MigrationState::NOT_INITIALIZED),
+            migration_state_int);
+  DCHECK_GE(static_cast<int>(MigrationState::COMPLETED), migration_state_int);
+  return static_cast<MigrationState>(migration_state_int);
+}
+
 bool UnifiedConsentService::ShouldShowConsentBump() {
   if (base::FeatureList::IsEnabled(unified_consent::kForceUnifiedConsentBump))
     return true;
-  if (internal::GetUnifiedConsentFeatureState() !=
-      UnifiedConsentFeatureState::kEnabledWithBump) {
-    return false;
-  }
-  return pref_service_->GetBoolean(prefs::kShouldShowUnifiedConsentBump);
+  return GetMigrationState() ==
+         MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP;
 }
 
-void UnifiedConsentService::MarkConsentBumpShown() {
-  // Record suppress reason kNone, which means that it was shown. This also sets
-  // the |kShouldShowConsentBump| pref to false.
-  RecordConsentBumpSuppressReason(ConsentBumpSuppressReason::kNone);
+void UnifiedConsentService::MarkMigrationComplete(
+    ConsentBumpSuppressReason suppress_reason) {
+  pref_service_->SetInteger(prefs::kUnifiedConsentMigrationState,
+                            static_cast<int>(MigrationState::COMPLETED));
+  // Record the suppress reason for the consent bump. After the migration is
+  // marked complete, the consent bump should not be shown anymore. Note:
+  // |suppress_reason| can be kNone in case the consent bump was actually shown.
+  RecordConsentBumpSuppressReason(suppress_reason);
 }
 
 void UnifiedConsentService::RecordConsentBumpSuppressReason(
     ConsentBumpSuppressReason suppress_reason) {
   UMA_HISTOGRAM_ENUMERATION("UnifiedConsent.ConsentBump.SuppressReason",
                             suppress_reason);
-
-  switch (suppress_reason) {
-    case ConsentBumpSuppressReason::kNone:
-    case ConsentBumpSuppressReason::kNotSignedIn:
-    case ConsentBumpSuppressReason::kSyncEverythingOff:
-    case ConsentBumpSuppressReason::kPrivacySettingOff:
-    case ConsentBumpSuppressReason::kSettingsOptIn:
-    case ConsentBumpSuppressReason::kUserSignedOut:
-      pref_service_->SetBoolean(prefs::kShouldShowUnifiedConsentBump, false);
-      break;
-    case ConsentBumpSuppressReason::kSyncPaused:
-      // Consent bump should be shown when sync is active again.
-      DCHECK(ShouldShowConsentBump());
-      break;
-  }
 }
 
 void UnifiedConsentService::Shutdown() {
@@ -228,24 +219,24 @@ void UnifiedConsentService::OnPrimaryAccountCleared(
                                      false);
   service_client_->SetServiceEnabled(Service::kSpellCheck, false);
 
-  if (GetMigrationState() != MigrationState::kCompleted) {
-    // When the user signs out, the migration is complete.
-    SetMigrationState(MigrationState::kCompleted);
+  switch (GetMigrationState()) {
+    case MigrationState::NOT_INITIALIZED:
+      NOTREACHED();
+      break;
+    case MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP:
+      // Only users that were signed in and have opted into sync before unified
+      // consent are eligible to see the unified consent bump. Since the user
+      // signs out of Chrome, mark the migration to unified consent as complete.
+      MarkMigrationComplete(ConsentBumpSuppressReason::kUserSignedOut);
+      break;
+    case MigrationState::COMPLETED:
+      break;
   }
-
-  if (ShouldShowConsentBump())
-    RecordConsentBumpSuppressReason(ConsentBumpSuppressReason::kUserSignedOut);
 }
 
 void UnifiedConsentService::OnStateChanged(syncer::SyncService* sync) {
-  if (sync_service_->GetDisableReasons() !=
-          syncer::SyncService::DISABLE_REASON_NONE ||
-      !sync_service_->IsEngineInitialized()) {
+  if (!sync_service_->IsEngineInitialized())
     return;
-  }
-
-  if (GetMigrationState() == MigrationState::kInProgressWaitForSyncInit)
-    UpdateSettingsForMigration();
 
   if (sync_service_->IsUsingSecondaryPassphrase() && IsUnifiedConsentGiven()) {
     // Force off unified consent given when the user sets a custom passphrase.
@@ -276,15 +267,12 @@ void UnifiedConsentService::OnUnifiedConsentGivenPrefChanged() {
   DCHECK(!sync_service_->HasDisableReason(
       syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY));
   DCHECK(identity_manager_->HasPrimaryAccount());
-  DCHECK_LT(MigrationState::kNotInitialized, GetMigrationState());
+  DCHECK_LT(MigrationState::NOT_INITIALIZED, GetMigrationState());
 
-  if (GetMigrationState() != MigrationState::kCompleted) {
-    // If the user opted into unified consent, the migration is completed.
-    SetMigrationState(MigrationState::kCompleted);
-  }
-
-  if (ShouldShowConsentBump())
-    RecordConsentBumpSuppressReason(ConsentBumpSuppressReason::kSettingsOptIn);
+  // If the user opts into unified consent throught settings, the consent bump
+  // doesn't need to be shown. Therefore mark the migration as complete.
+  if (GetMigrationState() != MigrationState::COMPLETED)
+    MarkMigrationComplete(ConsentBumpSuppressReason::kSettingsOptIn);
 
   // Enable all sync data types if possible, otherwise they will be enabled with
   // |OnStateChanged| once sync is active;
@@ -308,7 +296,7 @@ void UnifiedConsentService::SetSyncEverythingIfPossible(bool sync_everything) {
   if (sync_everything == sync_prefs.HasKeepEverythingSynced())
     return;
 
-  if (!IsSyncConfigurable())
+  if (!sync_service_->IsEngineInitialized())
     return;
 
   if (sync_everything) {
@@ -322,77 +310,39 @@ void UnifiedConsentService::SetSyncEverythingIfPossible(bool sync_everything) {
   }
 }
 
-MigrationState UnifiedConsentService::GetMigrationState() {
-  int migration_state_int =
-      pref_service_->GetInteger(prefs::kUnifiedConsentMigrationState);
-  DCHECK_LE(static_cast<int>(MigrationState::kNotInitialized),
-            migration_state_int);
-  DCHECK_GE(static_cast<int>(MigrationState::kCompleted), migration_state_int);
-  return static_cast<MigrationState>(migration_state_int);
-}
-
-void UnifiedConsentService::SetMigrationState(MigrationState migration_state) {
-  pref_service_->SetInteger(prefs::kUnifiedConsentMigrationState,
-                            static_cast<int>(migration_state));
-}
-
 void UnifiedConsentService::MigrateProfileToUnifiedConsent() {
-  DCHECK_EQ(GetMigrationState(), MigrationState::kNotInitialized);
+  DCHECK_EQ(GetMigrationState(), MigrationState::NOT_INITIALIZED);
   DCHECK(!IsUnifiedConsentGiven());
 
   if (!identity_manager_->HasPrimaryAccount()) {
-    RecordConsentBumpSuppressReason(ConsentBumpSuppressReason::kNotSignedIn);
-    SetMigrationState(MigrationState::kCompleted);
+    MarkMigrationComplete(ConsentBumpSuppressReason::kNotSignedIn);
     return;
   }
 
-  if (!syncer::SyncPrefs(pref_service_).HasKeepEverythingSynced()) {
-    RecordConsentBumpSuppressReason(
-        ConsentBumpSuppressReason::kSyncEverythingOff);
-  } else if (!AreAllOnByDefaultPrivacySettingsOn()) {
-    RecordConsentBumpSuppressReason(
-        ConsentBumpSuppressReason::kPrivacySettingOff);
-  } else {
-    // When the user was syncing everything, and all on-by-default privacy
-    // settings were on, the consent bump should be shown.
-    pref_service_->SetBoolean(prefs::kShouldShowUnifiedConsentBump, true);
-  }
-
-  UpdateSettingsForMigration();
-}
-
-void UnifiedConsentService::UpdateSettingsForMigration() {
-  if (!IsSyncConfigurable()) {
-    SetMigrationState(MigrationState::kInProgressWaitForSyncInit);
+  bool is_syncing_everything =
+      syncer::SyncPrefs(pref_service_).HasKeepEverythingSynced();
+  if (!is_syncing_everything) {
+    MarkMigrationComplete(ConsentBumpSuppressReason::kSyncEverythingOff);
     return;
   }
 
-  if (IsUnifiedConsentGiven()) {
-    // When the user opted into unified consent through the consent bump or the
-    // settings page while waiting for sync initialization, the migration is
-    // completed.
-    SetMigrationState(MigrationState::kCompleted);
+  // Set sync-everything to false to match the |kUnifiedConsentGiven| pref.
+  // Note: If the sync engine isn't initialized at this point,
+  // sync-everything is set to false once the sync engine state changes.
+  // Sync-everything can then be set to true again after going through the
+  // consent bump and opting into unified consent.
+  SetSyncEverythingIfPossible(false);
+
+  if (!AreAllOnByDefaultPrivacySettingsOn()) {
+    MarkMigrationComplete(ConsentBumpSuppressReason::kPrivacySettingOff);
     return;
   }
 
-  // Set URL-keyed anonymized metrics to the state it had before unified
-  // consent.
-  bool url_keyed_metrics_enabled = sync_service_->GetPreferredDataTypes().Has(
-                                       syncer::HISTORY_DELETE_DIRECTIVES) &&
-                                   !sync_service_->IsUsingSecondaryPassphrase();
-  pref_service_->SetBoolean(prefs::kUrlKeyedAnonymizedDataCollectionEnabled,
-                            url_keyed_metrics_enabled);
-
-  // Disable the datatype user events for newly migrated users. Also set
-  // sync-everything to false, so it matches unified consent given.
-  syncer::ModelTypeSet preferred_types_without_user_events =
-      sync_service_->GetPreferredDataTypes();
-  preferred_types_without_user_events.RetainAll(syncer::UserSelectableTypes());
-  preferred_types_without_user_events.Remove(syncer::USER_EVENTS);
-  sync_service_->OnUserChoseDatatypes(false /*sync everything */,
-                                      preferred_types_without_user_events);
-
-  SetMigrationState(MigrationState::kCompleted);
+  // When the user was syncing everything, and all on-by-default privacy
+  // settings were on, the consent bump should be shown when this is possible.
+  pref_service_->SetInteger(
+      prefs::kUnifiedConsentMigrationState,
+      static_cast<int>(MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP));
 }
 
 bool UnifiedConsentService::AreAllNonPersonalizedServicesEnabled() {
@@ -418,9 +368,4 @@ bool UnifiedConsentService::AreAllOnByDefaultPrivacySettingsOn() {
   return true;
 }
 
-bool UnifiedConsentService::IsSyncConfigurable() {
-  return sync_service_->GetTransportState() ==
-         syncer::SyncService::TransportState::ACTIVE;
-}
-
 }  //  namespace unified_consent

components/unified_consent/unified_consent_service.h

@@ -31,10 +31,10 @@ using Service = UnifiedConsentServiceClient::Service;
 using ServiceState = UnifiedConsentServiceClient::ServiceState;
 
 enum class MigrationState : int {
-  kNotInitialized = 0,
-  kInProgressWaitForSyncInit = 1,
-  // Reserve space for other kInProgress* entries to be added here.
-  kCompleted = 10,
+  NOT_INITIALIZED = 0,
+  IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP = 1,
+  // Reserve space for other IN_PROGRESS_* entries to be added here.
+  COMPLETED = 10,
 };
 
 enum class ConsentBumpSuppressReason {
@@ -75,14 +75,19 @@ class UnifiedConsentService : public KeyedService,
   void SetUnifiedConsentGiven(bool unified_consent_given);
   bool IsUnifiedConsentGiven();
 
-  // Returns true if all criteria is met to show the consent bump.
+  // Helper function for the consent bump. The consent bump has to
+  // be shown depending on the migration state.
+  MigrationState GetMigrationState();
+  // Returns true if the consent bump needs to be shown to the user as part
+  // of the migration of the Chrome profile to unified consent.
   bool ShouldShowConsentBump();
-  // Marks the consent bump as shown. Any future calls to
-  // |ShouldShowConsentBump| are guaranteed to return false.
-  void MarkConsentBumpShown();
-  // Records the consent bump suppress reason and updates the state whether the
-  // consent bump should be shown. Note: In some cases, e.g. sync paused,
-  // |ShouldShowConsentBump| will still return true.
+  // Finishes the migration to unified consent. All future calls to
+  // |GetMigrationState| are guranteed to return |MIGRATION_COMPLETED|.
+  // Takes as argument the suppress reason for not showing the consent
+  // bump if it wasn't shown.
+  void MarkMigrationComplete(ConsentBumpSuppressReason suppress_reason);
+  // Records the suppress reason for the consent bump without changing the
+  // migration state.
   void RecordConsentBumpSuppressReason(
       ConsentBumpSuppressReason suppress_reason);
 
@@ -110,17 +115,9 @@ class UnifiedConsentService : public KeyedService,
   // Enables/disables syncing everything if the sync engine is initialized.
   void SetSyncEverythingIfPossible(bool sync_everything);
 
-  // Migration helpers.
-  MigrationState GetMigrationState();
-  void SetMigrationState(MigrationState migration_state);
-  // Called when the unified consent service is created. This sets the
-  // |kShouldShowUnifiedConsentBump| pref to true if the user is eligible and
-  // calls |UpdateSettingsForMigration| at the end.
+  // Called when the unified consent service is created to resolve
+  // inconsistencies with sync-related prefs.
   void MigrateProfileToUnifiedConsent();
-  // Updates the settings preferences for the migration when the sync engine is
-  // initialized. When it is not, this function will be called again from
-  // |OnStateChanged| when the sync engine is initialized.
-  void UpdateSettingsForMigration();
 
   // Returns true if all non-personalized services are enabled.
   bool AreAllNonPersonalizedServicesEnabled();
@@ -128,10 +125,6 @@ class UnifiedConsentService : public KeyedService,
   // Checks if all on-by-default non-personalized services are on.
   bool AreAllOnByDefaultPrivacySettingsOn();
 
-  // Helper that checks whether it's okay to call
-  // |SyncService::OnUserChoseDatatypes|.
-  bool IsSyncConfigurable();
-
   std::unique_ptr<UnifiedConsentServiceClient> service_client_;
   PrefService* pref_service_;
   identity::IdentityManager* identity_manager_;

components/unified_consent/unified_consent_service_unittest.cc

@@ -14,7 +14,6 @@
 #include "components/sync/driver/fake_sync_service.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/unified_consent/pref_names.h"
-#include "components/unified_consent/scoped_unified_consent.h"
 #include "components/unified_consent/unified_consent_service_client.h"
 #include "services/identity/public/cpp/identity_test_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -39,10 +38,7 @@ class TestSyncService : public syncer::FakeSyncService {
     chosen_types_ = chosen_types;
   }
   syncer::ModelTypeSet GetPreferredDataTypes() const override {
-    syncer::ModelTypeSet preferred = chosen_types_;
-    // Add this for the Migration_UpdateSettings test.
-    preferred.Put(syncer::HISTORY_DELETE_DIRECTIVES);
-    return preferred;
+    return chosen_types_;
   }
   bool IsUsingSecondaryPassphrase() const override {
     return is_using_passphrase_;
@@ -139,11 +135,6 @@ class UnifiedConsentServiceTest : public testing::Test {
   }
 
   void CreateConsentService(bool client_services_on_by_default = false) {
-    if (!scoped_unified_consent_) {
-      SetUnifiedConsentFeatureState(
-          unified_consent::UnifiedConsentFeatureState::kEnabledWithBump);
-    }
-
     auto client =
         std::make_unique<FakeUnifiedConsentServiceClient>(&pref_service_);
     if (client_services_on_by_default) {
@@ -161,12 +152,6 @@ class UnifiedConsentServiceTest : public testing::Test {
                           consent_service_->service_client_.get();
   }
 
-  void SetUnifiedConsentFeatureState(
-      unified_consent::UnifiedConsentFeatureState feature_state) {
-    scoped_unified_consent_.reset(
-        new unified_consent::ScopedUnifiedConsent(feature_state));
-  }
-
   bool AreAllNonPersonalizedServicesEnabled() {
     return consent_service_->AreAllNonPersonalizedServicesEnabled();
   }
@@ -175,12 +160,6 @@ class UnifiedConsentServiceTest : public testing::Test {
     return consent_service_->AreAllOnByDefaultPrivacySettingsOn();
   }
 
-  unified_consent::MigrationState GetMigrationState() {
-    int migration_state_int =
-        pref_service_.GetInteger(prefs::kUnifiedConsentMigrationState);
-    return static_cast<unified_consent::MigrationState>(migration_state_int);
-  }
-
  protected:
   base::MessageLoop message_loop_;
   sync_preferences::TestingPrefServiceSyncable pref_service_;
@@ -188,8 +167,6 @@ class UnifiedConsentServiceTest : public testing::Test {
   TestSyncService sync_service_;
   std::unique_ptr<UnifiedConsentService> consent_service_;
   FakeUnifiedConsentServiceClient* service_client_ = nullptr;
-
-  std::unique_ptr<ScopedUnifiedConsent> scoped_unified_consent_;
 };
 
 TEST_F(UnifiedConsentServiceTest, DefaultValuesWhenSignedOut) {
@@ -354,36 +331,26 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndAllServicesOn) {
   syncer::SyncPrefs sync_prefs(&pref_service_);
   EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  sync_service_.SetTransportState(
-      syncer::SyncService::TransportState::PENDING_DESIRED_CONFIGURATION);
-  EXPECT_FALSE(sync_service_.IsSyncActive());
 
   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.
+  // After the creation of the consent service, inconsistencies are resolved and
+  // the migration state should be in-progress (i.e. the consent bump should be
+  // shown).
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_EQ(GetMigrationState(),
-            unified_consent::MigrationState::kInProgressWaitForSyncInit);
-  EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
-  // Sync-everything is still on because sync is not active yet.
-  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
-
-  // When sync is active, the migration should continue and finish.
-  sync_service_.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
-  sync_service_.FireStateChanged();
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
-
+  EXPECT_EQ(
+      consent_service_->GetMigrationState(),
+      unified_consent::MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP);
   // No metric for the consent bump suppress reason should have been recorded at
   // this point.
   histogram_tester.ExpectTotalCount("UnifiedConsent.ConsentBump.SuppressReason",
                                     0);
 
-  // When the user signs out, the migration state changes to completed and the
-  // consent bump doesn't need to be shown anymore.
+  // When the user signs out, the migration state changes to completed.
   identity_test_environment_.ClearPrimaryAccount();
-  EXPECT_EQ(GetMigrationState(), unified_consent::MigrationState::kCompleted);
-  EXPECT_FALSE(consent_service_->ShouldShowConsentBump());
+  EXPECT_EQ(consent_service_->GetMigrationState(),
+            unified_consent::MigrationState::COMPLETED);
   // A metric for the consent bump suppress reason should have been recorded at
   // this point.
   histogram_tester.ExpectBucketCount(
@@ -400,16 +367,16 @@ TEST_F(UnifiedConsentServiceTest, Migration_SyncingEverythingAndServicesOff) {
   syncer::SyncPrefs sync_prefs(&pref_service_);
   EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_TRUE(sync_service_.IsSyncActive());
 
   CreateConsentService();
   EXPECT_FALSE(AreAllOnByDefaultPrivacySettingsOn());
-  // After the creation of the consent service, the profile is migrated and
-  // |ShouldShowConsentBump| should return false.
+  // After the creation of the consent service, inconsistencies are resolved and
+  // 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_prefs.HasKeepEverythingSynced());
-  EXPECT_EQ(GetMigrationState(), unified_consent::MigrationState::kCompleted);
-  EXPECT_FALSE(consent_service_->ShouldShowConsentBump());
+  EXPECT_EQ(consent_service_->GetMigrationState(),
+            unified_consent::MigrationState::COMPLETED);
 
   // A metric for the consent bump suppress reason should have been recorded at
   // this point.
@@ -429,37 +396,16 @@ TEST_F(UnifiedConsentServiceTest, Migration_NotSyncingEverything) {
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
 
   CreateConsentService();
-  // When the user is not syncing everything the migration is completed after
-  // the creation of the consent service.
-  EXPECT_EQ(GetMigrationState(), unified_consent::MigrationState::kCompleted);
+  // Since there were not inconsistencies, the migration is completed after the
+  // creation of the consent service.
+  EXPECT_EQ(consent_service_->GetMigrationState(),
+            unified_consent::MigrationState::COMPLETED);
   // The suppress reason for not showing the consent bump should be recorded.
   histogram_tester.ExpectBucketCount(
       "UnifiedConsent.ConsentBump.SuppressReason",
       unified_consent::ConsentBumpSuppressReason::kSyncEverythingOff, 1);
 }
 
-TEST_F(UnifiedConsentServiceTest, Migration_UpdateSettings) {
-  // Create user that syncs everything
-  identity_test_environment_.SetPrimaryAccount("testaccount");
-  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
-  syncer::SyncPrefs sync_prefs(&pref_service_);
-  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
-  EXPECT_TRUE(sync_service_.IsSyncActive());
-  EXPECT_TRUE(sync_service_.GetPreferredDataTypes().Has(syncer::USER_EVENTS));
-  // Url keyed data collection is off before the migration.
-  EXPECT_FALSE(pref_service_.GetBoolean(
-      prefs::kUrlKeyedAnonymizedDataCollectionEnabled));
-
-  CreateConsentService();
-  EXPECT_EQ(GetMigrationState(), unified_consent::MigrationState::kCompleted);
-  // During the migration USER_EVENTS is disabled and Url keyed data collection
-  // is enabled.
-  EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
-  EXPECT_FALSE(sync_service_.GetPreferredDataTypes().Has(syncer::USER_EVENTS));
-  EXPECT_TRUE(pref_service_.GetBoolean(
-      prefs::kUrlKeyedAnonymizedDataCollectionEnabled));
-}
-
 #if !defined(OS_CHROMEOS)
 TEST_F(UnifiedConsentServiceTest, ClearPrimaryAccountDisablesSomeServices) {
   CreateConsentService();
@@ -503,7 +449,8 @@ TEST_F(UnifiedConsentServiceTest, Migration_NotSignedIn) {
   CreateConsentService();
   // Since there were not inconsistencies, the migration is completed after the
   // creation of the consent service.
-  EXPECT_EQ(GetMigrationState(), unified_consent::MigrationState::kCompleted);
+  EXPECT_EQ(consent_service_->GetMigrationState(),
+            unified_consent::MigrationState::COMPLETED);
   // The suppress reason for not showing the consent bump should be recorded.
   histogram_tester.ExpectBucketCount(
       "UnifiedConsent.ConsentBump.SuppressReason",
@@ -522,19 +469,20 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasSyncingEverything) {
   // Check expectations after migration.
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_EQ(unified_consent::MigrationState::kCompleted, GetMigrationState());
-  EXPECT_TRUE(consent_service_->ShouldShowConsentBump());
+  EXPECT_EQ(
+      unified_consent::MigrationState::IN_PROGRESS_SHOULD_SHOW_CONSENT_BUMP,
+      consent_service_->GetMigrationState());
 
   consent_service_->Shutdown();
   consent_service_.reset();
-  SetUnifiedConsentFeatureState(UnifiedConsentFeatureState::kDisabled);
 
   // Rollback
   UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_);
   // Unified consent prefs should be cleared.
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_EQ(unified_consent::MigrationState::kNotInitialized,
-            GetMigrationState());
+  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());
 
@@ -557,7 +505,8 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasNotSyncingEverything) {
   // Check expectations after migration.
   EXPECT_FALSE(sync_prefs.HasKeepEverythingSynced());
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_EQ(unified_consent::MigrationState::kCompleted, GetMigrationState());
+  EXPECT_EQ(unified_consent::MigrationState::COMPLETED,
+            consent_service_->GetMigrationState());
 
   consent_service_->Shutdown();
   consent_service_.reset();
@@ -566,9 +515,9 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasNotSyncingEverything) {
   UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_);
   // Unified consent prefs should be cleared.
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
-  EXPECT_EQ(unified_consent::MigrationState::kNotInitialized,
-            GetMigrationState());
-
+  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());