[HFM] Ignore URLs with non-default ports from Site Engagement Heuristic

Site Engagement heuristic can auto-enable HTTPS-First Mode if a hostname
has a high engagement score on its HTTPS URLs and a low score on its
HTTP URLs. Presently, it can auto-enable HFM on an origin with a
non-default port. However, the persisted HTTPS-enforcement list only
stores hostnames, so a high site engagement score on
https://example.com:8443 enables HFM on all of example.com.
Reverse is also true: a high score on https://example.com auto-enables
HFM on https://example.com:8443. This behavior may be confusing to
developers and is in general error prone.

This CL does two things:
- It ignores site engagement scores of origins with non-default ports.
- It stops automatically enforcing HTTPS on origins with non-default
ports.

Bug: 1442679
Change-Id: I69bc0376b32adf501f2e5f61e71cfd35eeb9dd8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5035039
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Chris Thompson <cthomp@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1225896}

android_webview/browser/aw_ssl_host_state_delegate.cc

@@ -82,8 +82,8 @@ void AwSSLHostStateDelegate::SetHttpsEnforcementForHost(
   // Intentional no-op for Android WebView.
 }
 
-bool AwSSLHostStateDelegate::IsHttpsEnforcedForHost(
+bool AwSSLHostStateDelegate::IsHttpsEnforcedForUrl(
-    const std::string& host,
+    const GURL& url,
     content::StoragePartition* storage_partition) {
   // Intentional no-op for Android WebView. Return value does not matter as
   // HTTPS-First Mode is not enabled on WebView.

android_webview/browser/aw_ssl_host_state_delegate.h

@@ -86,8 +86,8 @@ class AwSSLHostStateDelegate : public content::SSLHostStateDelegate {
       const std::string& host,
       bool enforce,
       content::StoragePartition* storage_partition) override;
-  bool IsHttpsEnforcedForHost(
+  bool IsHttpsEnforcedForUrl(
-      const std::string& host,
+      const GURL& url,
       content::StoragePartition* storage_partition) override;
 
   // Revokes all SSL certificate error allow exceptions made by the user for

chrome/browser/ssl/https_first_mode_settings_tracker.cc

@@ -527,7 +527,7 @@ void HttpsFirstModeService::ProcessEngagedSitesList(
   // TODO(crbug.com/1435222): Sites dropping off from the engaged sites list
   // should no longer have HTTPS enforced.
   for (const site_engagement::mojom::SiteEngagementDetails& detail : details) {
-    if (detail.origin.SchemeIsHTTPOrHTTPS()) {
+    if (detail.origin.SchemeIsHTTPOrHTTPS() && detail.origin.port().empty()) {
       MaybeEnableHttpsFirstModeForUrl(detail.origin, engagement_service, state);
     }
   }
@@ -541,8 +541,8 @@ void HttpsFirstModeService::MaybeEnableHttpsFirstModeForUrl(
     const GURL& url,
     site_engagement::SiteEngagementService* engagement_service,
     StatefulSSLHostStateDelegate* state) {
-  bool enforced = state->IsHttpsEnforcedForHost(
+  bool enforced =
-      url.host(), profile_->GetDefaultStoragePartition());
+      state->IsHttpsEnforcedForUrl(url, profile_->GetDefaultStoragePartition());
   GURL https_url = url.SchemeIsCryptographic() ? url : GetHttpsUrlFromHttp(url);
   GURL http_url = !url.SchemeIsCryptographic() ? url : GetHttpUrlFromHttps(url);
 

chrome/browser/ssl/https_first_mode_settings_tracker_unittest.cc

@@ -113,21 +113,72 @@ TEST_F(HttpsFirstModeSettingsTrackerTest,
   GURL https_url("https://example.com");
   engagement_service->ResetBaseScoreForURL(https_url, 90);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_TRUE(state->IsHttpsEnforcedForHost(
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
+      GURL("https://example.com"), profile()->GetDefaultStoragePartition()));
 
   // Disable HFM. This should clear the enforcelist.
   profile()->GetPrefs()->SetBoolean(prefs::kHttpsOnlyModeEnabled, false);
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
+      GURL("https://example.com"), profile()->GetDefaultStoragePartition()));
 
   // Check again. Should remain unenforced.
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
+      GURL("https://example.com"), profile()->GetDefaultStoragePartition()));
 }
 
-TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
+// Check that high site engagement scores of HTTPS URLs with non-default ports
+// do not auto-enable HTTPS-First Mode.
+TEST_F(
+    HttpsFirstModeSettingsTrackerTest,
+    SiteEngagementHeuristic_ShouldIgnoreEngagementScoreOfUrlWithNonDefaultPort) {
+  HttpsFirstModeService* service =
+      HttpsFirstModeServiceFactory::GetForProfile(profile());
+  ASSERT_TRUE(service);
+
+  base::HistogramTester histograms;
+  site_engagement::SiteEngagementService* engagement_service =
+      site_engagement::SiteEngagementService::Get(profile());
+  ASSERT_TRUE(engagement_service);
+
+  StatefulSSLHostStateDelegate* state =
+      StatefulSSLHostStateDelegateFactory::GetForProfile(profile());
+  ASSERT_TRUE(state);
+
+  // HFM should initially be disabled on this site by default.
+  MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
+  histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 0);
+  histograms.ExpectTotalCount(kSiteEngagementHeuristicHostCountHistogram, 0);
+  histograms.ExpectTotalCount(
+      kSiteEngagementHeuristicAccumulatedHostCountHistogram, 0);
+  histograms.ExpectTotalCount(
+      kSiteEngagementHeuristicEnforcementDurationHistogram, 0);
+
+  // Increase the score. Since the URL has a non-default port, HFM should remain
+  // disabled.
+  engagement_service->ResetBaseScoreForURL(GURL("https://example.com:8443"),
+                                           90);
+  MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
+  histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 0);
+  histograms.ExpectTotalCount(kSiteEngagementHeuristicHostCountHistogram, 0);
+  histograms.ExpectTotalCount(
+      kSiteEngagementHeuristicAccumulatedHostCountHistogram, 0);
+  histograms.ExpectTotalCount(
+      kSiteEngagementHeuristicEnforcementDurationHistogram, 0);
+}
+
+TEST_F(HttpsFirstModeSettingsTrackerTest,
+       SiteEngagementHeuristic_ShouldEnforceHttps) {
   HttpsFirstModeService* service =
       HttpsFirstModeServiceFactory::GetForProfile(profile());
   ASSERT_TRUE(service);
@@ -155,8 +206,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
 
   // Step 1: HFM should initially be disabled on this site by default.
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 0);
   histograms.ExpectTotalCount(kSiteEngagementHeuristicHostCountHistogram, 0);
   histograms.ExpectTotalCount(
@@ -167,8 +218,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
   // Step 2: Increase the score, should now enable HFM.
   engagement_service->ResetBaseScoreForURL(https_url, 90);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_TRUE(state->IsHttpsEnforcedForHost(
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   // Check events.
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 1);
   histograms.ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
@@ -191,15 +242,16 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
       kSiteEngagementHeuristicEnforcementDurationHistogram, 0);
 
   // Subdomains shouldn't be affected.
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(
-      "test.example.com", profile()->GetDefaultStoragePartition()));
+      state->IsHttpsEnforcedForUrl(GURL("http://test.example.com"),
+                                   profile()->GetDefaultStoragePartition()));
 
   // Step 3: Decrease the score, but only slightly. This shouldn't result in HFM
   // being disabled.
   engagement_service->ResetBaseScoreForURL(https_url, 85);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_TRUE(state->IsHttpsEnforcedForHost(
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   // Check events.
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 1);
   histograms.ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
@@ -226,8 +278,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
   clock_ptr->Advance(base::Hours(1));
   engagement_service->ResetBaseScoreForURL(https_url, 25);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   // Check events.
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 2);
   histograms.ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
