Backed out 2 changesets (bug 1857615, bug 1857926) for causing build bustages on HttpChannelChild.cpp. CLOSED TREE

Backed out changeset b29044126de6 (bug 1857615)
Backed out changeset 8dcb0b3fc1f3 (bug 1857926)

netwerk/metrics.yaml

@@ -165,118 +165,6 @@ networking:
     expires: never
     telemetry_mirror: DNS_FAILED_LOOKUP_TIME
 
-  http_content_onstart_delay:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between dispatching OnStartRequest from the socket thread and processing it on the main thread (content process).
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926#c
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
-  http_content_onstop_delay:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between dispatching OnStopRequest from the socket thread and processing it on the main thread (content process).
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926#c
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
-  http_content_ondatafinished_delay:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between dispatching OnDataFinished from the socket thread and processing it.
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857615
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857615#c
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
-  http_content_ondatafinished_to_onstop_delay:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between processing OnDataFinished and processing OnStopRequest (if OnDataFinished comes first)
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857615
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857615#c
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
-  http_content_ondatafinished_to_onstop_delay_negative:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between processing OnStopRequest and processing OnDataFinished (if OnStopRequest comes first)
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857615
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857615#c
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
-  http_content_html5parser_ondatafinished_to_onstop_delay:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between processing OnDataFinished and processing OnStopRequest (if OnDataFinished comes first)
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
-  http_content_html5parser_ondatafinished_to_onstop_delay_negative:
-    type: timing_distribution
-    time_unit: millisecond
-    description: >
-      The time between processing OnStopRequest and processing OnDataFinished (if OnStopRequest comes first)
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1857926
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - necko@mozilla.com
-      - rjesup@mozilla.com
-    expires: 130
-
   http_1_download_throughput:
     type: custom_distribution
     unit: mbps

netwerk/protocol/http/HttpChannelChild.cpp

