Convert GeneratedHttpsFirstModePref into a tri-state enum

GeneratedHttpsFirstModePref is used for managing the HTTPS-First Mode setting in the Security settings page. This updates it to match the HttpsFirstModeSettings enum that adds new states besides "disabled" and "fully enabled", and hooks it up to the two HTTPS-First Mode prefs kHttpsOnlyModeEnabled and kHttpsFIrstModeIncognito. This is an initial step towards adding a new Settings UI (when the HttpsFirstModeIcnognito feature flag is enabled) to control the tri-state of HFM (EnabledFull, EnabledIncognito, Disabled). Having the SettingsToggleButton control the underlying numeric setting will make it easier to have both versions of the Settings control use the same underlying generated pref logic. Bug: 1494186 Change-Id: Iae8dc496f786a60fb5d89bcd6f973eafbd5f7739 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5041962 Reviewed-by: Demetrios Papadopoulos <[email protected]> Code-Coverage: [email protected] <[email protected]> Reviewed-by: Mustafa Emre Acer <[email protected]> Commit-Queue: Chris Thompson <[email protected]> Reviewed-by: Rainhard Findling <[email protected]> Cr-Commit-Position: refs/heads/main@{#1227664}
Igalia · Nov 21, 2023 · 6e35023 · 6e35023
1 parent 3b9010d
commit 6e35023
Show file tree

Hide file tree

Showing 7 changed files with 262 additions and 35 deletions.
diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc b/chrome/browser/extensions/api/settings_private/prefs_util.cc
@@ -339,7 +339,7 @@ const PrefsUtil::TypedPrefMap& PrefsUtil::GetAllowlistedKeys() {
   (*s_allowlist)[::prefs::kHttpsOnlyModeEnabled] =
       settings_api::PrefType::kBoolean;
   (*s_allowlist)[::kGeneratedHttpsFirstModePref] =
-      settings_api::PrefType::kBoolean;
+      settings_api::PrefType::kNumber;
 
   // Cookies page
   (*s_allowlist)[::prefs::kCookieControlsMode] =

diff --git a/chrome/browser/resources/settings/privacy_page/security_page.html b/chrome/browser/resources/settings/privacy_page/security_page.html
@@ -69,7 +69,7 @@
       }
 
       #safeBrowsingEnhanced {
-        --cr-radio-button-unchecked-ripple-color: 
+        --cr-radio-button-unchecked-ripple-color:
             var(--cr-radio-button-checked-ripple-color);
       }
     </style>
@@ -260,7 +260,8 @@ <h2>$i18n{advancedPageTitle}</h2>
         pref="{{prefs.generated.https_first_mode_enabled}}"
         label="$i18n{httpsOnlyModeTitle}"
         sub-label="[[getHttpsFirstModeSubLabel_(
-                       prefs.generated.https_first_mode_enabled.*)]]">
+                       prefs.generated.https_first_mode_enabled.*)]]"
+        numeric-checked-value="[[httpsFirstModeSettingEnum_.ENABLED_FULL]]">
     </settings-toggle-button>
     <template is="dom-if" if="[[showSecureDnsSetting_]]">
       <settings-secure-dns prefs="{{prefs}}"></settings-secure-dns>

diff --git a/chrome/browser/resources/settings/privacy_page/security_page.ts b/chrome/browser/resources/settings/privacy_page/security_page.ts
@@ -47,6 +47,17 @@ export enum SafeBrowsingSetting {
   DISABLED = 2,
 }
 
