Bug 1891185 - Collect different downgrade time on timer for HTTPS-First telemetry r=freddyb

It does not make sense to collect the "downgrade time" as we currently do it
when a upgrade was caused by a faster http request on a timer. We can not
compare the navigation start timestamp with the current time, as the replacing
http channel has started loading earlier than the current time. So leave out the
case where the downgrade happens because of a faster http request on the timer,
as we already know that the time overhead there is exactly 3s.

Differential Revision: https://phabricator.services.mozilla.com/D207338

dom/security/metrics.yaml

@@ -118,8 +118,8 @@ httpsfirst:
     description: >
       If a HTTPS-First (`dom.security.https_first` enabled) upgrade isn't
       successful, measures the timespan between the navigation start and the
-      downgrade. This is essentially the overhead caused by HTTPS-First if a
-      site does not support HTTPS.
+      downgrade. This does not include the case in which the https request times
+      out and the http request sent after 3s gets a response faster.
     time_unit: millisecond
     bugs:
       - https://bugzilla.mozilla.org/show_bug.cgi?id=1868380
@@ -135,11 +135,11 @@ httpsfirst:
   downgrade_time_schemeless:
     type: timing_distribution
     description: >
-      If a schemeless HTTPS-First (`dom.security.https_first` disabled, but
-      load marked as schemeless) upgrade isn't successful, measures the
-      timespan between the navigation start and the downgrade. This is
-      essentially the overhead caused by HTTPS-First if a site does not support
-      HTTPS.
+      If a schemeless HTTPS-First (`dom.security.https_first` disabled, but load
+      marked as schemeless) upgrade isn't successful, measures the timespan
+      between the navigation start and the downgrade. This does not include the
+      case in which the https request times out and the http request sent after
+      3s gets a response faster.
     time_unit: millisecond
     bugs:
       - https://bugzilla.mozilla.org/show_bug.cgi?id=1868380

dom/security/nsHTTPSOnlyUtils.cpp

@@ -601,35 +601,28 @@ nsHTTPSOnlyUtils::PotentiallyDowngradeHttpsFirstRequest(
     if (navigationStart) {
       mozilla::TimeDuration duration =
           mozilla::TimeStamp::Now() - navigationStart;
+
       bool isPrivateWin =
           loadInfo->GetOriginAttributes().mPrivateBrowsingId > 0;
+      bool isSchemeless =
+          loadInfo->GetWasSchemelessInput() &&
+          !nsHTTPSOnlyUtils::IsHttpsFirstModeEnabled(isPrivateWin);
 
-      if (loadInfo->GetWasSchemelessInput() &&
-          !IsHttpsFirstModeEnabled(isPrivateWin)) {
-        mozilla::glean::httpsfirst::downgraded_schemeless.Add();
-        if (timing) {
-          mozilla::glean::httpsfirst::downgrade_time_schemeless
-              .AccumulateRawDuration(duration);
-        }
-      } else {
-        mozilla::glean::httpsfirst::downgraded.Add();
-        if (timing) {
-          mozilla::glean::httpsfirst::downgrade_time.AccumulateRawDuration(
-              duration);
-        }
-      }
+      using namespace mozilla::glean::httpsfirst;
+      auto downgradedMetric = isSchemeless ? downgraded_schemeless : downgraded;
+      auto downgradedOnTimerMetric =
+          isSchemeless ? downgraded_on_timer_schemeless : downgraded_on_timer;
+      auto downgradeTimeMetric =
+          isSchemeless ? downgrade_time_schemeless : downgrade_time;
 
       nsresult channelStatus;
       channel->GetStatus(&channelStatus);
       if (channelStatus == NS_ERROR_NET_TIMEOUT_EXTERNAL) {
-        if (loadInfo->GetWasSchemelessInput() &&
-            !nsHTTPSOnlyUtils::IsHttpsFirstModeEnabled(isPrivateWin)) {
-          mozilla::glean::httpsfirst::downgraded_on_timer_schemeless
-              .AddToNumerator();
-        } else {
-          mozilla::glean::httpsfirst::downgraded_on_timer.AddToNumerator();
-        }
+        downgradedOnTimerMetric.AddToNumerator();
+      } else {
+        downgradeTimeMetric.AccumulateRawDuration(duration);
       }
+      downgradedMetric.Add();
     }
   }
 

dom/security/test/https-first/browser_httpsfirst.js

@@ -99,11 +99,10 @@ add_task(async function () {
   is(Glean.httpsfirst.downgradedOnTimerSchemeless.testGetValue(), null);
   const downgradeSeconds =
     Glean.httpsfirst.downgradeTime.testGetValue().sum / 1_000_000_000;
-  ok(
-    downgradeSeconds > 2 && downgradeSeconds < 30,
-    `Summed downgrade time should be above 2 and below 30 seconds (is ${downgradeSeconds.toFixed(
-      2
-    )}s)`
+  Assert.less(
+    downgradeSeconds,
+    10,
+    "Summed downgrade time should be below 10 seconds"
   );
   is(null, Glean.httpsfirst.downgradeTimeSchemeless.testGetValue());
 });