From 6e3502301e8d50c5ddc7fb1a71dc840f0e8f2c23 Mon Sep 17 00:00:00 2001
From: Chris Thompson <cthomp@chromium.org>
Date: Tue, 21 Nov 2023 23:21:08 +0000
Subject: [PATCH] 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 <dpapad@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Chris Thompson <cthomp@chromium.org>
Reviewed-by: Rainhard Findling <rainhard@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1227664}
---
 .../api/settings_private/prefs_util.cc        |   2 +-
 .../settings/privacy_page/security_page.html  |   5 +-
 .../settings/privacy_page/security_page.ts    |  23 +++
 .../ssl/generated_https_first_mode_pref.cc    | 104 ++++++++++--
 .../ssl/generated_https_first_mode_pref.h     |   6 +-
 ...enerated_https_first_mode_pref_unittest.cc | 154 ++++++++++++++++--
 .../ssl/https_first_mode_settings_tracker.h   |   3 +
 7 files changed, 262 insertions(+), 35 deletions(-)

diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc b/chrome/browser/extensions/api/settings_private/prefs_util.cc
index 0c36d4a558eb39..6eea7035c37485 100644
--- 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
index 4dd4a5cfd9e47b..043c0a4a32328b 100644
--- 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
index e41c769b4ccccd..640c86723d7e3c 100644
--- 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
index ea35b9ddb2d118..2d828ee1ee64a4 100644
--- 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
index 3c5e98caa505fe..7f05228df7b1ce 100644
--- 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
diff --git a/chrome/browser/ssl/generated_https_first_mode_pref_unittest.cc b/chrome/browser/ssl/generated_https_first_mode_pref_unittest.cc
index 4b3334c88afbb6..e317684d10b1db 100644
--- a/chrome/browser/ssl/generated_https_first_mode_pref_unittest.cc
+++ b/chrome/browser/ssl/generated_https_first_mode_pref_unittest.cc
@@ -2,12 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ssl/generated_https_first_mode_pref.h"
+#include <memory>
+
+#include "base/test/scoped_feature_list.h"
+#include "base/values.h"
 #include "chrome/browser/extensions/api/settings_private/generated_pref_test_base.h"
 #include "chrome/browser/extensions/api/settings_private/generated_prefs_factory.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/signin/identity_test_environment_profile_adaptor.h"
+#include "chrome/browser/ssl/generated_https_first_mode_pref.h"
+#include "chrome/browser/ssl/https_first_mode_settings_tracker.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/signin/public/identity_manager/identity_test_environment.h"
@@ -79,14 +85,18 @@ TEST_F(GeneratedHttpsFirstModePrefTest,
   safe_browsing::AdvancedProtectionStatusManager* aps_manager =
       safe_browsing::AdvancedProtectionStatusManagerFactory::GetForProfile(
           profile());
-  EXPECT_FALSE(pref.GetPrefObject().value->GetBool());
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kDisabled);
   EXPECT_FALSE(*pref.GetPrefObject().user_control_disabled);
   EXPECT_EQ(test_observer.GetUpdatedPrefName(), kGeneratedHttpsFirstModePref);
   test_observer.Reset();
 
   // Enable Advanced Protection. This should disable changing the pref.
   aps_manager->SetAdvancedProtectionStatusForTesting(true);
-  EXPECT_TRUE(pref.GetPrefObject().value->GetBool());
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kEnabledFull);
   EXPECT_TRUE(*pref.GetPrefObject().user_control_disabled);
   EXPECT_EQ(test_observer.GetUpdatedPrefName(), kGeneratedHttpsFirstModePref);
 
@@ -106,7 +116,9 @@ TEST_F(GeneratedHttpsFirstModePrefTest,
   safe_browsing::AdvancedProtectionStatusManager* aps_manager =
       safe_browsing::AdvancedProtectionStatusManagerFactory::GetForProfile(
           profile());
-  EXPECT_FALSE(pref.GetPrefObject().value->GetBool());
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kDisabled);
   EXPECT_FALSE(*pref.GetPrefObject().user_control_disabled);
   // If the user isn't signed in, AP manager doesn't update the AP status on
   // startup, so the pref doesn't get a notification.
