Safety check: added separate states for SafeBrowsing standard and enhanced

Bug: 1015841, 1068587
Change-Id: I84a2b48fb179a5d8959864cf4429a18130ca2d12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141983Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Commit-Queue: Andrey Zaytsev <andzaytsev@google.com>
Cr-Commit-Position: refs/heads/master@{#759617}

chrome/app/settings_strings.grdp

@@ -1049,6 +1049,12 @@
   <message name="IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_ENABLED" desc="This text points out that Safe Browsing is enabled and that the user is protected.">
     Safe Browsing is on and protecting you from harmful sites and downloads
   </message>
+  <message name="IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_ENABLED_STANDARD" desc="This text points out that Safe Browsing is enabled as standard protection.">
+    Standard Protection is on
+  </message>
+  <message name="IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_ENABLED_ENHANCED" desc="This text points out that Safe Browsing is enabled as enhanced protection.">
+    Enhanced Protection is on
+  </message>
   <message name="IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_DISABLED_BY_ADMIN" desc="This text points out that Safe Browsing is disabled by an administrator.">
     <ph name="BEGIN_LINK">&lt;a target="_blank" href="$1"&gt;</ph>Your administrator<ph name="END_LINK">&lt;/a&gt;</ph> has turned off Safe Browsing
   </message>

chrome/browser/ui/webui/settings/safety_check_handler.cc

@@ -156,14 +156,17 @@ void SafetyCheckHandler::CheckUpdates() {
 
 void SafetyCheckHandler::CheckSafeBrowsing() {
   PrefService* pref_service = Profile::FromWebUI(web_ui())->GetPrefs();
-  const PrefService::Preference* pref =
+  const PrefService::Preference* enabled_pref =
       pref_service->FindPreference(prefs::kSafeBrowsingEnabled);
+  bool enabled = pref_service->GetBoolean(prefs::kSafeBrowsingEnabled);
   SafeBrowsingStatus status;
-  if (pref_service->GetBoolean(prefs::kSafeBrowsingEnabled)) {
-    status = SafeBrowsingStatus::kEnabled;
-  } else if (pref->IsManaged()) {
+  if (enabled && pref_service->GetBoolean(prefs::kSafeBrowsingEnhanced)) {
+    status = SafeBrowsingStatus::kEnabledEnhanced;
+  } else if (enabled) {
+    status = SafeBrowsingStatus::kEnabledStandard;
+  } else if (enabled_pref->IsManaged()) {
     status = SafeBrowsingStatus::kDisabledByAdmin;
-  } else if (pref->IsExtensionControlled()) {
+  } else if (enabled_pref->IsExtensionControlled()) {
     status = SafeBrowsingStatus::kDisabledByExtension;
   } else {
     status = SafeBrowsingStatus::kDisabled;
@@ -343,8 +346,12 @@ base::string16 SafetyCheckHandler::GetStringForSafeBrowsing(
     case SafeBrowsingStatus::kChecking:
       return l10n_util::GetStringUTF16(IDS_SETTINGS_SAFETY_CHECK_RUNNING);
     case SafeBrowsingStatus::kEnabled:
+    case SafeBrowsingStatus::kEnabledStandard:
       return l10n_util::GetStringUTF16(
-          IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_ENABLED);
+          IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_ENABLED_STANDARD);
+    case SafeBrowsingStatus::kEnabledEnhanced:
+      return l10n_util::GetStringUTF16(
+          IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_ENABLED_ENHANCED);
     case SafeBrowsingStatus::kDisabled:
       return l10n_util::GetStringUTF16(
           IDS_SETTINGS_SAFETY_CHECK_SAFE_BROWSING_DISABLED);

chrome/browser/ui/webui/settings/safety_check_handler.h

@@ -47,12 +47,15 @@ class SafetyCheckHandler
   };
   enum class SafeBrowsingStatus {
     kChecking = 0,
+    // Enabled is deprecated; kept not to break old UMA metrics (enums.xml).
     kEnabled = 1,
     kDisabled = 2,
     kDisabledByAdmin = 3,
     kDisabledByExtension = 4,
+    kEnabledStandard = 5,
+    kEnabledEnhanced = 6,
     // New enum values must go above here.
-    kMaxValue = kDisabledByExtension,
+    kMaxValue = kEnabledEnhanced,
   };
   enum class PasswordsStatus {
     kChecking = 0,

chrome/browser/ui/webui/settings/safety_check_handler_unittest.cc

@@ -446,22 +446,66 @@ TEST_F(SafetyCheckHandlerTest, CheckUpdates_DestroyedOnJavascriptDisallowed) {
   EXPECT_TRUE(TestDestructionVersionUpdater::GetDestructorInvoked());
 }
 
-TEST_F(SafetyCheckHandlerTest, CheckSafeBrowsing_Enabled) {
+TEST_F(SafetyCheckHandlerTest, CheckSafeBrowsing_EnabledStandard) {
   Profile::FromWebUI(&test_web_ui_)
       ->GetPrefs()
       ->SetBoolean(prefs::kSafeBrowsingEnabled, true);
+  Profile::FromWebUI(&test_web_ui_)
+      ->GetPrefs()
+      ->SetBoolean(prefs::kSafeBrowsingEnhanced, false);
   safety_check_->PerformSafetyCheck();
   const base::DictionaryValue* event =
       GetSafetyCheckStatusChangedWithDataIfExists(
           kSafeBrowsing,
-          static_cast<int>(SafetyCheckHandler::SafeBrowsingStatus::kEnabled));
+          static_cast<int>(
+              SafetyCheckHandler::SafeBrowsingStatus::kEnabledStandard));
   ASSERT_TRUE(event);
-  VerifyDisplayString(event,
-                      "Safe Browsing is on and protecting you from harmful "
-                      "sites and downloads");
+  VerifyDisplayString(event, "Standard Protection is on");
+  histogram_tester_.ExpectBucketCount(
+      "Settings.SafetyCheck.SafeBrowsingResult",
+      SafetyCheckHandler::SafeBrowsingStatus::kEnabledStandard, 1);
+}
+
+TEST_F(SafetyCheckHandlerTest, CheckSafeBrowsing_EnabledEnhanced) {
+  Profile::FromWebUI(&test_web_ui_)
+      ->GetPrefs()
+      ->SetBoolean(prefs::kSafeBrowsingEnabled, true);
+  Profile::FromWebUI(&test_web_ui_)
+      ->GetPrefs()
+      ->SetBoolean(prefs::kSafeBrowsingEnhanced, true);
+  safety_check_->PerformSafetyCheck();
+  const base::DictionaryValue* event =
+      GetSafetyCheckStatusChangedWithDataIfExists(
+          kSafeBrowsing,
+          static_cast<int>(
+              SafetyCheckHandler::SafeBrowsingStatus::kEnabledEnhanced));
+  ASSERT_TRUE(event);
+  VerifyDisplayString(event, "Enhanced Protection is on");
   histogram_tester_.ExpectBucketCount(
       "Settings.SafetyCheck.SafeBrowsingResult",
-      SafetyCheckHandler::SafeBrowsingStatus::kEnabled, 1);
+      SafetyCheckHandler::SafeBrowsingStatus::kEnabledEnhanced, 1);
+}
+
+TEST_F(SafetyCheckHandlerTest, CheckSafeBrowsing_InconsistentEnhanced) {
+  // Tests the corner case of SafeBrowsingEnhanced pref being on, while
+  // SafeBrowsing enabled is off. This should be treated as SB off.
+  Profile::FromWebUI(&test_web_ui_)
+      ->GetPrefs()
+      ->SetBoolean(prefs::kSafeBrowsingEnabled, false);
+  Profile::FromWebUI(&test_web_ui_)
+      ->GetPrefs()
+      ->SetBoolean(prefs::kSafeBrowsingEnhanced, true);
+  safety_check_->PerformSafetyCheck();
+  const base::DictionaryValue* event =
+      GetSafetyCheckStatusChangedWithDataIfExists(
+          kSafeBrowsing,
+          static_cast<int>(SafetyCheckHandler::SafeBrowsingStatus::kDisabled));
+  ASSERT_TRUE(event);
+  VerifyDisplayString(
+      event, "Safe Browsing is off. Browser recommends turning it on.");
+  histogram_tester_.ExpectBucketCount(
+      "Settings.SafetyCheck.SafeBrowsingResult",
+      SafetyCheckHandler::SafeBrowsingStatus::kDisabled, 1);
 }
 
 TEST_F(SafetyCheckHandlerTest, CheckSafeBrowsing_Disabled) {

tools/metrics/histograms/enums.xml

@@ -58065,6 +58065,8 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
   <int value="2" label="DISABLED"/>
   <int value="3" label="DISABLED_BY_ADMIN"/>
   <int value="4" label="DISABLED_BY_EXTENSION"/>
+  <int value="5" label="ENABLED_STANDARD"/>
+  <int value="6" label="ENABLED_ENHANCED"/>
 </enum>
 
 <enum name="SafetyCheckUpdateStatus">