[unified-consent] Disable off-by-default services during rollback

When the user opted into unified consent and then the feature is
rolled back, all off-by-default services are disabled.

Bug: 894504
Change-Id: I50adddb41cab7c38162344a538eef52e04dfa999
Reviewed-on: https://chromium-review.googlesource.com/c/1280445
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600746}

chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/unified_consent/unified_consent_service_factory.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/metrics/metrics_pref_names.h"
@@ -235,7 +236,8 @@ class SafeBrowsingBlockingPageTest : public ChromeRenderViewHostTestHarness {
     CANCEL
   };
 
-  SafeBrowsingBlockingPageTest() {
+  SafeBrowsingBlockingPageTest()
+      : scoped_testing_local_state_(TestingBrowserProcess::GetGlobal()) {
     ResetUserResponse();
     // The safe browsing UI manager does not need a service for this test.
     ui_manager_ = new TestSafeBrowsingUIManager(NULL);
@@ -390,6 +392,7 @@ class SafeBrowsingBlockingPageTest : public ChromeRenderViewHostTestHarness {
     resource->threat_source = safe_browsing::ThreatSource::LOCAL_PVER3;
   }
 
+  ScopedTestingLocalState scoped_testing_local_state_;
   UserResponse user_response_;
   TestSafeBrowsingBlockingPageFactory factory_;
 };
@@ -1028,7 +1031,8 @@ class SafeBrowsingBlockingQuietPageTest
   // The decision the user made.
   enum UserResponse { PENDING, OK, CANCEL };
 
-  SafeBrowsingBlockingQuietPageTest() {
+  SafeBrowsingBlockingQuietPageTest()
+      : scoped_testing_local_state_(TestingBrowserProcess::GetGlobal()) {
     // The safe browsing UI manager does not need a service for this test.
     ui_manager_ = new TestSafeBrowsingUIManager(NULL);
   }
@@ -1125,6 +1129,7 @@ class SafeBrowsingBlockingQuietPageTest
     resource->threat_source = safe_browsing::ThreatSource::LOCAL_PVER3;
   }
 
+  ScopedTestingLocalState scoped_testing_local_state_;
   UserResponse user_response_;
   TestSafeBrowsingBlockingQuietPageFactory factory_;
   scoped_refptr<net::URLRequestContextGetter> system_request_context_getter_;
@@ -1218,7 +1223,6 @@ TEST_F(TEST_CLASS_ExtendedReportingNotShownUnifiedConsent,
   // Fake sign in so unified consent can be given.
   SigninManagerFactory::GetForProfile(profile)->SetAuthenticatedAccountInfo(
       "gaia_id", "user");
-  TestingBrowserProcess::GetGlobal()->SetLocalState(profile->GetPrefs());
 
   // Give unified consent.
   UnifiedConsentServiceFactory::GetForProfile(profile)->SetUnifiedConsentGiven(
@@ -1244,7 +1248,6 @@ TEST_F(TEST_CLASS_ExtendedReportingNotShownUnifiedConsent,
   // The interstitial should be gone.
   EXPECT_EQ(CANCEL, user_response());
   EXPECT_FALSE(GetSafeBrowsingBlockingPage());
-  TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr);
 }
 
 }  // namespace safe_browsing

chrome/browser/unified_consent/unified_consent_service_factory.cc

@@ -52,8 +52,9 @@ KeyedService* UnifiedConsentServiceFactory::BuildServiceInstanceFor(
     return nullptr;
 
   if (!unified_consent::IsUnifiedConsentFeatureEnabled()) {
+    ChromeUnifiedConsentServiceClient service_client(profile->GetPrefs());
     unified_consent::UnifiedConsentService::RollbackIfNeeded(
-        profile->GetPrefs(), sync_service);
+        profile->GetPrefs(), sync_service, &service_client);
     return nullptr;
   }
 

components/unified_consent/pref_names.cc

@@ -7,6 +7,12 @@
 namespace unified_consent {
 namespace prefs {
 
+// Boolean indicating whether all unified consent services were ever enabled
+// because the user opted into unified consent. This pref is used during
+// rollback to disable off-by-default services.
+const char kAllUnifiedConsentServicesWereEnabled[] =
+    "unified_consent.all_services_were_enabled";
+
 // Boolean indicating whether the user had everything synced before migrating to
 // unified consent.
 const char kHadEverythingSyncedBeforeMigration[] =

components/unified_consent/pref_names.h

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

components/unified_consent/unified_consent_service.cc

@@ -139,6 +139,12 @@ UnifiedConsentService::UnifiedConsentService(
         metrics::UnifiedConsentRevokeReason::kServiceWasDisabled);
   }
 
+  // Update pref for existing users.
+  // TODO(tangltom): Delete this when all users are migrated.
+  if (pref_service_->GetBoolean(prefs::kUnifiedConsentGiven))
+    pref_service_->SetBoolean(prefs::kAllUnifiedConsentServicesWereEnabled,
+                              true);
+
   RecordSettingsHistogram();
 }
 
@@ -156,13 +162,17 @@ void UnifiedConsentService::RegisterPrefs(
   registry->RegisterBooleanPref(prefs::kShouldShowUnifiedConsentBump, false);
   registry->RegisterBooleanPref(prefs::kHadEverythingSyncedBeforeMigration,
                                 false);
+  registry->RegisterBooleanPref(prefs::kAllUnifiedConsentServicesWereEnabled,
+                                false);
 }
 
 // static
 void UnifiedConsentService::RollbackIfNeeded(
     PrefService* user_pref_service,
-    syncer::SyncService* sync_service) {
+    syncer::SyncService* sync_service,
+    UnifiedConsentServiceClient* service_client) {
   DCHECK(user_pref_service);
+  DCHECK(service_client);
 
   if (user_pref_service->GetInteger(prefs::kUnifiedConsentMigrationState) ==
       static_cast<int>(MigrationState::kNotInitialized)) {
@@ -182,12 +192,24 @@ void UnifiedConsentService::RollbackIfNeeded(
     new RollbackHelper(sync_service);
   }
 
+  // Turn off all off-by-default services if services were enabled due to
+  // unified consent.
+  if (user_pref_service->GetBoolean(
+          prefs::kAllUnifiedConsentServicesWereEnabled)) {
+    service_client->SetServiceEnabled(Service::kSafeBrowsingExtendedReporting,
+                                      false);
+    service_client->SetServiceEnabled(Service::kSpellCheck, false);
+    contextual_search::ContextualSearchPreference::GetInstance()->SetPref(
+        user_pref_service, false);
+  }
+
   // Clear all unified consent prefs.
   user_pref_service->ClearPref(prefs::kUrlKeyedAnonymizedDataCollectionEnabled);
   user_pref_service->ClearPref(prefs::kUnifiedConsentGiven);
   user_pref_service->ClearPref(prefs::kUnifiedConsentMigrationState);
   user_pref_service->ClearPref(prefs::kShouldShowUnifiedConsentBump);
   user_pref_service->ClearPref(prefs::kHadEverythingSyncedBeforeMigration);
+  user_pref_service->ClearPref(prefs::kAllUnifiedConsentServicesWereEnabled);
 }
 
 void UnifiedConsentService::SetUnifiedConsentGiven(bool unified_consent_given) {
@@ -264,6 +286,8 @@ void UnifiedConsentService::OnPrimaryAccountCleared(
     RecordUnifiedConsentRevoked(
         metrics::UnifiedConsentRevokeReason::kUserSignedOut);
   }
+  pref_service_->SetBoolean(prefs::kAllUnifiedConsentServicesWereEnabled,
+                            false);
 
   // By design, signing out of Chrome automatically disables off-by-default
   // services.
@@ -397,6 +421,8 @@ void UnifiedConsentService::OnUnifiedConsentGivenPrefChanged() {
       service_client_->SetServiceEnabled(service, true);
     }
   }
+
+  pref_service_->SetBoolean(prefs::kAllUnifiedConsentServicesWereEnabled, true);
 }
 
 MigrationState UnifiedConsentService::GetMigrationState() {

components/unified_consent/unified_consent_service.h

@@ -59,7 +59,8 @@ class UnifiedConsentService : public KeyedService,
   // 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);
+                               syncer::SyncService* sync_service,
+                               UnifiedConsentServiceClient* service_client);
 
   // This updates the consent pref and if |unified_consent_given| is true, all
   // unified consent services are enabled.

components/unified_consent/unified_consent_service_unittest.cc

@@ -113,21 +113,27 @@ class FakeUnifiedConsentServiceClient : public UnifiedConsentServiceClient {
     is_not_supported_[service] = true;
   }
 
+  static void ClearServiceStates() {
+    service_enabled_.clear();
+    is_not_supported_.clear();
+  }
+
  private:
-  std::map<Service, bool> service_enabled_;
-  std::map<Service, bool> is_not_supported_;
+  // Service states are shared between multiple instances of this class.
+  static std::map<Service, bool> service_enabled_;
+  static std::map<Service, bool> is_not_supported_;
 
   PrefService* pref_service_;
 };
 