@@ -116,7 +128,9 @@ TEST_F(GeneratedHttpsFirstModePrefTest,
   // Enabled Advanced Protection. This should disable changing the pref.
   SignIn(/*is_under_advanced_protection=*/true);
   // aps_manager->SetAdvancedProtectionStatusForTesting(true);
-  EXPECT_TRUE(pref.GetPrefObject().value->GetBool());
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kEnabledFull);
   EXPECT_TRUE(*pref.GetPrefObject().user_control_disabled);
   EXPECT_EQ(test_observer.GetUpdatedPrefName(), kGeneratedHttpsFirstModePref);
 
@@ -132,26 +146,138 @@ TEST_F(GeneratedHttpsFirstModePrefTest, UpdatePreference) {
                                base::Value(false));
 
   // Check setting the generated pref updates the underlying preference.
-  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(true).get()),
+  EXPECT_EQ(
+      pref.SetPref(std::make_unique<base::Value>(
+                       static_cast<int>(HttpsFirstModeSetting::kEnabledFull))
+                       .get()),
+      settings_private::SetPrefResult::SUCCESS);
+  EXPECT_TRUE(prefs()->GetUserPref(prefs::kHttpsOnlyModeEnabled)->GetBool());
+
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(
+                             static_cast<int>(HttpsFirstModeSetting::kDisabled))
+                             .get()),
             settings_private::SetPrefResult::SUCCESS);
+  EXPECT_FALSE(prefs()->GetUserPref(prefs::kHttpsOnlyModeEnabled)->GetBool());
+
+  // Check that changing the underlying preference correctly updates the
+  // generated pref.
+  prefs()->SetUserPref(prefs::kHttpsOnlyModeEnabled,
+                       std::make_unique<base::Value>(true));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kEnabledFull);
+
+  prefs()->SetUserPref(prefs::kHttpsOnlyModeEnabled,
+                       std::make_unique<base::Value>(false));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kDisabled);
+
+  // Confirm that a type mismatch is reported as such.
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(true).get()),
+            extensions::settings_private::SetPrefResult::PREF_TYPE_MISMATCH);
+
+  // Confirm that an integer value outside the enum range is reported as a type
+  // mismatch.
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(10).get()),
+            extensions::settings_private::SetPrefResult::PREF_TYPE_MISMATCH);
+
+  // With HFM-in-Incognito feature disabled, check that trying to set the
+  // generated pref to kEnabledIncognito fails and the underlying pref remains
+  // disabled.
+  EXPECT_EQ(pref.SetPref(
+                std::make_unique<base::Value>(
+                    static_cast<int>(HttpsFirstModeSetting::kEnabledIncognito))
+                    .get()),
+            settings_private::SetPrefResult::PREF_TYPE_UNSUPPORTED);
+
+  // With HFM-in-Incognito feature disabled, check that setting the underlying
+  // Incognito pref to `true` does not change the generated pref from kDisabled.
+  prefs()->SetUserPref(prefs::kHttpsFirstModeIncognito,
+                       std::make_unique<base::Value>(true));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kDisabled);
+}
+
+// Variant of UpdatePreference, but with the HFM-in-Incognito feature flag
+// enabled.
+TEST_F(GeneratedHttpsFirstModePrefTest, UpdatePref_HttpsFirstModeIncognito) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kHttpsFirstModeIncognito);
+
+  GeneratedHttpsFirstModePref pref(profile());
+
+  // Setup baseline profile preference.
+  prefs()->SetDefaultPrefValue(prefs::kHttpsOnlyModeEnabled,
+                               base::Value(false));
+
+  // Check setting the generated pref updates the underlying preference.
+  EXPECT_EQ(
+      pref.SetPref(std::make_unique<base::Value>(
+                       static_cast<int>(HttpsFirstModeSetting::kEnabledFull))
+                       .get()),
+      settings_private::SetPrefResult::SUCCESS);
   EXPECT_TRUE(prefs()->GetUserPref(prefs::kHttpsOnlyModeEnabled)->GetBool());
+  EXPECT_TRUE(prefs()->GetUserPref(prefs::kHttpsFirstModeIncognito)->GetBool());
+
+  EXPECT_EQ(pref.SetPref(
+                std::make_unique<base::Value>(
+                    static_cast<int>(HttpsFirstModeSetting::kEnabledIncognito))
+                    .get()),
+            settings_private::SetPrefResult::SUCCESS);
+  EXPECT_FALSE(prefs()->GetUserPref(prefs::kHttpsOnlyModeEnabled)->GetBool());
+  EXPECT_TRUE(prefs()->GetUserPref(prefs::kHttpsFirstModeIncognito)->GetBool());
 