@@ -259,8 +311,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
   clock_ptr->Advance(base::Hours(2));
   engagement_service->ResetBaseScoreForURL(https_url, 90);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_TRUE(state->IsHttpsEnforcedForHost(
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   // Check state.
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 3);
   histograms.ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
@@ -292,8 +344,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
   // the HTTPS score is still high.
   engagement_service->ResetBaseScoreForURL(http_url, 20);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   // Check state.
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 4);
   histograms.ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
@@ -329,8 +381,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest, SiteEngagementHeuristic) {
   engagement_service->ResetBaseScoreForURL(https_url, 0);
   engagement_service->ResetBaseScoreForURL(http_url, 100);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_FALSE(state->IsHttpsEnforcedForHost(
+  EXPECT_FALSE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
   // Check state.
   histograms.ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 4);
   histograms.ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
@@ -388,8 +440,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest,
   // Increase the HTTPS engagement score to enable HFM on this host.
   engagement_service->ResetBaseScoreForURL(https_url, 90);
   MaybeEnableHttpsFirstModeForEngagedSitesAndWait(service);
-  EXPECT_TRUE(state->IsHttpsEnforcedForHost(
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
 
   // Clear up the engagement scores. This should remove the enforcement.
   HostContentSettingsMap* settings_map =
@@ -402,8 +454,8 @@ TEST_F(HttpsFirstModeSettingsTrackerTest,
 
   // TODO(crbug.com/1435222): Sites that fall outside the site engagement list
   // should no longer have HTTPS enforced.
-  EXPECT_TRUE(state->IsHttpsEnforcedForHost(
+  EXPECT_TRUE(state->IsHttpsEnforcedForUrl(
-      "example.com", profile()->GetDefaultStoragePartition()));
+      GURL("http://example.com"), profile()->GetDefaultStoragePartition()));
 
   service->Shutdown();
 }

chrome/browser/ssl/https_upgrades_browsertest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <algorithm>
+#include <memory>
 #include <vector>
 
 #include "base/memory/raw_ptr.h"
@@ -727,13 +728,41 @@ void MaybeEnableHttpsFirstModeForEngagedSitesAndWait(
   run_loop.Run();
 }
 
+// Returns a URL loader interceptor that responds to HTTPS URLs with a cert
+// error and to HTTP URLs with a good response.
+std::unique_ptr<content::URLLoaderInterceptor>
+MakeInterceptorForSiteEngagementHeuristic() {
+  return std::make_unique<content::URLLoaderInterceptor>(
+      base::BindLambdaForTesting(
+          [](content::URLLoaderInterceptor::RequestParams* params) {
+            if (params->url_request.url.SchemeIs("https")) {
+              // Fail with an SSL error.
+              network::URLLoaderCompletionStatus status;
+              status.error_code = net::ERR_CERT_COMMON_NAME_INVALID;
+              status.ssl_info = net::SSLInfo();
+              status.ssl_info->cert_status =
+                  net::CERT_STATUS_COMMON_NAME_INVALID;
+              // The cert doesn't matter.
+              status.ssl_info->cert = net::ImportCertFromFile(
+                  net::GetTestCertsDirectory(), "ok_cert.pem");
+              status.ssl_info->unverified_cert = status.ssl_info->cert;
+              params->client->OnComplete(status);
+              return true;
+            }
+            content::URLLoaderInterceptor::WriteResponse(
+                "HTTP/1.1 200 OK\nContent-type: text/html\n\n",
+                "<html>Done</html>", params->client.get());
+            return true;
+          }));
+}
+
 // TODO(https://crbug.com/1435222): Fails on the linux-wayland-rel bot.
 #if defined(OZONE_PLATFORM_WAYLAND)
-#define MAYBE_UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_SiteEngagement \
+#define MAYBE_UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldInterstitial \
-  DISABLED_UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_SiteEngagement
+  DISABLED_UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldInterstitial
 #else
-#define MAYBE_UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_SiteEngagement \
+#define MAYBE_UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldInterstitial \
-  UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_SiteEngagement
+  UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldInterstitial
 #endif
 // Test for Site Engagement Heuristic, a feature that enables HFM on specific
 // sites based on their site engagement scores.
@@ -743,11 +772,16 @@ void MaybeEnableHttpsFirstModeForEngagedSitesAndWait(
 // Heuristic if the interstitial isn't enabled.
 IN_PROC_BROWSER_TEST_P(
     HttpsUpgradesBrowserTest,
-    MAYBE_UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_SiteEngagement) {
+    MAYBE_UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldInterstitial) {
   // HFM+SE is not enabled in Incognito.
   if (IsIncognito()) {
     return;
   }
+  // Disable the testing port configuration, as this test doesn't use the
+  // EmbeddedTestServer.
+  HttpsUpgradesInterceptor::SetHttpsPortForTesting(0);
+  HttpsUpgradesInterceptor::SetHttpPortForTesting(0);
+  auto url_loader_interceptor = MakeInterceptorForSiteEngagementHeuristic();
 
   content::WebContents* contents =
       GetBrowser()->tab_strip_model()->GetActiveWebContents();
@@ -764,8 +798,8 @@ IN_PROC_BROWSER_TEST_P(
   // Start the clock at standard system time.
   clock_ptr->SetNow(base::Time::NowFromSystemTime());
 
-  GURL http_url = http_server()->GetURL("bad-https.com", "/simple.html");
+  GURL http_url("http://bad-https.com");
-  GURL https_url = https_server()->GetURL("bad-https.com", "/simple.html");
+  GURL https_url("https://bad-https.com");
   SetSiteEngagementScore(http_url, kLowSiteEngagementScore);
   SetSiteEngagementScore(https_url, kHighSiteEnagementScore);
   HttpsFirstModeService* hfm_service =
@@ -959,6 +993,123 @@ IN_PROC_BROWSER_TEST_P(
   }
 }
 
+// Test that Site Engagement Heuristic doesn't enforce HTTPS on URLs with
+// non-default ports.
+IN_PROC_BROWSER_TEST_P(
+    HttpsUpgradesBrowserTest,
+    UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldIgnoreUrlsWithNonDefaultPorts) {
+  // HFM+SE is not enabled in Incognito.
+  if (IsIncognito()) {
+    return;
+  }
+  // Disable the testing port configuration, as this test doesn't use the
+  // EmbeddedTestServer.
+  HttpsUpgradesInterceptor::SetHttpsPortForTesting(0);
+  HttpsUpgradesInterceptor::SetHttpPortForTesting(0);
+  auto url_loader_interceptor = MakeInterceptorForSiteEngagementHeuristic();
+
+  content::WebContents* contents =
+      GetBrowser()->tab_strip_model()->GetActiveWebContents();
+  Profile* profile = GetBrowser()->profile();
+  content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
+
+  // Set test clock.
+  auto clock = std::make_unique<base::SimpleTestClock>();
+  auto* clock_ptr = clock.get();
+  StatefulSSLHostStateDelegate* chrome_state =
+      static_cast<StatefulSSLHostStateDelegate*>(state);
+  chrome_state->SetClockForTesting(std::move(clock));
+
+  // Start the clock at standard system time.
+  clock_ptr->SetNow(base::Time::NowFromSystemTime());
+
+  GURL http_url("http://bad-https.com");
+  GURL https_url("https://bad-https.com");
+  GURL navigated_url("http://bad-https.com:8080");
+
+  SetSiteEngagementScore(http_url, kLowSiteEngagementScore);
+  SetSiteEngagementScore(https_url, kHighSiteEnagementScore);
+  HttpsFirstModeService* hfm_service =
+      HttpsFirstModeServiceFactory::GetForProfile(profile);
+  MaybeEnableHttpsFirstModeForEngagedSitesAndWait(hfm_service);
+
+  // This URL should be upgraded by HTTPS-Upgrades, but not have HFM
+  // auto-enabled on it because it has a non-default port.
+  NavigateAndWaitForFallback(contents, navigated_url);
+  EXPECT_EQ(navigated_url, contents->GetLastCommittedURL());
+
+  if (IsHttpsFirstModePrefEnabled()) {
+    EXPECT_TRUE(
+        chrome_browser_interstitials::IsShowingHttpsFirstModeInterstitial(
+            contents));
+    EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText(
+        contents->GetPrimaryMainFrame(),
+        "You are seeing this warning because this site does not support "
+        "HTTPS."));
+  } else {
+    EXPECT_FALSE(
+        chrome_browser_interstitials::IsShowingHttpsFirstModeInterstitial(
+            contents));
+  }
+
+  // Verify that navigation event metrics were correctly recorded.
+  if (IsHttpUpgradingEnabled()) {
+    histograms()->ExpectTotalCount(kEventHistogram, 3);
+    histograms()->ExpectBucketCount(kEventHistogram, Event::kUpgradeAttempted,
+                                    1);
+    histograms()->ExpectBucketCount(kEventHistogram, Event::kUpgradeFailed, 1);
+    histograms()->ExpectBucketCount(kEventHistogram, Event::kUpgradeCertError,
+                                    1);
+
+    // Engagement heuristic shouldn't handle any navigation events because we
+    // didn't navigate to example.com.
+    histograms()->ExpectTotalCount(kEventHistogramWithEngagementHeuristic, 0);
+
+    // Check engagement heuristic metrics. These are only recorded when the
+    // interstitial isn't enabled by the user pref.
+    if (!IsHttpsFirstModePrefEnabled()) {
+      // Check the heuristic state. The heuristic should enable HFM for
+      // example.com
+      histograms()->ExpectTotalCount(kSiteEngagementHeuristicStateHistogram, 1);
+      histograms()->ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
+                                      SiteEngagementHeuristicState::kDisabled,
+                                      0);
+      histograms()->ExpectBucketCount(kSiteEngagementHeuristicStateHistogram,
+                                      SiteEngagementHeuristicState::kEnabled,
+                                      1);
+      // Check host count.
+      histograms()->ExpectTotalCount(kSiteEngagementHeuristicHostCountHistogram,
+                                     1);
+      histograms()->ExpectBucketCount(
+          kSiteEngagementHeuristicHostCountHistogram, 0,
+          /*expected_count=*/0);
+      histograms()->ExpectBucketCount(
+          kSiteEngagementHeuristicHostCountHistogram, 1,
+          /*expected_count=*/1);
+      // Check accumulated host count.
+      histograms()->ExpectTotalCount(
+          kSiteEngagementHeuristicAccumulatedHostCountHistogram, 1);
+      histograms()->ExpectBucketCount(
+          kSiteEngagementHeuristicAccumulatedHostCountHistogram, 0,
+          /*expected_count=*/0);
+      histograms()->ExpectBucketCount(
+          kSiteEngagementHeuristicAccumulatedHostCountHistogram, 1,
+          /*expected_count=*/1);
+      // Check enforcement duration. Since the host isn't removed from HFM
+      // enforcement list, no duration should be recorded yet.
+      histograms()->ExpectTotalCount(
+          kSiteEngagementHeuristicEnforcementDurationHistogram, 0);
+    } else {
+      histograms()->ExpectTotalCount(kEventHistogramWithEngagementHeuristic, 0);
+    }
+  } else {
+    histograms()->ExpectTotalCount(kEventHistogram, 1);
+    histograms()->ExpectBucketCount(kEventHistogram,
+                                    Event::kUpgradeNotAttempted, 1);
+    histograms()->ExpectTotalCount(kEventHistogramWithEngagementHeuristic, 0);
+  }
+}
+
 IN_PROC_BROWSER_TEST_P(
     HttpsUpgradesBrowserTest,
     PRE_UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_TypicallySecureUser) {
@@ -2422,14 +2573,20 @@ IN_PROC_BROWSER_TEST_P(
   if (!IsSiteEngagementHeuristicEnabled()) {
     return;
   }
+  // Disable the testing port configuration, as this test doesn't use the
+  // EmbeddedTestServer.
+  HttpsUpgradesInterceptor::SetHttpsPortForTesting(0);
+  HttpsUpgradesInterceptor::SetHttpPortForTesting(0);
+  auto url_loader_interceptor = MakeInterceptorForSiteEngagementHeuristic();
+
   content::WebContents* contents =
       GetBrowser()->tab_strip_model()->GetActiveWebContents();
   auto* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
 
   // Without any policy allowlist, navigate to an HTTP URL. It should show the
   // HFM+SE interstitial.
-  auto http_url = http_server()->GetURL("bad-https.com", "/simple.html");
+  GURL http_url("http://bad-https.com");
-  auto https_url = https_server()->GetURL("bad-https.com", "/simple.html");
+  GURL https_url("https://bad-https.com");
   SetSiteEngagementScore(http_url, kLowSiteEngagementScore);
   SetSiteEngagementScore(https_url, kHighSiteEnagementScore);
 

chrome/browser/ssl/https_upgrades_interceptor.cc

@@ -268,8 +268,8 @@ void HttpsUpgradesInterceptor::MaybeCreateLoader(
       security_interstitials::https_only_mode::HttpInterstitialState>();
   interstitial_state_->enabled_by_pref = http_interstitial_enabled_by_pref_;
   // StatefulSSLHostStateDelegate can be null during tests.
-  if (state && state->IsHttpsEnforcedForHost(
+  if (state && state->IsHttpsEnforcedForUrl(tentative_resource_request.url,
-                   tentative_resource_request.url.host(), storage_partition)) {
+                                            storage_partition)) {
     interstitial_state_->enabled_by_engagement_heuristic = true;
   }
 

chrome/browser/ssl/https_upgrades_navigation_throttle.cc

@@ -87,8 +87,8 @@ HttpsUpgradesNavigationThrottle::MaybeCreateThrottleFor(
   }
 
   // StatefulSSLHostStateDelegate can be null during tests.
-  if (state && state->IsHttpsEnforcedForHost(handle->GetURL().host(),
+  if (state &&
-                                             storage_partition)) {
+      state->IsHttpsEnforcedForUrl(handle->GetURL(), storage_partition)) {
     interstitial_state.enabled_by_engagement_heuristic = true;
   }
 

components/page_info/page_info.cc

@@ -1159,9 +1159,8 @@ void PageInfo::ComputeUIInputs(const GURL& url) {
   // revocation button for HTTP allowlist entries added because HTTPS was
   // enforced by HTTPS-First Mode.
   bool is_https_enforced =
-      delegate->IsHttpsEnforcedForHost(
+      delegate->IsHttpsEnforcedForUrl(
-          url.host(),
+          url, web_contents_->GetPrimaryMainFrame()->GetStoragePartition()) ||
-          web_contents_->GetPrimaryMainFrame()->GetStoragePartition()) ||
       delegate_->IsHttpsFirstModeEnabled();
 
   bool has_warning_bypass_exception =

components/security_interstitials/content/stateful_ssl_host_state_delegate.cc

@@ -390,14 +390,14 @@ void StatefulSSLHostStateDelegate::SetHttpsEnforcementForHost(
   }
 }
 
-bool StatefulSSLHostStateDelegate::IsHttpsEnforcedForHost(
+bool StatefulSSLHostStateDelegate::IsHttpsEnforcedForUrl(
-    const std::string& host,
+    const GURL& url,
     content::StoragePartition* storage_partition) {
   bool is_nondefault_storage =
       !storage_partition ||
       storage_partition != browser_context_->GetDefaultStoragePartition();
-  return https_only_mode_enforcelist_.IsEnforcedForHost(host,
+  return https_only_mode_enforcelist_.IsEnforcedForUrl(url,
-                                                        is_nondefault_storage);
+                                                       is_nondefault_storage);
 }
 
 void StatefulSSLHostStateDelegate::ClearHttpsOnlyModeAllowlist() {

components/security_interstitials/content/stateful_ssl_host_state_delegate.h

@@ -92,8 +92,8 @@ class StatefulSSLHostStateDelegate : public content::SSLHostStateDelegate,
       const std::string& host,
       bool enforced,
       content::StoragePartition* storage_partition) override;
-  bool IsHttpsEnforcedForHost(
+  bool IsHttpsEnforcedForUrl(
-      const std::string& host,
+      const GURL& url,
       content::StoragePartition* storage_partition) override;
 
   // Clears all entries from the HTTP allowlist.

components/security_interstitials/core/https_only_mode_enforcelist.cc

@@ -50,7 +50,7 @@ void HttpsOnlyModeEnforcelist::EnforceForHost(const std::string& host,
     enforce_https_hosts_for_non_default_storage_partitions_.insert(host);
     return;
   }
-  DCHECK(!IsEnforcedForHost(host, is_nondefault_storage));
+  DCHECK(!IsEnforcedForUrl(GURL("http://" + host), is_nondefault_storage));
 
   // We want to count how many HTTPS-enforced hosts accumulate over time, so
   // use a dictionary here.
@@ -75,7 +75,7 @@ void HttpsOnlyModeEnforcelist::UnenforceForHost(const std::string& host,
     enforce_https_hosts_for_non_default_storage_partitions_.erase(host);
     return;
   }
-  DCHECK(IsEnforcedForHost(host, is_nondefault_storage));
+  DCHECK(IsEnforcedForUrl(GURL("http://" + host), is_nondefault_storage));
 
   // We want to count how many HTTPS-enforced hosts accumulate over time, so
   // don't remove the value, just set it to false.
@@ -104,17 +104,21 @@ void HttpsOnlyModeEnforcelist::UnenforceForHost(const std::string& host,
   RecordMetrics(is_nondefault_storage);
 }
 
-bool HttpsOnlyModeEnforcelist::IsEnforcedForHost(
+bool HttpsOnlyModeEnforcelist::IsEnforcedForUrl(
-    const std::string& host,
+    const GURL& url,
     bool is_nondefault_storage) const {
+  // HTTPS-First Mode is never auto-enabled for URLs with non-default ports.
+  if (!url.port().empty()) {
+    return false;
+  }
   if (is_nondefault_storage) {
     return base::Contains(
-        enforce_https_hosts_for_non_default_storage_partitions_, host);
+        enforce_https_hosts_for_non_default_storage_partitions_, url.host());
   }
 
-  GURL url = GetSecureGURLForHost(host);
+  GURL secure_url = GetSecureGURLForHost(url.host());
   const base::Value value = host_content_settings_map_->GetWebsiteSetting(
-      url, url, ContentSettingsType::HTTPS_ENFORCED, nullptr);
+      secure_url, secure_url, ContentSettingsType::HTTPS_ENFORCED, nullptr);
   if (!value.is_dict()) {
     return false;
   }

components/security_interstitials/core/https_only_mode_enforcelist.h

@@ -45,9 +45,10 @@ class HttpsOnlyModeEnforcelist {
   // enforced for this host.
   void UnenforceForHost(const std::string& host, bool is_nondefault_storage);
 
-  // Returns true if host is not allowed to be loaded over HTTP.
+  // Returns true if URL is not allowed to be loaded over HTTP. This check
-  bool IsEnforcedForHost(const std::string& host,
+  // ignores the scheme of the URL: calling it with http:// or https:// will
-                         bool is_nondefault_storage) const;
+  // return the same result.
+  bool IsEnforcedForUrl(const GURL& url, bool is_nondefault_storage) const;
 
   // Revokes all HTTPS enforcements for host.
   void RevokeEnforcements(const std::string& host);

content/public/browser/ssl_host_state_delegate.h

@@ -11,11 +11,13 @@
 #include "base/functional/callback_forward.h"
 #include "net/cert/x509_certificate.h"
 
+class GURL;
+
 namespace content {
 
 class StoragePartition;
 
-// The SSLHostStateDelegate encapulates the host-specific state for SSL errors.
+// The SSLHostStateDelegate encapsulates the host-specific state for SSL errors.
 // For example, SSLHostStateDelegate remembers whether the user has whitelisted
 // a particular broken cert for use with particular host.  We separate this
 // state from the SSLManager because this state is shared across many navigation
@@ -89,9 +91,11 @@ class SSLHostStateDelegate {
       const std::string& host,
       bool enforce,
       StoragePartition* storage_partition) = 0;
-  // Returns whether HTTPS-First Mode is enabled for the given `host`.
+  // Returns whether HTTPS-First Mode is enabled for the given `url`. This check
-  virtual bool IsHttpsEnforcedForHost(const std::string& host,
+  // ignores the scheme of `url`. E.g. http://example.com and
-                                      StoragePartition* storage_partition) = 0;
+  // https://example.com will return the same result.
+  virtual bool IsHttpsEnforcedForUrl(const GURL& url,
+                                     StoragePartition* storage_partition) = 0;
 
   // Returns whether the user has allowed a certificate error exception or
   // HTTP exception for |host|. This does not mean that *all* certificate errors

content/test/mock_ssl_host_state_delegate.cc

@@ -6,6 +6,7 @@
 
 #include "base/containers/contains.h"
 #include "base/functional/callback.h"
+#include "url/gurl.h"
 
 namespace content {
 
@@ -85,10 +86,14 @@ void MockSSLHostStateDelegate::SetHttpsEnforcementForHost(
   }
 }
 
-bool MockSSLHostStateDelegate::IsHttpsEnforcedForHost(
+bool MockSSLHostStateDelegate::IsHttpsEnforcedForUrl(
-    const std::string& host,
+    const GURL& url,
     StoragePartition* storage_partition) {
-  return base::Contains(enforce_https_hosts_, host);
+  // HTTPS-First Mode is never auto-enabled for URLs with non-default ports.
+  if (!url.port().empty()) {
+    return false;
+  }
+  return base::Contains(enforce_https_hosts_, url.host());
 }
 
 void MockSSLHostStateDelegate::RevokeUserAllowExceptions(

content/test/mock_ssl_host_state_delegate.h

@@ -46,8 +46,8 @@ class MockSSLHostStateDelegate : public SSLHostStateDelegate {
   void SetHttpsEnforcementForHost(const std::string& host,
                                   bool enforce,
                                   StoragePartition* storage_partition) override;
-  bool IsHttpsEnforcedForHost(const std::string& host,
+  bool IsHttpsEnforcedForUrl(const GURL& url,
-                              StoragePartition* storage_partition) override;
+                             StoragePartition* storage_partition) override;
 
   void RevokeUserAllowExceptions(const std::string& host) override;