diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index b84d2177d3a1..4851148d889b 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -9937,8 +9937,6 @@ nsIPrincipal* nsDocShell::GetInheritedPrincipal( } if (nsCOMPtr timedChannel = do_QueryInterface(channel)) { - timedChannel->SetTimingEnabled(true); - nsString initiatorType; switch (aLoadInfo->InternalContentPolicyType()) { case nsIContentPolicy::TYPE_INTERNAL_EMBED: diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index 169472b9c7e9..430c1e210d34 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -2476,12 +2476,6 @@ void nsPIDOMWindowInner::CreatePerformanceObjectIfNeeded() { } RefPtr timing = mDoc->GetNavigationTiming(); nsCOMPtr timedChannel(do_QueryInterface(mDoc->GetChannel())); - bool timingEnabled = false; - if (!timedChannel || - !NS_SUCCEEDED(timedChannel->GetTimingEnabled(&timingEnabled)) || - !timingEnabled) { - timedChannel = nullptr; - } if (timing) { mPerformance = Performance::CreateForMainThread(this, mDoc->NodePrincipal(), timing, timedChannel); diff --git a/dom/ipc/DOMTypes.ipdlh b/dom/ipc/DOMTypes.ipdlh index cdd1f8760f93..6ba958475e4e 100644 --- a/dom/ipc/DOMTypes.ipdlh +++ b/dom/ipc/DOMTypes.ipdlh @@ -234,7 +234,6 @@ struct DocShellLoadStateInit struct TimedChannelInfo { - bool timingEnabled; int8_t redirectCount; int8_t internalRedirectCount; TimeStamp asyncOpen; diff --git a/netwerk/base/nsITimedChannel.idl b/netwerk/base/nsITimedChannel.idl index 8e6d64649a78..20eba78c4b8e 100644 --- a/netwerk/base/nsITimedChannel.idl +++ b/netwerk/base/nsITimedChannel.idl @@ -28,11 +28,6 @@ interface nsIServerTiming : nsISupports { // All properties return zero if the value is not available [scriptable, builtinclass, uuid(ca63784d-959c-4c3a-9a59-234a2a520de0)] interface nsITimedChannel : nsISupports { - // Set this attribute to true to enable collection of timing data. - // channelCreationTime will be available even with this attribute set to - // false. - attribute boolean timingEnabled; - // The number of redirects attribute uint8_t redirectCount; attribute uint8_t internalRedirectCount; diff --git a/netwerk/base/nsLoadGroup.cpp b/netwerk/base/nsLoadGroup.cpp index 21b28a80f021..a494c03e5407 100644 --- a/netwerk/base/nsLoadGroup.cpp +++ b/netwerk/base/nsLoadGroup.cpp @@ -415,7 +415,6 @@ nsLoadGroup::SetDefaultLoadRequest(nsIRequest* aRequest) { mDefaultLoadIsTimed = timedChannel != nullptr; if (mDefaultLoadIsTimed) { timedChannel->GetChannelCreation(&mDefaultRequestCreationTime); - timedChannel->SetTimingEnabled(true); } } // Else, do not change the group's load flags (see bug 95981) @@ -470,9 +469,6 @@ nsLoadGroup::AddRequest(nsIRequest* request, nsISupports* ctxt) { if (mPriority != 0) RescheduleRequest(request, mPriority); - nsCOMPtr timedChannel = do_QueryInterface(request); - if (timedChannel) timedChannel->SetTimingEnabled(true); - bool foreground = !(flags & nsIRequest::LOAD_BACKGROUND); if (foreground) { // Update the count of foreground URIs.. @@ -829,9 +825,6 @@ void nsLoadGroup::TelemetryReport() { void nsLoadGroup::TelemetryReportChannel(nsITimedChannel* aTimedChannel, bool aDefaultRequest) { nsresult rv; - bool timingEnabled; - rv = aTimedChannel->GetTimingEnabled(&timingEnabled); - if (NS_FAILED(rv) || !timingEnabled) return; TimeStamp asyncOpen; rv = aTimedChannel->GetAsyncOpen(&asyncOpen); diff --git a/netwerk/dns/TRR.cpp b/netwerk/dns/TRR.cpp index d6714b61fa82..ed8d5dd7e4e9 100644 --- a/netwerk/dns/TRR.cpp +++ b/netwerk/dns/TRR.cpp @@ -440,11 +440,6 @@ nsresult TRR::SetupTRRServiceChannelInternal(nsIHttpChannel* aChannel, LOG(("TRR::SetupTRRServiceChannelInternal: couldn't set content-type!\n")); } - nsCOMPtr timedChan(do_QueryInterface(httpChannel)); - if (timedChan) { - timedChan->SetTimingEnabled(true); - } - return NS_OK; } diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 8aa3d7fbbc0d..7c34cb421f95 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -4711,7 +4711,6 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod, do_QueryInterface(static_cast(this))); if (oldTimedChannel) { config.timedChannelInfo = Some(dom::TimedChannelInfo()); - config.timedChannelInfo->timingEnabled() = LoadTimingEnabled(); config.timedChannelInfo->redirectCount() = mRedirectCount; config.timedChannelInfo->internalRedirectCount() = mInternalRedirectCount; config.timedChannelInfo->asyncOpen() = mAsyncOpenTime; @@ -4807,8 +4806,6 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod, // Transfer the timing data (if we are dealing with an nsITimedChannel). nsCOMPtr newTimedChannel(do_QueryInterface(newChannel)); if (config.timedChannelInfo && newTimedChannel) { - newTimedChannel->SetTimingEnabled(config.timedChannelInfo->timingEnabled()); - // If we're an internal redirect, or a document channel replacement, // then we shouldn't record any new timing for this and just copy // over the existing values. @@ -5376,18 +5373,6 @@ HttpBaseChannel::SetMatchedTrackingInfo( // HttpBaseChannel::nsITimedChannel //----------------------------------------------------------------------------- -NS_IMETHODIMP -HttpBaseChannel::SetTimingEnabled(bool enabled) { - StoreTimingEnabled(enabled); - return NS_OK; -} - -NS_IMETHODIMP -HttpBaseChannel::GetTimingEnabled(bool* _retval) { - *_retval = LoadTimingEnabled(); - return NS_OK; -} - NS_IMETHODIMP HttpBaseChannel::GetChannelCreation(TimeStamp* _retval) { *_retval = mChannelCreationTimestamp; @@ -5860,12 +5845,6 @@ IMPL_TIMING_ATTR(TransactionPending) #undef IMPL_TIMING_ATTR void HttpBaseChannel::MaybeReportTimingData() { - // If performance timing is disabled, there is no need for the Performance - // object anymore. - if (!LoadTimingEnabled()) { - return; - } - // There is no point in continuing, since the performance object in the parent // isn't the same as the one in the child which will be reporting resource // performance. diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index b6b482b15900..800f3adc825d 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -924,8 +924,6 @@ class HttpBaseChannel : public nsHashPropertyBag, (uint32_t, UploadStreamHasHeaders, 1), (uint32_t, ChannelIsForDownload, 1), (uint32_t, TracingEnabled, 1), - // True if timing collection is enabled - (uint32_t, TimingEnabled, 1), (uint32_t, ReportTiming, 1), (uint32_t, AllowSpdy, 1), (uint32_t, AllowHttp3, 1), diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index 406287a93085..84ad393e03e6 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -530,7 +530,6 @@ bool HttpChannelParent::DoAsyncOpen( if (httpChannelImpl) { httpChannelImpl->SetWarningReporter(this); } - httpChannel->SetTimingEnabled(true); if (mPBOverride != kPBOverride_Unset) { httpChannel->SetPrivate(mPBOverride == kPBOverride_Private); } diff --git a/netwerk/protocol/http/InterceptedHttpChannel.cpp b/netwerk/protocol/http/InterceptedHttpChannel.cpp index 6ce60c336615..b7c129dcc21c 100644 --- a/netwerk/protocol/http/InterceptedHttpChannel.cpp +++ b/netwerk/protocol/http/InterceptedHttpChannel.cpp @@ -125,9 +125,7 @@ void InterceptedHttpChannel::AsyncOpenInternal() { // We should have pre-set the AsyncOpen time based on the original channel if // timings are enabled. - if (LoadTimingEnabled()) { - MOZ_DIAGNOSTIC_ASSERT(!mAsyncOpenTime.IsNull()); - } + MOZ_DIAGNOSTIC_ASSERT(!mAsyncOpenTime.IsNull()); StoreIsPending(true); StoreResponseCouldBeSynthesized(true); diff --git a/netwerk/protocol/http/NullHttpChannel.cpp b/netwerk/protocol/http/NullHttpChannel.cpp index 735ee7dc8af0..69c39a9822c8 100644 --- a/netwerk/protocol/http/NullHttpChannel.cpp +++ b/netwerk/protocol/http/NullHttpChannel.cpp @@ -496,18 +496,6 @@ NullHttpChannel::GetIsDocument(bool* aIsDocument) { // NullHttpChannel::nsITimedChannel //----------------------------------------------------------------------------- -NS_IMETHODIMP -NullHttpChannel::GetTimingEnabled(bool* aTimingEnabled) { - // We don't want to report timing for null channels. - *aTimingEnabled = false; - return NS_OK; -} - -NS_IMETHODIMP -NullHttpChannel::SetTimingEnabled(bool aTimingEnabled) { - return NS_ERROR_NOT_IMPLEMENTED; -} - NS_IMETHODIMP NullHttpChannel::GetRedirectCount(uint8_t* aRedirectCount) { return NS_ERROR_NOT_IMPLEMENTED; diff --git a/netwerk/protocol/http/TRRServiceChannel.cpp b/netwerk/protocol/http/TRRServiceChannel.cpp index 80363f7dc27b..37783d104ea5 100644 --- a/netwerk/protocol/http/TRRServiceChannel.cpp +++ b/netwerk/protocol/http/TRRServiceChannel.cpp @@ -444,10 +444,6 @@ nsresult TRRServiceChannel::BeginConnect() { mConnectionInfo->SetNoSpdy(true); } - // If TimingEnabled flag is not set after OnModifyRequest() then - // clear the already recorded AsyncOpen value for consistency. - if (!LoadTimingEnabled()) mAsyncOpenTime = TimeStamp(); - // if this somehow fails we can go on without it Unused << gHttpHandler->AddConnectionHeader(&mRequestHead, mCaps); @@ -651,8 +647,6 @@ nsresult TRRServiceChannel::SetupTransaction() { // See bug #466080. Transfer LOAD_ANONYMOUS flag to socket-layer. if (mLoadFlags & LOAD_ANONYMOUS) mCaps |= NS_HTTP_LOAD_ANONYMOUS; - if (LoadTimingEnabled()) mCaps |= NS_HTTP_TIMING_ENABLED; - nsCOMPtr pushListener; NS_QueryNotificationCallbacks(mCallbacks, mLoadGroup, NS_GET_IID(nsIHttpPushListener), @@ -776,9 +770,8 @@ void TRRServiceChannel::MaybeStartDNSPrefetch() { this, mCaps & NS_HTTP_REFRESH_DNS ? ", refresh requested" : "")); OriginAttributes originAttributes; - mDNSPrefetch = - new nsDNSPrefetch(mURI, originAttributes, nsIRequest::GetTRRMode(), this, - LoadTimingEnabled()); + mDNSPrefetch = new nsDNSPrefetch(mURI, originAttributes, + nsIRequest::GetTRRMode(), this, true); nsIDNSService::DNSFlags dnsFlags = nsIDNSService::RESOLVE_DEFAULT_FLAGS; if (mCaps & NS_HTTP_REFRESH_DNS) { dnsFlags |= nsIDNSService::RESOLVE_BYPASS_CACHE; diff --git a/netwerk/protocol/http/nsHttp.h b/netwerk/protocol/http/nsHttp.h index 9270d60f5d3d..c35bfc925b93 100644 --- a/netwerk/protocol/http/nsHttp.h +++ b/netwerk/protocol/http/nsHttp.h @@ -105,9 +105,6 @@ constexpr nsLiteralCString kHttp3Versions[] = {"h3-29"_ns, "h3-30"_ns, // to the server (see bug #466080), but is may also be used for other things #define NS_HTTP_LOAD_ANONYMOUS (1 << 4) -// a transaction with this caps flag keeps timing information -#define NS_HTTP_TIMING_ENABLED (1 << 5) - // a transaction with this flag blocks the initiation of other transactons // in the same load group until it is complete #define NS_HTTP_LOAD_AS_BLOCKING (1 << 6) diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index c176824dfb48..a109a8096f1a 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -1721,8 +1721,6 @@ nsresult nsHttpChannel::SetupChannelForTransaction() { // See bug #466080. Transfer LOAD_ANONYMOUS flag to socket-layer. if (mLoadFlags & LOAD_ANONYMOUS) mCaps |= NS_HTTP_LOAD_ANONYMOUS; - if (LoadTimingEnabled()) mCaps |= NS_HTTP_TIMING_ENABLED; - if (mUpgradeProtocolCallback) { rv = mRequestHead.SetHeader(nsHttp::Upgrade, mUpgradeProtocol, false); MOZ_ASSERT(NS_SUCCEEDED(rv)); @@ -5263,8 +5261,6 @@ nsresult nsHttpChannel::ReadFromCache(void) { rv = mCachePump->AsyncRead(this); if (NS_FAILED(rv)) return rv; - if (LoadTimingEnabled()) mCacheReadStart = TimeStamp::Now(); - uint32_t suspendCount = mSuspendCount; if (LoadAsyncResumePending()) { LOG( @@ -7179,9 +7175,8 @@ void nsHttpChannel::MaybeStartDNSPrefetch() { StoragePrincipalHelper::GetOriginAttributesForNetworkState( this, originAttributes); - mDNSPrefetch = - new nsDNSPrefetch(mURI, originAttributes, nsIRequest::GetTRRMode(), - this, LoadTimingEnabled()); + mDNSPrefetch = new nsDNSPrefetch(mURI, originAttributes, + nsIRequest::GetTRRMode(), this, true); nsIDNSService::DNSFlags dnsFlags = nsIDNSService::RESOLVE_DEFAULT_FLAGS; if (mCaps & NS_HTTP_REFRESH_DNS) { dnsFlags |= nsIDNSService::RESOLVE_BYPASS_CACHE; @@ -8318,7 +8313,7 @@ nsHttpChannel::OnStopRequest(nsIRequest* request, nsresult status) { gIOService->RecheckCaptivePortal(); } - if (LoadTimingEnabled() && request == mCachePump) { + if (request == mCachePump) { mCacheReadEnd = TimeStamp::Now(); } diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index 58815d3222f4..28cb94072e6f 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -605,7 +605,7 @@ void nsHttpTransaction::OnTransportStatus(nsITransport* transport, // for domainLookupStart/End and connectStart/End // If we are using a persistent connection they will remain null, // and the correct value will be returned in Performance. - if (TimingEnabled() && GetRequestStart().IsNull()) { + if (GetRequestStart().IsNull()) { if (status == NS_NET_STATUS_RESOLVING_HOST) { SetDomainLookupStart(TimeStamp::Now(), true); } else if (status == NS_NET_STATUS_RESOLVED_HOST) { @@ -816,10 +816,8 @@ nsresult nsHttpTransaction::WritePipeSegment(nsIOutputStream* stream, if (trans->mTransactionDone) return NS_BASE_STREAM_CLOSED; // stop iterating - if (trans->TimingEnabled()) { - // Set the timestamp to Now(), only if it null - trans->SetResponseStart(TimeStamp::Now(), true); - } + // Set the timestamp to Now(), only if it null + trans->SetResponseStart(TimeStamp::Now(), true); // Bug 1153929 - add checks to fix windows crash MOZ_ASSERT(trans->mWriter); @@ -1675,46 +1673,45 @@ void nsHttpTransaction::Close(nsresult reason) { // mTimings.responseEnd is normally recorded based on the end of a // HTTP delimiter such as chunked-encodings or content-length. However, // EOF or an error still require an end time be recorded. - if (TimingEnabled()) { - const TimingStruct timings = Timings(); - if (timings.responseEnd.IsNull() && !timings.responseStart.IsNull()) { - SetResponseEnd(TimeStamp::Now()); - } - // Accumulate download throughput telemetry - if ((mContentRead > TELEMETRY_REQUEST_SIZE_10M) && - !timings.requestStart.IsNull() && !timings.responseEnd.IsNull()) { - TimeDuration elapsed = timings.responseEnd - timings.requestStart; - double megabits = static_cast(mContentRead) * 8.0 / 1000000.0; - uint32_t mpbs = static_cast(megabits / elapsed.ToSeconds()); + const TimingStruct timings = Timings(); + if (timings.responseEnd.IsNull() && !timings.responseStart.IsNull()) { + SetResponseEnd(TimeStamp::Now()); + } - switch (mHttpVersion) { - case HttpVersion::v1_0: - case HttpVersion::v1_1: - glean::networking::http_1_download_throughput.AccumulateSingleSample( - mpbs); - break; - case HttpVersion::v2_0: - glean::networking::http_2_download_throughput.AccumulateSingleSample( - mpbs); - break; - case HttpVersion::v3_0: - glean::networking::http_3_download_throughput.AccumulateSingleSample( - mpbs); - if (mContentRead <= TELEMETRY_REQUEST_SIZE_50M) { - glean::networking::http_3_download_throughput_10_50 - .AccumulateSingleSample(mpbs); - } else if (mContentRead <= TELEMETRY_REQUEST_SIZE_100M) { - glean::networking::http_3_download_throughput_50_100 - .AccumulateSingleSample(mpbs); - } else { - glean::networking::http_3_download_throughput_100 - .AccumulateSingleSample(mpbs); - } - break; - default: - break; - } + // Accumulate download throughput telemetry + if ((mContentRead > TELEMETRY_REQUEST_SIZE_10M) && + !timings.requestStart.IsNull() && !timings.responseEnd.IsNull()) { + TimeDuration elapsed = timings.responseEnd - timings.requestStart; + double megabits = static_cast(mContentRead) * 8.0 / 1000000.0; + uint32_t mpbs = static_cast(megabits / elapsed.ToSeconds()); + + switch (mHttpVersion) { + case HttpVersion::v1_0: + case HttpVersion::v1_1: + glean::networking::http_1_download_throughput.AccumulateSingleSample( + mpbs); + break; + case HttpVersion::v2_0: + glean::networking::http_2_download_throughput.AccumulateSingleSample( + mpbs); + break; + case HttpVersion::v3_0: + glean::networking::http_3_download_throughput.AccumulateSingleSample( + mpbs); + if (mContentRead <= TELEMETRY_REQUEST_SIZE_50M) { + glean::networking::http_3_download_throughput_10_50 + .AccumulateSingleSample(mpbs); + } else if (mContentRead <= TELEMETRY_REQUEST_SIZE_100M) { + glean::networking::http_3_download_throughput_50_100 + .AccumulateSingleSample(mpbs); + } else { + glean::networking::http_3_download_throughput_100 + .AccumulateSingleSample(mpbs); + } + break; + default: + break; } } @@ -2506,9 +2503,7 @@ nsresult nsHttpTransaction::HandleContent(char* buf, uint32_t count, } ReleaseBlockingTransaction(); - if (TimingEnabled()) { - SetResponseEnd(TimeStamp::Now()); - } + SetResponseEnd(TimeStamp::Now()); // report the entire response has arrived gHttpHandler->ObserveHttpActivityWithArgs( diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h index 4bfab42a70a9..465a7ff0fab4 100644 --- a/netwerk/protocol/http/nsHttpTransaction.h +++ b/netwerk/protocol/http/nsHttpTransaction.h @@ -223,8 +223,6 @@ class nsHttpTransaction final : public nsAHttpTransaction, [[nodiscard]] static nsresult WritePipeSegment(nsIOutputStream*, void*, char*, uint32_t, uint32_t, uint32_t*); - bool TimingEnabled() const { return mCaps & NS_HTTP_TIMING_ENABLED; } - bool ResponseTimeoutEnabled() const final; void ReuseConnectionOnRestartOK(bool reuseOk) override { diff --git a/netwerk/test/unit/test_tls_handshake_timing.js b/netwerk/test/unit/test_tls_handshake_timing.js index f5b24faeffce..298ea7c7863f 100644 --- a/netwerk/test/unit/test_tls_handshake_timing.js +++ b/netwerk/test/unit/test_tls_handshake_timing.js @@ -67,7 +67,6 @@ async function do_test_timing(url) { let chan = makeChan(url); let timedChannel = chan.QueryInterface(Ci.nsITimedChannel); - timedChannel.timingEnabled = true; await channelOpenPromise(chan); info(`secureConnectionStartTime=${timedChannel.secureConnectionStartTime}`); info(`connectEndTime=${timedChannel.connectEndTime}`);