-  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(false).get()),
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(
+                             static_cast<int>(HttpsFirstModeSetting::kDisabled))
+                             .get()),
             settings_private::SetPrefResult::SUCCESS);
   EXPECT_FALSE(prefs()->GetUserPref(prefs::kHttpsOnlyModeEnabled)->GetBool());
+  EXPECT_FALSE(
+      prefs()->GetUserPref(prefs::kHttpsFirstModeIncognito)->GetBool());
 
   // Check that changing the underlying preference correctly updates the
   // generated pref.
   prefs()->SetUserPref(prefs::kHttpsOnlyModeEnabled,
                        std::make_unique<base::Value>(true));
-  EXPECT_TRUE(pref.GetPrefObject().value->GetBool());
+  prefs()->SetUserPref(prefs::kHttpsFirstModeIncognito,
+                       std::make_unique<base::Value>(true));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kEnabledFull);
 
   prefs()->SetUserPref(prefs::kHttpsOnlyModeEnabled,
                        std::make_unique<base::Value>(false));
-  EXPECT_FALSE(pref.GetPrefObject().value->GetBool());
+  prefs()->SetUserPref(prefs::kHttpsFirstModeIncognito,
+                       std::make_unique<base::Value>(true));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kEnabledIncognito);
+
+  prefs()->SetUserPref(prefs::kHttpsOnlyModeEnabled,
+                       std::make_unique<base::Value>(false));
+  prefs()->SetUserPref(prefs::kHttpsFirstModeIncognito,
+                       std::make_unique<base::Value>(false));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kDisabled);
+
+  prefs()->SetUserPref(prefs::kHttpsOnlyModeEnabled,
+                       std::make_unique<base::Value>(true));
+  prefs()->SetUserPref(prefs::kHttpsFirstModeIncognito,
+                       std::make_unique<base::Value>(false));
+  EXPECT_EQ(
+      static_cast<HttpsFirstModeSetting>(pref.GetPrefObject().value->GetInt()),
+      HttpsFirstModeSetting::kEnabledFull);
 
   // Confirm that a type mismatch is reported as such.
-  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(2).get()),
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(true).get()),
+            extensions::settings_private::SetPrefResult::PREF_TYPE_MISMATCH);
+
+  // Confirm that an integer value outside the enum range is reported as a type
+  // mismatch.
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(10).get()),
             extensions::settings_private::SetPrefResult::PREF_TYPE_MISMATCH);
 }
 
@@ -169,7 +295,9 @@ TEST_F(GeneratedHttpsFirstModePrefTest, ManagementState) {
                               std::make_unique<base::Value>(true));
   EXPECT_EQ(pref.GetPrefObject().enforcement,
             settings_api::Enforcement::kRecommended);
-  EXPECT_EQ(pref.GetPrefObject().recommended_value->GetBool(), true);
+  EXPECT_EQ(static_cast<HttpsFirstModeSetting>(
+                pref.GetPrefObject().recommended_value->GetInt()),
+            HttpsFirstModeSetting::kEnabledFull);
 
   // Set HTTPS-Only Mode with full enforcement and check the generated pref.
   prefs()->SetManagedPref(prefs::kHttpsOnlyModeEnabled,
@@ -181,6 +309,8 @@ TEST_F(GeneratedHttpsFirstModePrefTest, ManagementState) {
 
   // Check that the generated pref cannot be changed when the backing pref is
   // managed.
-  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(true).get()),
+  EXPECT_EQ(pref.SetPref(std::make_unique<base::Value>(
+                             static_cast<int>(HttpsFirstModeSetting::kDisabled))
+                             .get()),
             settings_private::SetPrefResult::PREF_NOT_MODIFIABLE);
 }
diff --git a/chrome/browser/ssl/https_first_mode_settings_tracker.h b/chrome/browser/ssl/https_first_mode_settings_tracker.h
index 698404795658a6..50286c129e211a 100644
--- a/chrome/browser/ssl/https_first_mode_settings_tracker.h
+++ b/chrome/browser/ssl/https_first_mode_settings_tracker.h
@@ -30,8 +30,11 @@ class SiteEngagementService;
 
 class StatefulSSLHostStateDelegate;
 
+// The set of valid states of the user-controllable HTTPS-First Mode setting.
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
+// Must be kept in sync with the HttpsFirstModeSetting enum located in
+// chrome/browser/resources/settings/privacy_page/security_page.js.
 enum class HttpsFirstModeSetting {
   kDisabled = 0,
   kEnabledIncognito = 1,