+std::map<Service, bool> FakeUnifiedConsentServiceClient::service_enabled_;
+std::map<Service, bool> FakeUnifiedConsentServiceClient::is_not_supported_;
+
 }  // namespace
 
 class UnifiedConsentServiceTest : public testing::Test {
  public:
-  UnifiedConsentServiceTest() : sync_service_(&pref_service_) {}
-
-  // testing::Test:
-  void SetUp() override {
+  UnifiedConsentServiceTest() : sync_service_(&pref_service_) {
     pref_service_.registry()->RegisterBooleanPref(
         autofill::prefs::kAutofillWalletImportEnabled, false);
     UnifiedConsentService::RegisterPrefs(pref_service_.registry());
@@ -138,9 +144,13 @@ class UnifiedConsentServiceTest : public testing::Test {
     pref_service_.registry()->RegisterStringPref(
         contextual_search::GetPrefName(), "");
 #endif  // defined(OS_ANDROID)
+
+    FakeUnifiedConsentServiceClient::ClearServiceStates();
+    service_client_ =
+        std::make_unique<FakeUnifiedConsentServiceClient>(&pref_service_);
   }
 
-  void TearDown() override {
+  ~UnifiedConsentServiceTest() override {
     if (consent_service_)
       consent_service_->Shutdown();
   }
@@ -164,8 +174,6 @@ class UnifiedConsentServiceTest : public testing::Test {
     consent_service_ = std::make_unique<UnifiedConsentService>(
         std::move(client), &pref_service_,
         identity_test_environment_.identity_manager(), &sync_service_);
-    service_client_ = (FakeUnifiedConsentServiceClient*)
-                          consent_service_->service_client_.get();
 
     sync_service_.FireStateChanged();
     // Run until idle so the migration can finish.
@@ -202,7 +210,7 @@ class UnifiedConsentServiceTest : public testing::Test {
   identity::IdentityTestEnvironment identity_test_environment_;
   TestSyncService sync_service_;
   std::unique_ptr<UnifiedConsentService> consent_service_;
-  FakeUnifiedConsentServiceClient* service_client_ = nullptr;
+  std::unique_ptr<FakeUnifiedConsentServiceClient> service_client_;
 
   std::unique_ptr<ScopedUnifiedConsent> scoped_unified_consent_;
 };
@@ -570,7 +578,8 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasSyncingEverything) {
   SetUnifiedConsentFeatureState(UnifiedConsentFeatureState::kDisabled);
 
   // Rollback
-  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_);
+  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_,
+                                          service_client_.get());
   base::RunLoop().RunUntilIdle();
 
   // Unified consent prefs should be cleared.
@@ -609,7 +618,8 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasNotSyncingEverything) {
   consent_service_.reset();
 
   // Rollback
-  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_);
+  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_,
+                                          service_client_.get());
   // Unified consent prefs should be cleared.
   EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
   EXPECT_EQ(unified_consent::MigrationState::kNotInitialized,
@@ -622,6 +632,50 @@ TEST_F(UnifiedConsentServiceTest, Rollback_WasNotSyncingEverything) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST_F(UnifiedConsentServiceTest, Rollback_UserOptedIntoUnifiedConsent) {
+  identity_test_environment_.SetPrimaryAccount("testaccount");
+  syncer::SyncPrefs sync_prefs(&pref_service_);
+  sync_service_.OnUserChoseDatatypes(true, syncer::UserSelectableTypes());
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+
+  // Migrate and opt into unified consent.
+  CreateConsentService();
+  consent_service_->SetUnifiedConsentGiven(true);
+  // Check expectations after opt-in.
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+  EXPECT_TRUE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
+  EXPECT_EQ(unified_consent::MigrationState::kCompleted, GetMigrationState());
+  EXPECT_TRUE(
+      pref_service_.GetBoolean(prefs::kHadEverythingSyncedBeforeMigration));
+  EXPECT_TRUE(
+      pref_service_.GetBoolean(prefs::kAllUnifiedConsentServicesWereEnabled));
+
+  consent_service_->Shutdown();
+  consent_service_.reset();
+  SetUnifiedConsentFeatureState(UnifiedConsentFeatureState::kDisabled);
+
+  // Rollback
+  UnifiedConsentService::RollbackIfNeeded(&pref_service_, &sync_service_,
+                                          service_client_.get());
+  base::RunLoop().RunUntilIdle();
+
+  // Unified consent prefs should be cleared.
+  EXPECT_FALSE(pref_service_.GetBoolean(prefs::kUnifiedConsentGiven));
+  EXPECT_EQ(unified_consent::MigrationState::kNotInitialized,
+            GetMigrationState());
+  EXPECT_FALSE(
+      pref_service_.GetBoolean(prefs::kHadEverythingSyncedBeforeMigration));
+  // Sync everything should still be on.
+  EXPECT_TRUE(sync_prefs.HasKeepEverythingSynced());
+  // Off-by-default services should be turned off.
+  EXPECT_NE(ServiceState::kEnabled,
+            service_client_->GetServiceState(
+                Service::kSafeBrowsingExtendedReporting));
+  EXPECT_NE(ServiceState::kEnabled,
+            service_client_->GetServiceState(Service::kSpellCheck));
+  EXPECT_FALSE(contextual_search::IsEnabled(pref_service_));
+}
+
 TEST_F(UnifiedConsentServiceTest, SettingsHistogram_None) {
   base::HistogramTester histogram_tester;
   // Disable all services.
@@ -767,13 +821,14 @@ TEST_F(UnifiedConsentServiceTest,
   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.
+  // Disable privacy setting.
+  service_client_->SetServiceEnabled(Service::kSafeBrowsing, false);
+
+  // 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(