From cea51c8354155cda3e3b8c7b58013fdfdcb436d2 Mon Sep 17 00:00:00 2001 From: Kershaw Chang Date: Wed, 21 Apr 2021 08:59:20 +0000 Subject: [PATCH] Bug 1703934 - P2: Use another parallel limit for backup speculative connection, r=dragana,necko-reviewers Differential Revision: https://phabricator.services.mozilla.com/D112349 --- modules/libpref/init/StaticPrefList.yaml | 6 +++ netwerk/protocol/http/nsHttpConnectionMgr.cpp | 3 +- netwerk/protocol/http/nsHttpTransaction.cpp | 27 +++++++---- netwerk/test/unit/test_http3_fast_fallback.js | 45 ++++--------------- 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 5bec125adff6..6f26c542ade7 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -9417,6 +9417,12 @@ 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 diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 43f13843d125..416bd9228d3e 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -3304,7 +3304,8 @@ void nsHttpConnectionMgr::DoSpeculativeConnection( } else { LOG( ("OnMsgSpeculativeConnect Transport " - "not created due to existing connection count\n")); + "not created due to existing connection count:%d", + parallelSpeculativeConnectLimit)); } } diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index 11d6eca528b4..0142d8f50fd7 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -3229,6 +3229,21 @@ void nsHttpTransaction::OnBackupConnectionReady(bool aHTTPSRRUsed) { } } +static void CreateBackupConnection( + nsHttpConnectionInfo* aBackupConnInfo, nsIInterfaceRequestor* aCallbacks, + uint32_t aCaps, std::function&& aResultCallback) { + RefPtr 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"); @@ -3246,10 +3261,8 @@ void nsHttpTransaction::OnHttp3BackupTimer() { } }; - RefPtr trans = new SpeculativeTransaction( - mBackupConnInfo, mCallbacks, mCaps | NS_HTTP_DISALLOW_HTTP3, - std::move(callback)); - gHttpHandler->ConnMgr()->DoSpeculativeConnection(trans, false); + CreateBackupConnection(mBackupConnInfo, mCallbacks, mCaps, + std::move(callback)); } void nsHttpTransaction::OnFastFallbackTimer() { @@ -3283,10 +3296,8 @@ void nsHttpTransaction::OnFastFallbackTimer() { self->OnBackupConnectionReady(true); }; - RefPtr trans = new SpeculativeTransaction( - mBackupConnInfo, mCallbacks, mCaps | NS_HTTP_DISALLOW_HTTP3, - std::move(callback)); - gHttpHandler->ConnMgr()->DoSpeculativeConnection(trans, false); + CreateBackupConnection(mBackupConnInfo, mCallbacks, mCaps, + std::move(callback)); } void nsHttpTransaction::HandleFallback( diff --git a/netwerk/test/unit/test_http3_fast_fallback.js b/netwerk/test/unit/test_http3_fast_fallback.js index 41ea72bf9d92..241f7f1ebb2d 100644 --- a/netwerk/test/unit/test_http3_fast_fallback.js +++ b/netwerk/test/unit/test_http3_fast_fallback.js @@ -61,6 +61,9 @@ 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(); } @@ -154,34 +157,6 @@ 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. @@ -193,11 +168,6 @@ 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( @@ -647,9 +617,10 @@ add_task(async function testH3FallbackWithMultipleTransactions() { // Disable fast fallback. Services.prefs.setIntPref( - "network.dns.httpssvc.http3_fast_fallback_timeout", + "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: [ @@ -684,8 +655,10 @@ add_task(async function testH3FallbackWithMultipleTransactions() { 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 chan = makeChan( + `https://test.multiple_trans.org:${h2Port}/server-timing` + ); + promises.push(channelOpenPromise(chan)); } let res = await Promise.all(promises);