Gracefully handle kSafeBrowsingEnhanced in the Extensions API

See the linked bug for the explanation of the approach.

Bug: 1064722
Change-Id: I8e2343f7be297acc212985ad079a79a374d1ef9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2120722Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Auto-Submit: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755041}

chrome/browser/extensions/api/preference/preference_api.cc

@@ -805,6 +805,18 @@ ExtensionFunction::ResponseAction SetPreferenceFunction::Run() {
         base::Value(browser_pref_value->GetBool()));
   }
 
+  // Whenever an extension takes control of the |kSafeBrowsingEnabled|
+  // preference, it must also set |kSafeBrowsingEnhanced| to false.
+  // See crbug.com/1064722 for more background.
+  //
+  // TODO(crbug.com/1064722): Consider extending
+  // chrome.privacy.services.safeBrowsingEnabled to a three-state enum.
+  if (prefs::kSafeBrowsingEnabled == browser_pref) {
+    preference_api->SetExtensionControlledPref(extension_id(),
+                                               prefs::kSafeBrowsingEnhanced,
+                                               scope, base::Value(false));
+  }
+
   preference_api->SetExtensionControlledPref(
       extension_id(), browser_pref, scope,
       base::Value::FromUniquePtrValue(std::move(browser_pref_value)));
@@ -857,6 +869,19 @@ ExtensionFunction::ResponseAction ClearPreferenceFunction::Run() {
 
   PreferenceAPI::Get(browser_context())
       ->RemoveExtensionControlledPref(extension_id(), browser_pref, scope);
+
+  // Whenever an extension clears the |kSafeBrowsingEnabled| preference,
+  // it must also clear |kSafeBrowsingEnhanced|. See crbug.com/1064722 for
+  // more background.
+  //
+  // TODO(crbug.com/1064722): Consider extending
+  // chrome.privacy.services.safeBrowsingEnabled to a three-state enum.
+  if (prefs::kSafeBrowsingEnabled == browser_pref) {
+    PreferenceAPI::Get(browser_context())
+        ->RemoveExtensionControlledPref(extension_id(),
+                                        prefs::kSafeBrowsingEnhanced, scope);
+  }
+
   return RespondNow(NoArguments());
 }
 

chrome/browser/extensions/api/preference/preference_apitest.cc

@@ -89,6 +89,23 @@ class ExtensionPreferenceApiTest : public extensions::ExtensionApiTest {
     EXPECT_FALSE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
   }
 
