Bug 1620875 - Don't duplicate loadFlags in the http specific config as well as the generic config for DocumentChannel replacement. r=nika

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

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

dom/ipc/DOMTypes.ipdlh

@@ -309,7 +309,6 @@ struct TimedChannelInfo
 
 struct ReplacementChannelConfigInit
 {
-  uint32_t loadFlags;
   uint32_t redirectFlags;
   uint32_t classOfService;
   bool? privateBrowsing;

netwerk/ipc/DocumentLoadListener.cpp

@@ -805,16 +805,11 @@ void DocumentLoadListener::SerializeRedirectData(
   }
 
   if (baseChannel) {
-    uint32_t loadFlags = 0;
-    if (!aIsCrossProcess) {
-      loadFlags |= nsIChannel::LOAD_REPLACE;
-    }
-
-    aArgs.init() = Some(
-        baseChannel
+    aArgs.init() =
+        Some(baseChannel
                  ->CloneReplacementChannelConfig(
                      true, aRedirectFlags,
-                HttpBaseChannel::ReplacementReason::DocumentChannel, loadFlags)
+                     HttpBaseChannel::ReplacementReason::DocumentChannel)
                  .Serialize());
   }
 

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -3109,24 +3109,8 @@ bool HttpBaseChannel::ShouldRewriteRedirectToGET(
 HttpBaseChannel::ReplacementChannelConfig
 HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
                                                uint32_t aRedirectFlags,
-                                               ReplacementReason aReason,
-                                               uint32_t aExtraLoadFlags) {
+                                               ReplacementReason aReason) {
   ReplacementChannelConfig config;
-  config.loadFlags = mLoadFlags;
-  config.loadFlags |= aExtraLoadFlags;
-
-  // if the original channel was using SSL and this channel is not using
-  // SSL, then no need to inhibit persistent caching.  however, if the
-  // original channel was not using SSL and has INHIBIT_PERSISTENT_CACHING
-  // set, then allow the flag to apply to the redirected channel as well.
-  // since we force set INHIBIT_PERSISTENT_CACHING on all HTTPS channels,
-  // we only need to check if the original channel was using SSL.
-  if (mURI->SchemeIs("https")) {
-    config.loadFlags &= ~INHIBIT_PERSISTENT_CACHING;
-  }
-
-  // Do not pass along LOAD_CHECK_OFFLINE_CACHE
-  config.loadFlags &= ~nsICachingChannel::LOAD_CHECK_OFFLINE_CACHE;
   config.redirectFlags = aRedirectFlags;
   config.classOfService = mClassOfService;
 
@@ -3240,8 +3224,6 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
 /* static */ void HttpBaseChannel::ConfigureReplacementChannel(
     nsIChannel* newChannel, const ReplacementChannelConfig& config,
     ReplacementReason aReason) {
-  newChannel->SetLoadFlags(config.loadFlags);
-
   nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(newChannel));
   if (cos) {
     cos->SetClassFlags(config.classOfService);
@@ -3411,7 +3393,6 @@ HttpBaseChannel::CloneReplacementChannelConfig(bool aPreserveMethod,
 
 HttpBaseChannel::ReplacementChannelConfig::ReplacementChannelConfig(
     const dom::ReplacementChannelConfigInit& aInit) {
-  loadFlags = aInit.loadFlags();
   redirectFlags = aInit.redirectFlags();
   classOfService = aInit.classOfService();
   privateBrowsing = aInit.privateBrowsing();
@@ -3427,7 +3408,6 @@ HttpBaseChannel::ReplacementChannelConfig::ReplacementChannelConfig(
 dom::ReplacementChannelConfigInit
 HttpBaseChannel::ReplacementChannelConfig::Serialize() {
   dom::ReplacementChannelConfigInit config;
-  config.loadFlags() = loadFlags;
   config.redirectFlags() = redirectFlags;
   config.classOfService() = classOfService;
   config.privateBrowsing() = privateBrowsing;
@@ -3471,6 +3451,23 @@ nsresult HttpBaseChannel::SetupReplacementChannel(nsIURI* newURI,
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
+  nsLoadFlags loadFlags = mLoadFlags;
+  loadFlags |= LOAD_REPLACE;
+
+  // if the original channel was using SSL and this channel is not using
+  // SSL, then no need to inhibit persistent caching.  however, if the
+  // original channel was not using SSL and has INHIBIT_PERSISTENT_CACHING
+  // set, then allow the flag to apply to the redirected channel as well.
+  // since we force set INHIBIT_PERSISTENT_CACHING on all HTTPS channels,
+  // we only need to check if the original channel was using SSL.
+  if (mURI->SchemeIs("https")) {
+    loadFlags &= ~INHIBIT_PERSISTENT_CACHING;
+  }
+
+  // Do not pass along LOAD_CHECK_OFFLINE_CACHE
+  loadFlags &= ~nsICachingChannel::LOAD_CHECK_OFFLINE_CACHE;
+  newChannel->SetLoadFlags(loadFlags);
+
   nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(newChannel);
 
   ReplacementReason redirectType =
@@ -3478,7 +3475,7 @@ nsresult HttpBaseChannel::SetupReplacementChannel(nsIURI* newURI,
           ? ReplacementReason::InternalRedirect
           : ReplacementReason::Redirect;
   ReplacementChannelConfig config = CloneReplacementChannelConfig(
-      preserveMethod, redirectFlags, redirectType, LOAD_REPLACE);
+      preserveMethod, redirectFlags, redirectType);
   ConfigureReplacementChannel(newChannel, config, redirectType);
 
   // Check whether or not this was a cross-domain redirect.

netwerk/protocol/http/HttpBaseChannel.h

@@ -485,7 +485,6 @@ class HttpBaseChannel : public nsHashPropertyBag,
     explicit ReplacementChannelConfig(
         const dom::ReplacementChannelConfigInit& aInit);
 
-    uint32_t loadFlags = 0;
     uint32_t redirectFlags = 0;
     uint32_t classOfService = 0;
     Maybe<bool> privateBrowsing = Nothing();
@@ -509,8 +508,7 @@ class HttpBaseChannel : public nsHashPropertyBag,
   // Create a ReplacementChannelConfig object that can be used to duplicate the
   // current channel.
   ReplacementChannelConfig CloneReplacementChannelConfig(
-      bool aPreserveMethod, uint32_t aRedirectFlags, ReplacementReason aReason,
-      uint32_t aExtraLoadFlags = 0);
+      bool aPreserveMethod, uint32_t aRedirectFlags, ReplacementReason aReason);
 
   static void ConfigureReplacementChannel(nsIChannel*,
                                           const ReplacementChannelConfig&,