Backed out 2 changesets (bug 1703934) as requested on irc by kershaw for causing a possible regression. CLOSED TREE

Backed out changeset 46cd307ba50c (bug 1703934)
Backed out changeset 8a7e66be31e2 (bug 1703934)

modules/libpref/init/StaticPrefList.yaml

@@ -9417,12 +9417,6 @@
   value: 100
   mirror: always
 
-# The global half open sockets allowed for creating a backup connection.
-- name: network.http.http3.parallel_fallback_conn_limit
-  type: RelaxedAtomicUint32
-  value: 32
-  mirror: always
-
 # When true, a http request will be upgraded to https when HTTPS RR is
 # available.
 - name: network.dns.upgrade_with_https_rr

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -3304,8 +3304,7 @@ void nsHttpConnectionMgr::DoSpeculativeConnection(
   } else {
     LOG(
         ("OnMsgSpeculativeConnect Transport "
-         "not created due to existing connection count:%d",
-         parallelSpeculativeConnectLimit));
+         "not created due to existing connection count\n"));
   }
 }
 
@@ -3402,9 +3401,6 @@ void nsHttpConnectionMgr::ExcludeHttp3(const nsHttpConnectionInfo* ci) {
   }
 
   ent->DontReuseHttp3Conn();
-  // Need to cancel the transactions in the pending queue. Otherwise, they'll
-  // stay in the queue forever.
-  ent->CancelAllTransactions(NS_ERROR_NET_RESET);
 }
 
 void nsHttpConnectionMgr::MoveToWildCardConnEntry(

netwerk/protocol/http/nsHttpTransaction.cpp

@@ -481,8 +481,7 @@ static inline void CreateAndStartTimer(nsCOMPtr<nsITimer>& aTimer,
 void nsHttpTransaction::OnPendingQueueInserted() {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
-  // Don't create mHttp3BackupTimer if HTTPS RR is in play.
-  if (mConnInfo->IsHttp3() && !mOrigConnInfo) {
+  if (mConnInfo->IsHttp3() && !mResolver) {
     // Backup timer should only be created once.
     if (!mHttp3BackupTimerCreated) {
       CreateAndStartTimer(mHttp3BackupTimer, this,
@@ -3133,7 +3132,7 @@ nsresult nsHttpTransaction::OnHTTPSRRAvailable(
 
   RefPtr<nsHttpConnectionInfo> newInfo =
       mConnInfo->CloneAndAdoptHTTPSSVCRecord(svcbRecord);
-  bool needFastFallback = newInfo->IsHttp3();
+  bool needFastFallback = !mConnInfo->IsHttp3() && newInfo->IsHttp3();
   if (!gHttpHandler->ConnMgr()->MoveTransToNewConnEntry(this, newInfo)) {
     // MoveTransToNewConnEntry() returning fail means this transaction is
     // not in the connection entry's pending queue. This could happen if
@@ -3143,9 +3142,6 @@ nsresult nsHttpTransaction::OnHTTPSRRAvailable(
     UpdateConnectionInfo(newInfo);
   }
 
-  // In case we already have mHttp3BackupTimer, cancel it.
-  MaybeCancelFallbackTimer();
-
   if (needFastFallback) {
     CreateAndStartTimer(
         mFastFallbackTimer, this,
@@ -3229,21 +3225,6 @@ void nsHttpTransaction::OnBackupConnectionReady(bool aHTTPSRRUsed) {
   }
 }
 
-static void CreateBackupConnection(
-    nsHttpConnectionInfo* aBackupConnInfo, nsIInterfaceRequestor* aCallbacks,
-    uint32_t aCaps, std::function<void(bool)>&& aResultCallback) {
-  RefPtr<SpeculativeTransaction> trans = new SpeculativeTransaction(
-      aBackupConnInfo, aCallbacks, aCaps | NS_HTTP_DISALLOW_HTTP3,
-      std::move(aResultCallback));
-  uint32_t limit =
-      StaticPrefs::network_http_http3_parallel_fallback_conn_limit();
-  if (limit) {
-    trans->SetParallelSpeculativeConnectLimit(limit);
-    trans->SetIgnoreIdle(true);
-  }
-  gHttpHandler->ConnMgr()->DoSpeculativeConnection(trans, false);
-}
-
 void nsHttpTransaction::OnHttp3BackupTimer() {
   LOG(("nsHttpTransaction::OnHttp3BackupTimer [%p]", this));
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
@@ -3261,8 +3242,10 @@ void nsHttpTransaction::OnHttp3BackupTimer() {
     }
   };
 
-  CreateBackupConnection(mBackupConnInfo, mCallbacks, mCaps,
-                         std::move(callback));
+  RefPtr<SpeculativeTransaction> trans = new SpeculativeTransaction(
+      mBackupConnInfo, mCallbacks, mCaps | NS_HTTP_DISALLOW_HTTP3,
+      std::move(callback));
+  gHttpHandler->ConnMgr()->DoSpeculativeConnection(trans, false);
 }
 
 void nsHttpTransaction::OnFastFallbackTimer() {
@@ -3296,8 +3279,10 @@ void nsHttpTransaction::OnFastFallbackTimer() {
     self->OnBackupConnectionReady(true);
   };
 
-  CreateBackupConnection(mBackupConnInfo, mCallbacks, mCaps,
-                         std::move(callback));
+  RefPtr<SpeculativeTransaction> trans = new SpeculativeTransaction(
+      mBackupConnInfo, mCallbacks, mCaps | NS_HTTP_DISALLOW_HTTP3,
+      std::move(callback));
+  gHttpHandler->ConnMgr()->DoSpeculativeConnection(trans, false);
 }
 
 void nsHttpTransaction::HandleFallback(

netwerk/test/unit/test_http3_fast_fallback.js

@@ -61,9 +61,6 @@ registerCleanupFunction(async () => {
   );
   Services.prefs.clearUserPref("network.http.http3.backup_timer_delay");
   Services.prefs.clearUserPref("network.http.speculative-parallel-limit");
-  Services.prefs.clearUserPref(
-    "network.http.http3.parallel_fallback_conn_limit"
-  );
   if (trrServer) {
     await trrServer.stop();
   }
@@ -157,6 +154,34 @@ add_task(async function test_fast_fallback_with_speculative_connection() {
   await fast_fallback_test();
 });
 
+let HTTPObserver = {
+  observeActivity(
+    aChannel,
+    aType,
+    aSubtype,
+    aTimestamp,
+    aSizeData,
+    aStringData
+  ) {
+    aChannel.QueryInterface(Ci.nsIChannel);
+    if (aChannel.URI.spec == `https://foo.example.com:${h2Port}/`) {
+      if (
+        aType == Ci.nsIHttpActivityDistributor.ACTIVITY_TYPE_HTTP_TRANSACTION &&
+        aSubtype ==
+          Ci.nsIHttpActivityDistributor.ACTIVITY_SUBTYPE_REQUEST_HEADER
+      ) {
+        // We need to enable speculative connection again, since the backup
+        // connection is done by using speculative connection.
+        Services.prefs.setIntPref("network.http.speculative-parallel-limit", 6);
+        let observerService = Cc[
+          "@mozilla.org/network/http-activity-distributor;1"
+        ].getService(Ci.nsIHttpActivityDistributor);
+        observerService.removeObserver(HTTPObserver);
+      }
+    }
+  },
+};
+
 // Test the case when speculative connection is disabled. In this case, when the
 // back connection is ready, the http transaction is already activated,
 // but the socket is not ready to write.
@@ -168,6 +193,11 @@ add_task(async function test_fast_fallback_without_speculative_connection() {
   Services.obs.notifyObservers(null, "network:reset-http3-excluded-list");
   Services.prefs.setIntPref("network.http.speculative-parallel-limit", 0);
 
+  let observerService = Cc[
+    "@mozilla.org/network/http-activity-distributor;1"
+  ].getService(Ci.nsIHttpActivityDistributor);
+  observerService.addObserver(HTTPObserver);
+
   await fast_fallback_test();
 
   Services.prefs.clearUserPref(
@@ -600,74 +630,3 @@ add_task(async function testFastfallbackWithoutEchConfig() {
 
   await trrServer.stop();
 });
-
-add_task(async function testH3FallbackWithMultipleTransactions() {
-  trrServer = new TRRServer();
-  await trrServer.start();
-  Services.prefs.setBoolPref("network.dns.upgrade_with_https_rr", true);
-  Services.prefs.setBoolPref("network.dns.use_https_rr_as_altsvc", true);
-  Services.prefs.setBoolPref("network.dns.echconfig.enabled", false);
-
-  Services.prefs.setIntPref("network.trr.mode", 3);
-  Services.prefs.setCharPref(
-    "network.trr.uri",
-    `https://foo.example.com:${trrServer.port}/dns-query`
-  );
-  Services.prefs.setBoolPref("network.http.http3.enabled", true);
-
-  // Disable fast fallback.
-  Services.prefs.setIntPref(
-    "network.http.http3.parallel_fallback_conn_limit",
-    0
-  );
-  Services.prefs.setIntPref("network.http.speculative-parallel-limit", 0);
-
-  await trrServer.registerDoHAnswers("test.multiple_trans.org", "HTTPS", {
-    answers: [
-      {
-        name: "test.multiple_trans.org",
-        ttl: 55,
-        type: "HTTPS",
-        flush: false,
-        data: {
-          priority: 1,
-          name: "test.multiple_trans.org",
-          values: [
-            { key: "alpn", value: "h3-27" },
-            { key: "port", value: h3Port },
-          ],
-        },
-      },
-    ],
-  });
-
-  await trrServer.registerDoHAnswers("test.multiple_trans.org", "A", {
-    answers: [
-      {
-        name: "test.multiple_trans.org",
-        ttl: 55,
-        type: "A",
-        flush: false,
-        data: "127.0.0.1",
-      },
-    ],
-  });
-
-  let promises = [];
-  for (let i = 0; i < 2; ++i) {
-    let chan = makeChan(
-      `https://test.multiple_trans.org:${h2Port}/server-timing`
-    );
-    promises.push(channelOpenPromise(chan));
-  }
-
-  let res = await Promise.all(promises);
-  res.forEach(function(e) {
-    let [req] = e;
-    Assert.equal(req.protocolVersion, "h2");
-    let internal = req.QueryInterface(Ci.nsIHttpChannelInternal);
-    Assert.equal(internal.remotePort, h2Port);
-  });
-
-  await trrServer.stop();
-});