@@ -353,17 +353,13 @@ void HttpChannelChild::ProcessOnStartRequest(
   LOG(("HttpChannelChild::ProcessOnStartRequest [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
 
-  TimeStamp start = TimeStamp::Now();
-
   mAltDataInputStream = DeserializeIPCStream(aAltData.altDataInputStream());
 
   mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
-      this, [self = UnsafePtr<HttpChannelChild>(this), aResponseHead,
-             aUseResponseHead, aRequestHeaders, aArgs, start]() {
-        TimeDuration delay = TimeStamp::Now() - start;
-        glean::networking::http_content_onstart_delay.AccumulateRawDuration(
-            delay);
-
+      this,
+      [self = UnsafePtr<HttpChannelChild>(this), aResponseHead,
+       aUseResponseHead, aRequestHeaders, aArgs, aOnStartRequestStartTime]() {
+        self->mOnStartRequestStartTime = aOnStartRequestStartTime;
         self->OnStartRequest(aResponseHead, aUseResponseHead, aRequestHeaders,
                              aArgs);
       }));
@@ -833,10 +829,6 @@ void HttpChannelChild::SendOnDataFinished(const nsresult& aChannelStatus) {
           ("HttpChannelChild::SendOnDataFinished sending data end "
            "notification[this=%p]\n",
            this));
-      // We want to calculate the delta time between this call and
-      // ProcessOnStopRequest.  Complicating things is that OnStopRequest
-      // could come first, and that it will run on a different thread, so
-      // we need to synchronize and lock data.
       omtEventListener->OnDataFinished(aChannelStatus);
     } else {
       LOG(
@@ -864,43 +856,12 @@ void HttpChannelChild::ProcessOnStopRequest(
     mTransferSize = aTiming.transferSize();
     mEncodedBodySize = aTiming.encodedBodySize();
   }
-
-  class RecordStopRequestDelta final {
-   public:
-    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(RecordStopRequestDelta);
-
-    TimeStamp mOnStopRequestTime;
-    TimeStamp mOnDataFinishedTime;
-
-   private:
-    ~RecordStopRequestDelta() {
-      TimeDuration delta = (mOnStopRequestTime - mOnDataFinishedTime);
-      if (delta.ToMilliseconds() < 0) {
-        // Because Telemetry can't handle negatives
-        delta = -delta;
-        glean::networking::http_content_ondatafinished_to_onstop_delay_negative
-            .AccumulateRawDuration(delta);
-      } else {
-        glean::networking::http_content_ondatafinished_to_onstop_delay
-            .AccumulateRawDuration(delta);
-      }
-    }
-  };
-
-  RefPtr<RecordStopRequestDelta> timing(new RecordStopRequestDelta);
-  TimeStamp start = TimeStamp::Now();
   if (StaticPrefs::network_send_OnDataFinished()) {
     mEventQ->RunOrEnqueue(new ChannelFunctionEvent(
         [self = UnsafePtr<HttpChannelChild>(this)]() {
           return self->GetODATarget();
         },
-        [self = UnsafePtr<HttpChannelChild>(this), status = aChannelStatus,
-         start, timing]() {
-          TimeStamp now = TimeStamp::Now();
-          TimeDuration delay = now - start;
-          glean::networking::http_content_ondatafinished_delay
-              .AccumulateRawDuration(delay);
-          timing->mOnDataFinishedTime = now;
+        [self = UnsafePtr<HttpChannelChild>(this), status = aChannelStatus]() {
           self->SendOnDataFinished(status);
         }));
   }
@@ -908,12 +869,8 @@ void HttpChannelChild::ProcessOnStopRequest(
       this, [self = UnsafePtr<HttpChannelChild>(this), aChannelStatus, aTiming,
              aResponseTrailers,
              consoleReports = CopyableTArray{aConsoleReports.Clone()},
-             aFromSocketProcess, start, timing]() mutable {
-        TimeStamp now = TimeStamp::Now();
-        TimeDuration delay = now - start;
-        glean::networking::http_content_onstop_delay.AccumulateRawDuration(
-            delay);
-        timing->mOnStopRequestTime = now;
+             aFromSocketProcess, aOnStopRequestStartTime]() mutable {
+        self->mOnStopRequestStartTime = aOnStopRequestStartTime;
         self->OnStopRequest(aChannelStatus, aTiming, aResponseTrailers);
         if (!aFromSocketProcess) {
           self->DoOnConsoleReport(std::move(consoleReports));

parser/html/nsHtml5StreamParser.cpp

@@ -1395,50 +1395,34 @@ nsresult nsHtml5StreamParser::OnStopRequest(
     const mozilla::ReentrantMonitorAutoEnter& aProofOfLock) {
   MOZ_ASSERT_IF(aRequest, mRequest == aRequest);
   if (mOnStopCalled) {
-    if (mOnDataFinishedTime) {
-      mOnStopRequestTime = TimeStamp::Now();
-    } else {
-      mOnDataFinishedTime = TimeStamp::Now();
-    }
-  } else {
-    mOnStopCalled = true;
-
-    if (MOZ_UNLIKELY(NS_IsMainThread())) {
-      mOnStopRequestTime = TimeStamp::Now();
-      nsCOMPtr<nsIRunnable> stopper = new nsHtml5RequestStopper(this);
-      if (NS_FAILED(
-              mEventTarget->Dispatch(stopper, nsIThread::DISPATCH_NORMAL))) {
-        NS_WARNING("Dispatching StopRequest event failed.");
-      }
-    } else {
-      mOnDataFinishedTime = TimeStamp::Now();
-
-      if (StaticPrefs::network_send_OnDataFinished_html5parser()) {
-        MOZ_ASSERT(IsParserThread(), "Wrong thread!");
-
-        mozilla::MutexAutoLock autoLock(mTokenizerMutex);
-        DoStopRequest();
-        PostLoadFlusher();
-      } else {
-        // Let the MainThread event handle this, even though it will just
-        // send it back to this thread, so we can accurately judge the impact
-        // of this change.   This should eventually be removed
-      }
-    }
+    return NS_OK;
   }
-  if (!mOnStopRequestTime.IsNull() && !mOnDataFinishedTime.IsNull()) {
-    TimeDuration delta = (mOnStopRequestTime - mOnDataFinishedTime);
-    if (delta.ToMilliseconds() < 0) {
-      // Because Telemetry can't handle negatives
-      delta = -delta;
-      glean::networking::
-          http_content_html5parser_ondatafinished_to_onstop_delay_negative
-              .AccumulateRawDuration(delta);
-    } else {
-      glean::networking::http_content_html5parser_ondatafinished_to_onstop_delay
-          .AccumulateRawDuration(delta);
+
+  mOnStopCalled = true;
+
+  if (MOZ_UNLIKELY(NS_IsMainThread())) {
+    nsCOMPtr<nsIRunnable> stopper = new nsHtml5RequestStopper(this);
+    if (NS_FAILED(
+            mEventTarget->Dispatch(stopper, nsIThread::DISPATCH_NORMAL))) {
+      NS_WARNING("Dispatching StopRequest event failed.");
     }
+    return NS_OK;
   }
+
+  if (!StaticPrefs::network_send_OnDataFinished_html5parser()) {
+    // Let the MainThread event handle this, even though it will just
+    // send it back to this thread, so we can accurately judge the impact
+    // of this change. This should eventually be removed once the PoC is
+    // complete
+    mOnStopCalled = false;
+    return NS_OK;
+  }
+
+  MOZ_ASSERT(IsParserThread(), "Wrong thread!");
+
+  mozilla::MutexAutoLock autoLock(mTokenizerMutex);
+  DoStopRequest();
+  PostLoadFlusher();
   return NS_OK;
 }
 

parser/html/nsHtml5StreamParser.h

@@ -771,13 +771,6 @@ class nsHtml5StreamParser final : public nsISupports {
    * nsHtml5StreamListener::mDelegateMonitor passed in the OnStopRequest
    */
   bool mOnStopCalled{false};
-
-  /*
-   * Used for telemetry about OnStopRequest vs OnDataFinished
-   */
-  // guarded by nsHtml5StreamListener::mDelegateMonitor
-  mozilla::TimeStamp mOnStopRequestTime;
-  mozilla::TimeStamp mOnDataFinishedTime;
 };
 
 #endif  // nsHtml5StreamParser_h