Bug 1579765 - Don't overwrite redirect timings when we convert DocumentChannel to a real channel. r=mayhemer

DocumentChannel doesn't use the REDIRECT_INTERNAL flag when replacing itself with a real channel if there has been a real redirect handled in the parent.
This is so that the docshell knows about the URI change, and to add history entries.
The timing data however always wanted to be treated as an internal redirect, so that we don't record a new 'redirectEnd' at the time of the switch.
This adds a new parameter to ConfigureReplacementChannel so that we can more accurately describe the behaviour we need.

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

--HG--
extra : moz-landing-system : lando

dom/ipc/ContentChild.cpp

@@ -3657,7 +3657,9 @@ mozilla::ipc::IPCResult ContentChild::RecvCrossProcessRedirect(
   }
 
   HttpBaseChannel::ReplacementChannelConfig config(aConfig);
-  HttpBaseChannel::ConfigureReplacementChannel(newChannel, config);
+  HttpBaseChannel::ConfigureReplacementChannel(
+      newChannel, config,
+      HttpBaseChannel::ConfigureReason::DocumentChannelReplacement);
 
   // connect parent.
   rv = httpChild->ConnectParent(aRegistrarId);  // creates parent channel

netwerk/ipc/DocumentChannelChild.cpp

@@ -115,7 +115,7 @@ DocumentChannelChild::AsyncOpen(nsIStreamListener* aListener) {
     // We may have been canceled already, either by on-modify-request
     // listeners or by load group observers; in that case, don't create IPDL
     // connection. See nsHttpChannel::AsyncOpen().
-     return mStatus;
+    return mStatus;
   }
 
   gHttpHandler->OnOpeningDocumentRequest(this);
@@ -317,7 +317,9 @@ IPCResult DocumentChannelChild::RecvRedirectToRealChannel(
 
   if (aInit) {
     HttpBaseChannel::ReplacementChannelConfig config(*aInit);
-    HttpBaseChannel::ConfigureReplacementChannel(newChannel, config);
+    HttpBaseChannel::ConfigureReplacementChannel(
+        newChannel, config,
+        HttpBaseChannel::ConfigureReason::DocumentChannelReplacement);
   }
 
   if (aContentDisposition) {

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -3247,7 +3247,8 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
 }
 
 /* static */ void HttpBaseChannel::ConfigureReplacementChannel(
-    nsIChannel* newChannel, const ReplacementChannelConfig& config) {
+    nsIChannel* newChannel, const ReplacementChannelConfig& config,
+    ConfigureReason aReason) {
   newChannel->SetLoadFlags(config.loadFlags);
 
   nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(newChannel));
@@ -3269,8 +3270,11 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
   if (config.timedChannel && newTimedChannel) {
     newTimedChannel->SetTimingEnabled(config.timedChannel->timingEnabled());
 
-    uint32_t redirectFlags = config.redirectFlags;
-    if (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) {
+    // 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.
+    bool shouldHideTiming = aReason != ConfigureReason::Redirect;
+    if (shouldHideTiming) {
       newTimedChannel->SetRedirectCount(config.timedChannel->redirectCount());
       int8_t newCount = config.timedChannel->internalRedirectCount() + 1;
       newTimedChannel->SetInternalRedirectCount(
@@ -3283,7 +3287,7 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
           config.timedChannel->internalRedirectCount());
     }
 
-    if (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) {
+    if (shouldHideTiming) {
       if (!config.timedChannel->channelCreation().IsNull()) {
         newTimedChannel->SetChannelCreation(
             config.timedChannel->channelCreation());
@@ -3298,7 +3302,7 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
     // previous channel (this is the first redirect in the redirects chain).
     if (config.timedChannel->redirectStart().IsNull()) {
       // Only do this for real redirects.  Internal redirects should be hidden.
-      if (!(redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL)) {
+      if (!shouldHideTiming) {
         newTimedChannel->SetRedirectStart(config.timedChannel->asyncOpen());
       }
     } else {
@@ -3309,7 +3313,7 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
     // forward.  Otherwise the new redirect end time is the last response
     // end time.
     TimeStamp newRedirectEnd;
-    if (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) {
+    if (shouldHideTiming) {
       newRedirectEnd = config.timedChannel->redirectEnd();
     } else {
       newRedirectEnd = config.timedChannel->responseEnd();
@@ -3480,7 +3484,11 @@ nsresult HttpBaseChannel::SetupReplacementChannel(nsIURI* newURI,
 
   ReplacementChannelConfig config = CloneReplacementChannelConfig(
       preserveMethod, redirectFlags, LOAD_REPLACE);
-  ConfigureReplacementChannel(newChannel, config);
+  ConfigureReason redirectType =
+      (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL)
+          ? ConfigureReason::InternalRedirect
+          : ConfigureReason::Redirect;
+  ConfigureReplacementChannel(newChannel, config, redirectType);
 
   // Check whether or not this was a cross-domain redirect.
   nsCOMPtr<nsITimedChannel> newTimedChannel(do_QueryInterface(newChannel));

netwerk/protocol/http/HttpBaseChannel.h

@@ -508,8 +508,15 @@ class HttpBaseChannel : public nsHashPropertyBag,
       bool aPreserveMethod, uint32_t aRedirectFlags,
       uint32_t aExtraLoadFlags = 0);
 
+  enum class ConfigureReason {
+    Redirect,
+    InternalRedirect,
+    DocumentChannelReplacement,
+  };
+
   static void ConfigureReplacementChannel(nsIChannel*,
-                                          const ReplacementChannelConfig&);
+                                          const ReplacementChannelConfig&,
+                                          ConfigureReason);
 
   // Called before we create the redirect target channel.
   already_AddRefed<nsILoadInfo> CloneLoadInfoForRedirect(