+/**
+ * Enumeration of all HTTPS-First Mode setting states. Must be kept in sync with
+ * the enum of the same name located in:
+ * chrome/browser/ssl/https_first_mode_settings_tracker.h
+ */
+enum HttpsFirstModeSetting {
+  DISABLED = 0,
+  ENABLED_INCOGNITO = 1,
+  ENABLED_FULL = 2,
+}
+
 export interface SettingsSecurityPageElement {
   $: {
     passwordsLeakToggle: SettingsToggleButtonElement,
@@ -127,6 +138,14 @@ export class SettingsSecurityPageElement extends
         value: SafeBrowsingSetting,
       },
 
+      /**
+       * Valid HTTPS-First Mode states.
+       */
+      httpsFirstModeSettingEnum_: {
+        type: Object,
+        value: HttpsFirstModeSetting,
+      },
+
       enableSecurityKeysSubpage_: {
         type: Boolean,
         readOnly: true,
@@ -224,6 +243,10 @@ export class SettingsSecurityPageElement extends
       } else if (prefValue === SafeBrowsingSetting.STANDARD) {
         this.$.safeBrowsingStandard.expanded = true;
       }
+
+      assert(
+          this.getPref('generated.https_first_mode_enabled').value !==
+          HttpsFirstModeSetting.ENABLED_INCOGNITO);
     });
 
     this.registerHelpBubble(

diff --git a/chrome/browser/ssl/generated_https_first_mode_pref.cc b/chrome/browser/ssl/generated_https_first_mode_pref.cc
@@ -5,10 +5,12 @@
 #include "chrome/browser/ssl/generated_https_first_mode_pref.h"
 
 #include "base/feature_list.h"
-#include "base/logging.h"
+#include "base/functional/bind.h"
+#include "chrome/browser/extensions/api/settings_private/prefs_util_enums.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/advanced_protection_status_manager.h"
 #include "chrome/browser/safe_browsing/advanced_protection_status_manager_factory.h"
+#include "chrome/browser/ssl/https_first_mode_settings_tracker.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/api/settings_private.h"
 #include "chrome/common/pref_names.h"
@@ -27,6 +29,11 @@ GeneratedHttpsFirstModePref::GeneratedHttpsFirstModePref(Profile* profile)
       base::BindRepeating(
           &GeneratedHttpsFirstModePref::OnSourcePreferencesChanged,
           base::Unretained(this)));