+  // Verifies whether the boolean |preference| has the |expected_value| and is
+  // |expected_controlled| by an extension.
+  void VerifyPrefValueAndControlledState(const std::string& preference,
+                                         bool expected_value,
+                                         bool expected_controlled) {
+    SCOPED_TRACE(preference);
+
+    PrefService* prefs = profile_->GetPrefs();
+    const PrefService::Preference* pref = prefs->FindPreference(preference);
+    ASSERT_TRUE(pref);
+    const base::Value* actual_value = pref->GetValue();
+    ASSERT_TRUE(actual_value->is_bool());
+
+    EXPECT_EQ(expected_value, actual_value->GetBool());
+    EXPECT_EQ(expected_controlled, pref->IsExtensionControlled());
+  }
+
   void SetUpOnMainThread() override {
     extensions::ExtensionApiTest::SetUpOnMainThread();
 
@@ -408,3 +425,67 @@ IN_PROC_BROWSER_TEST_F(ExtensionPreferenceApiTest, DataReductionProxy) {
   EXPECT_TRUE(RunExtensionTest("preference/data_reduction_proxy")) <<
       message_;
 }
+
+// Tests the behavior of the Safe Browsing API as described in
+// crbug.com/1064722.
+IN_PROC_BROWSER_TEST_F(ExtensionPreferenceApiTest, SafeBrowsing_SetTrue) {
+  ExtensionTestMessageListener listener_true("set to true",
+                                             /* will_reply */ true);
+  ExtensionTestMessageListener listener_clear("cleared", /* will_reply */ true);
+  ExtensionTestMessageListener listener_false("set to false",
+                                              /* will_reply */ true);
+  ExtensionTestMessageListener listener_done("done", /* will_reply */ false);
+
+  const base::FilePath extension_path =
+      test_data_dir_.AppendASCII("preference").AppendASCII("safe_browsing");
+  const extensions::Extension* extension = LoadExtension(extension_path);
+  ASSERT_TRUE(extension);
+
+  // Step 1. of the test sets the API to TRUE.
+  // Both preferences are now controlled by extension. |kSafeBrowsingEnabled| is
+  // set to TRUE, while |kSafeBrowsingEnhanced| is always FALSE.
+  ASSERT_TRUE(listener_true.WaitUntilSatisfied());
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnabled,
+                                    /* expected_value */ true,
+                                    /* expected_controlled */ true);
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnhanced,
+                                    /* expected_value */ false,
+                                    /* expected_controlled */ true);
+  listener_true.Reply("ok");
+
+  // Step 2. of the test clears the value.
+  // Neither preference is now controlled by extension, and they take on their
+  // default values - TRUE and FALSE, respectively.
+  ASSERT_TRUE(listener_clear.WaitUntilSatisfied());
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnabled,
+                                    /* expected_value */ true,
+                                    /* expected_controlled */ false);
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnhanced,
+                                    /* expected_value */ false,
+                                    /* expected_controlled */ false);
+  listener_clear.Reply("ok");
+
+  // Step 3. of the test sets the API to FALSE.
+  // Both preferences are now controlled by extension. |kSafeBrowsingEnabled| is
+  // set to FALSE, and |kSafeBrowsingEnhanced| is also FALSE.
+  ASSERT_TRUE(listener_false.WaitUntilSatisfied());
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnabled,
+                                    /* expected_value */ false,
+                                    /* expected_controlled */ true);
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnhanced,
+                                    /* expected_value */ false,
+                                    /* expected_controlled */ true);
+  listener_false.Reply("ok");
+
+  // Step 4. of the test uninstalls the extension.
+  // Neither preference is now controlled by extension, and they take on their
+  // default values - TRUE and FALSE, respectively.
+  ASSERT_TRUE(listener_done.WaitUntilSatisfied());
+  UninstallExtension(extension->id());
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnabled,
+                                    /* expected_value */ true,
+                                    /* expected_controlled */ false);
+  VerifyPrefValueAndControlledState(prefs::kSafeBrowsingEnhanced,
+                                    /* expected_value */ false,
+                                    /* expected_controlled */ false);
+}

chrome/test/data/extensions/api_test/preference/safe_browsing/manifest.json

+{
+  "name" : "Preference API Test Extension (Safe Browsing - True)",
+  "version" : "1.0",
+  "manifest_version": 2,
+  "description" : "Tests the behavior of the Safe Browsing API.",
+  "permissions": [ "privacy" ],
+  "background": {
+    "page": "test.html"
+  }
+}

chrome/test/data/extensions/api_test/preference/safe_browsing/test.html

+<!--
+ * Copyright 2020 The Chromium Authors. All rights reserved.  Use of this
+ * source code is governed by a BSD-style license that can be found in the
+ * LICENSE file.
+-->
+<script src="test.js"></script>

chrome/test/data/extensions/api_test/preference/safe_browsing/test.js

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// API test for chrome.privacy.services.safeBrowsingEnabled
+// Run with browser_tests --gtest_filter=ExtensionApiTest.SafeBrowsing
+
+function setTrue(callback) {
+  chrome.privacy.services.safeBrowsingEnabled.set({ value: true }, function() {
+    chrome.test.sendMessage("set to true", callback);
+  });
+}
+
+function setFalse(callback) {
+  chrome.privacy.services.safeBrowsingEnabled.set({ value: false }, function() {
+    chrome.test.sendMessage("set to false", callback);
+  });
+}
+
+function clearPref(callback) {
+  chrome.privacy.services.safeBrowsingEnabled.clear({}, function() {
+    chrome.test.sendMessage("cleared", callback);
+  });
+}
+
+// Run the following steps:
+// 1. Set the pref to true.
+// 2. Clear the pref.
+// 3. Set the pref to false.
+// 4. Done.
+// The callback of each step is calling the next step.
+setTrue(clearPref.bind(this, setFalse.bind(this, function() {
+  chrome.test.sendMessage("done");
+})));