Bug 1931514 - Remove nsITimedChannel.timingEnabled r=necko-reviewers,kershaw

Timing should always be enabled.

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

docshell/base/nsDocShell.cpp

@@ -9937,8 +9937,6 @@ nsIPrincipal* nsDocShell::GetInheritedPrincipal(
   }
 
   if (nsCOMPtr<nsITimedChannel> timedChannel = do_QueryInterface(channel)) {
-    timedChannel->SetTimingEnabled(true);
-
     nsString initiatorType;
     switch (aLoadInfo->InternalContentPolicyType()) {
       case nsIContentPolicy::TYPE_INTERNAL_EMBED:

dom/base/nsGlobalWindowInner.cpp

@@ -2476,12 +2476,6 @@ void nsPIDOMWindowInner::CreatePerformanceObjectIfNeeded() {
   }
   RefPtr<nsDOMNavigationTiming> timing = mDoc->GetNavigationTiming();
   nsCOMPtr<nsITimedChannel> 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);

dom/ipc/DOMTypes.ipdlh

@@ -234,7 +234,6 @@ struct DocShellLoadStateInit
 
 struct TimedChannelInfo
 {
-  bool timingEnabled;
   int8_t redirectCount;
   int8_t internalRedirectCount;
   TimeStamp asyncOpen;

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;

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<nsITimedChannel> 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);

netwerk/dns/TRR.cpp

@@ -440,11 +440,6 @@ nsresult TRR::SetupTRRServiceChannelInternal(nsIHttpChannel* aChannel,
     LOG(("TRR::SetupTRRServiceChannelInternal: couldn't set content-type!\n"));
   }
 
-  nsCOMPtr<nsITimedChannel> timedChan(do_QueryInterface(httpChannel));
-  if (timedChan) {
-    timedChan->SetTimingEnabled(true);
-  }
-
   return NS_OK;
 }
 

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -4711,7 +4711,6 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
       do_QueryInterface(static_cast<nsIHttpChannel*>(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<nsITimedChannel> 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.

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),

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);
   }

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);

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;

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<nsIHttpPushListener> 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 =
+  mDNSPrefetch = new nsDNSPrefetch(mURI, originAttributes,
-      new nsDNSPrefetch(mURI, originAttributes, nsIRequest::GetTRRMode(), this,
+                                   nsIRequest::GetTRRMode(), this, true);
-                        LoadTimingEnabled());
   nsIDNSService::DNSFlags dnsFlags = nsIDNSService::RESOLVE_DEFAULT_FLAGS;
   if (mCaps & NS_HTTP_REFRESH_DNS) {
     dnsFlags |= nsIDNSService::RESOLVE_BYPASS_CACHE;

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)

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 =
+    mDNSPrefetch = new nsDNSPrefetch(mURI, originAttributes,
-        new nsDNSPrefetch(mURI, originAttributes, nsIRequest::GetTRRMode(),
+                                     nsIRequest::GetTRRMode(), this, true);
-                          this, LoadTimingEnabled());
     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();
   }
 

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
-    // Set the timestamp to Now(), only if it null
+  trans->SetResponseStart(TimeStamp::Now(), true);
-    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
+  const TimingStruct timings = Timings();
-    if ((mContentRead > TELEMETRY_REQUEST_SIZE_10M) &&
+  if (timings.responseEnd.IsNull() && !timings.responseStart.IsNull()) {
-        !timings.requestStart.IsNull() && !timings.responseEnd.IsNull()) {
+    SetResponseEnd(TimeStamp::Now());
-      TimeDuration elapsed = timings.responseEnd - timings.requestStart;
+  }
-      double megabits = static_cast<double>(mContentRead) * 8.0 / 1000000.0;
-      uint32_t mpbs = static_cast<uint32_t>(megabits / elapsed.ToSeconds());
 
-      switch (mHttpVersion) {
+  // Accumulate download throughput telemetry
-        case HttpVersion::v1_0:
+  if ((mContentRead > TELEMETRY_REQUEST_SIZE_10M) &&
-        case HttpVersion::v1_1:
+      !timings.requestStart.IsNull() && !timings.responseEnd.IsNull()) {
-          glean::networking::http_1_download_throughput.AccumulateSingleSample(
+    TimeDuration elapsed = timings.responseEnd - timings.requestStart;
-              mpbs);
+    double megabits = static_cast<double>(mContentRead) * 8.0 / 1000000.0;
-          break;
+    uint32_t mpbs = static_cast<uint32_t>(megabits / elapsed.ToSeconds());
-        case HttpVersion::v2_0:
+
-          glean::networking::http_2_download_throughput.AccumulateSingleSample(
+    switch (mHttpVersion) {
-              mpbs);
+      case HttpVersion::v1_0:
-          break;
+      case HttpVersion::v1_1:
-        case HttpVersion::v3_0:
+        glean::networking::http_1_download_throughput.AccumulateSingleSample(
-          glean::networking::http_3_download_throughput.AccumulateSingleSample(
+            mpbs);
-              mpbs);
+        break;
-          if (mContentRead <= TELEMETRY_REQUEST_SIZE_50M) {
+      case HttpVersion::v2_0:
-            glean::networking::http_3_download_throughput_10_50
+        glean::networking::http_2_download_throughput.AccumulateSingleSample(
-                .AccumulateSingleSample(mpbs);
+            mpbs);
-          } else if (mContentRead <= TELEMETRY_REQUEST_SIZE_100M) {
+        break;
-            glean::networking::http_3_download_throughput_50_100
+      case HttpVersion::v3_0:
-                .AccumulateSingleSample(mpbs);
+        glean::networking::http_3_download_throughput.AccumulateSingleSample(
-          } else {
+            mpbs);
-            glean::networking::http_3_download_throughput_100
+        if (mContentRead <= TELEMETRY_REQUEST_SIZE_50M) {
-                .AccumulateSingleSample(mpbs);
+          glean::networking::http_3_download_throughput_10_50
-          }
+              .AccumulateSingleSample(mpbs);
-          break;
+        } else if (mContentRead <= TELEMETRY_REQUEST_SIZE_100M) {
-        default:
+          glean::networking::http_3_download_throughput_50_100
-          break;
+              .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(

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 {

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}`);