+  user_prefs_registrar_.Add(
+      prefs::kHttpsFirstModeIncognito,
+      base::BindRepeating(
+          &GeneratedHttpsFirstModePref::OnSourcePreferencesChanged,
+          base::Unretained(this)));
 
   // Track Advanced Protection status.
   if (base::FeatureList::IsEnabled(
@@ -56,48 +63,111 @@ void GeneratedHttpsFirstModePref::OnAdvancedProtectionStatusChanged(
   NotifyObservers(kGeneratedHttpsFirstModePref);
 }
 
+// Convert the setting selection into values for the two underlying boolean
+// prefs.
 extensions::settings_private::SetPrefResult
 GeneratedHttpsFirstModePref::SetPref(const base::Value* value) {
-  if (!value->is_bool()) {
+  if (!value->is_int()) {
     return extensions::settings_private::SetPrefResult::PREF_TYPE_MISMATCH;
   }
 
-  if (!profile_->GetPrefs()
-           ->FindPreference(prefs::kHttpsOnlyModeEnabled)
-           ->IsUserModifiable()) {
+  auto selection = static_cast<HttpsFirstModeSetting>(value->GetInt());
+
+  if (selection != HttpsFirstModeSetting::kDisabled &&
+      selection != HttpsFirstModeSetting::kEnabledIncognito &&
+      selection != HttpsFirstModeSetting::kEnabledFull) {
+    return extensions::settings_private::SetPrefResult::PREF_TYPE_MISMATCH;
+  }
+
+  if (!base::FeatureList::IsEnabled(features::kHttpsFirstModeIncognito) &&
+      selection == HttpsFirstModeSetting::kEnabledIncognito) {
+    return extensions::settings_private::SetPrefResult::PREF_TYPE_UNSUPPORTED;
+  }
+
+  // kHttpsOnlyModeEnabled is considered the canonical source for HFM
+  // management. Policy enforcement turns it fully on or fully off.
+  const PrefService::Preference* enabled_pref =
+      profile_->GetPrefs()->FindPreference(prefs::kHttpsOnlyModeEnabled);
+  if (!enabled_pref->IsUserModifiable()) {
     return extensions::settings_private::SetPrefResult::PREF_NOT_MODIFIABLE;
   }
 
-  profile_->GetPrefs()->SetBoolean(prefs::kHttpsOnlyModeEnabled,
-                                   value->GetBool());
+  // Update both HTTPS-First Mode preferences to match the selection.
+  //
+  // Note that the HttpsFirstModeSetting::kEnabledIncognito is not available by
+  // default. If the feature flag is disabled, then the kEnabledFull and
+  // kDisabled settings will only be mapped to the kHttpsOnlyModeEnabled pref.
+  if (base::FeatureList::IsEnabled(features::kHttpsFirstModeIncognito)) {
+    profile_->GetPrefs()->SetBoolean(
+        prefs::kHttpsFirstModeIncognito,
+        selection != HttpsFirstModeSetting::kDisabled);
+  }
+  profile_->GetPrefs()->SetBoolean(
+      prefs::kHttpsOnlyModeEnabled,
+      selection == HttpsFirstModeSetting::kEnabledFull);
+
   return extensions::settings_private::SetPrefResult::SUCCESS;
 }
 
+// Convert the underlying boolean prefs into the setting selection.
 settings_api::PrefObject GeneratedHttpsFirstModePref::GetPrefObject() const {
   bool is_advanced_protection_enabled =
       safe_browsing::AdvancedProtectionStatusManagerFactory::GetForProfile(
           profile_)
           ->IsUnderAdvancedProtection();
 
-  auto* backing_preference =
+  // prefs::kHttpsOnlyModeEnabled is the backing pref that can be controlled by
+  // enterprise policy.
+  auto* hfm_fully_enabled_pref =
       profile_->GetPrefs()->FindPreference(prefs::kHttpsOnlyModeEnabled);
 
+  // The HttpsFirstModeSetting::kEnabledIncognito setting is not available  by
+  // default -- if the feature flag is disabled, then the kEnabledFull and
+  // kDisabled settings will be mapped to the kHttpsOnlyModeEnabled pref on its
+  // own.
+  bool hfm_incognito_enabled =
+      base::FeatureList::IsEnabled(features::kHttpsFirstModeIncognito) &&
+      profile_->GetPrefs()->GetBoolean(prefs::kHttpsFirstModeIncognito);
+
+  bool fully_enabled = hfm_fully_enabled_pref->GetValue()->GetBool() ||
+                       is_advanced_protection_enabled;
+
   settings_api::PrefObject pref_object;
   pref_object.key = kGeneratedHttpsFirstModePref;
-  pref_object.type = settings_api::PrefType::kBoolean;
-  pref_object.value = base::Value(backing_preference->GetValue()->GetBool() ||
-                                  is_advanced_protection_enabled);
+  pref_object.type = settings_api::PrefType::kNumber;
+
+  // Map the two boolean prefs to the enum setting states.
+  if (fully_enabled) {
+    pref_object.value =
+        base::Value(static_cast<int>(HttpsFirstModeSetting::kEnabledFull));
+  } else if (hfm_incognito_enabled) {
+    pref_object.value =
+        base::Value(static_cast<int>(HttpsFirstModeSetting::kEnabledIncognito));
+  } else {
+    pref_object.value =
+        base::Value(static_cast<int>(HttpsFirstModeSetting::kDisabled));
+  }
+
   pref_object.user_control_disabled = is_advanced_protection_enabled;
 
-  if (!backing_preference->IsUserModifiable()) {
-    // The pref was disabled by the enterprise policy.
+  if (!hfm_fully_enabled_pref->IsUserModifiable()) {
+    // The pref was controlled by the enterprise policy.
     pref_object.enforcement = settings_api::Enforcement::kEnforced;
     extensions::settings_private::GeneratedPref::ApplyControlledByFromPref(
-        &pref_object, backing_preference);
-  } else if (backing_preference->GetRecommendedValue()) {
+        &pref_object, hfm_fully_enabled_pref);
+  } else if (hfm_fully_enabled_pref->GetRecommendedValue()) {
+    // Policy can set a recommended setting of fully enabled or fully disabled.
+    // Map those to the enum values.
     pref_object.enforcement = settings_api::Enforcement::kRecommended;
-    pref_object.recommended_value =
-        base::Value(backing_preference->GetRecommendedValue()->GetBool());
+    if (hfm_fully_enabled_pref->GetRecommendedValue()->GetBool()) {
+      pref_object.recommended_value =
+          base::Value(static_cast<int>(HttpsFirstModeSetting::kEnabledFull));
+    } else {
+      // TODO(crbug.com/1494186): Consider supporting a recommended value of
+      // kEnabledIncognito after the enterprise policy support is updated.
+      pref_object.recommended_value =
+          base::Value(static_cast<int>(HttpsFirstModeSetting::kDisabled));
+    }
   }
 
   return pref_object;

diff --git a/chrome/browser/ssl/generated_https_first_mode_pref.h b/chrome/browser/ssl/generated_https_first_mode_pref.h
@@ -12,9 +12,9 @@
 #include "chrome/browser/safe_browsing/advanced_protection_status_manager.h"
 #include "components/prefs/pref_change_registrar.h"
 
-// The generated pref for HTTPS-First Mode. Only used for enabling/disabling
-// the setting in the Security UI settings page. The actual HTTPS-First Mode
-// is controlled by prefs::kHttpsOnlyModeEnabled.
+// The generated pref for HTTPS-First Mode. Only used for managing the setting
+// in the Security UI settings page. The actual HTTPS-First Mode is controlled
+// by prefs::kHttpsOnlyModeEnabled and prefs::kHttpsFirstModeIncognito.
 extern const char kGeneratedHttpsFirstModePref[];
 
 class GeneratedHttpsFirstModePref