From 35553256966d377ecc88bdbb2ac222b3efdbf577 Mon Sep 17 00:00:00 2001 From: Kershaw Chang Date: Tue, 9 Feb 2021 09:11:45 +0000 Subject: [PATCH] Bug 1686421 - Add probes to understand how HTTPS RR is used r=necko-reviewers,dragana Differential Revision: https://phabricator.services.mozilla.com/D101631 --- netwerk/protocol/http/nsHttpChannel.cpp | 35 ++++++++++++++++++-- netwerk/protocol/http/nsHttpChannel.h | 4 +-- toolkit/components/telemetry/Histograms.json | 27 +++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 5abeeb724972..b175a91599b7 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -627,7 +627,7 @@ nsresult nsHttpChannel::MaybeUseHTTPSRRForUpgrade(bool aShouldUpgrade, this)); StoreWaitHTTPSSVCRecord(false); bool hasHTTPSRR = (mHTTPSSVCRecord.ref() != nullptr); - return ContinueOnBeforeConnect(hasHTTPSRR, aStatus); + return ContinueOnBeforeConnect(hasHTTPSRR, aStatus, hasHTTPSRR); } LOG(("nsHttpChannel::MaybeUseHTTPSRRForUpgrade [%p] wait for HTTPS RR", @@ -637,7 +637,8 @@ nsresult nsHttpChannel::MaybeUseHTTPSRRForUpgrade(bool aShouldUpgrade, } nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade, - nsresult aStatus) { + nsresult aStatus, + bool aUpgradeWithHTTPSRR) { LOG( ("nsHttpChannel::ContinueOnBeforeConnect " "[this=%p aShouldUpgrade=%d rv=%" PRIx32 "]\n", @@ -650,6 +651,8 @@ nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade, } if (aShouldUpgrade) { + Telemetry::Accumulate(Telemetry::HTTPS_UPGRADE_WITH_HTTPS_RR, + aUpgradeWithHTTPSRR); return AsyncCall(&nsHttpChannel::HandleAsyncRedirectChannelToHttps); } @@ -7563,6 +7566,26 @@ nsresult nsHttpChannel::ContinueOnStartRequest4(nsresult result) { return CallOnStartRequest(); } +static void ReportHTTPSRRTelemetry( + const Maybe>& aMaybeRecord) { + bool hasHTTPSRR = aMaybeRecord && (aMaybeRecord.ref() != nullptr); + Telemetry::Accumulate(Telemetry::HTTPS_RR_PRESENTED, hasHTTPSRR); + if (!hasHTTPSRR) { + return; + } + + const nsCOMPtr& record = aMaybeRecord.ref(); + nsCOMPtr svcbRecord; + if (NS_SUCCEEDED(record->GetServiceModeRecord(false, false, + getter_AddRefs(svcbRecord)))) { + MOZ_ASSERT(svcbRecord); + + Maybe> alpn = svcbRecord->GetAlpn(); + bool isHttp3 = alpn ? Get<1>(*alpn) : false; + Telemetry::Accumulate(Telemetry::HTTPS_RR_WITH_HTTP3_PRESENTED, isHttp3); + } +} + NS_IMETHODIMP nsHttpChannel::OnStopRequest(nsIRequest* request, nsresult status) { AUTO_PROFILER_LABEL("nsHttpChannel::OnStopRequest", NETWORK); @@ -7580,6 +7603,12 @@ nsHttpChannel::OnStopRequest(nsIRequest* request, nsresult status) { return NS_OK; } + // It's possible that LoadUseHTTPSSVC() is false, but we already have + // mHTTPSSVCRecord. + if (LoadUseHTTPSSVC() || mHTTPSSVCRecord) { + ReportHTTPSRRTelemetry(mHTTPSSVCRecord); + } + // If this load failed because of a security error, it may be because we // are in a captive portal - trigger an async check to make sure. int32_t nsprError = -1 * NS_ERROR_GET_CODE(status); @@ -9000,7 +9029,7 @@ void nsHttpChannel::OnHTTPSRRAvailable(nsIDNSHTTPSSVCRecord* aRecord) { MOZ_ASSERT(mURI->SchemeIs("http")); StoreWaitHTTPSSVCRecord(false); - nsresult rv = ContinueOnBeforeConnect(!!httprr, mStatus); + nsresult rv = ContinueOnBeforeConnect(!!httprr, mStatus, !!httprr); if (NS_FAILED(rv)) { CloseCacheEntry(false); Unused << AsyncAbort(rv); diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index 9db74cbe24bb..ba761745931f 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -332,8 +332,8 @@ class nsHttpChannel final : public HttpBaseChannel, [[nodiscard]] nsresult PrepareToConnect(); void HandleOnBeforeConnect(); [[nodiscard]] nsresult OnBeforeConnect(); - [[nodiscard]] nsresult ContinueOnBeforeConnect(bool aShouldUpgrade, - nsresult aStatus); + [[nodiscard]] nsresult ContinueOnBeforeConnect( + bool aShouldUpgrade, nsresult aStatus, bool aUpgradeWithHTTPSRR = false); nsresult MaybeUseHTTPSRRForUpgrade(bool aShouldUpgrade, nsresult aStatus); void OnHTTPSRRAvailable(nsIDNSHTTPSSVCRecord* aRecord); void OnBeforeConnectContinue(); diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index b025c1c0f7d8..6d7cd2cb91a5 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -16455,5 +16455,32 @@ "description": "TLS handshake with and without EchConfig success rate.", "alert_emails": ["necko@mozilla.com", "kershaw@mozilla.com"], "bug_numbers": [1682555] + }, + "HTTPS_RR_PRESENTED": { + "record_in_processes": ["main"], + "products": ["firefox"], + "alert_emails": ["necko@mozilla.com", "kershaw@mozilla.com"], + "bug_numbers": [1686421], + "expires_in_version": "never", + "kind": "boolean", + "description": "HTTPS RR is presented or not." + }, + "HTTPS_RR_WITH_HTTP3_PRESENTED": { + "record_in_processes": ["main"], + "products": ["firefox"], + "alert_emails": ["necko@mozilla.com", "kershaw@mozilla.com"], + "bug_numbers": [1686421], + "expires_in_version": "never", + "kind": "boolean", + "description": "Whether an HTTPS RR indicates that HTTP3 should be used." + }, + "HTTPS_UPGRADE_WITH_HTTPS_RR": { + "record_in_processes": ["main"], + "products": ["firefox"], + "alert_emails": ["necko@mozilla.com", "kershaw@mozilla.com"], + "bug_numbers": [1686421], + "expires_in_version": "never", + "kind": "boolean", + "description": "Whether an HTTP request gets upgraded to HTTPS because HTTPS RR is presented" } }