From 6bc2c7bf4567171ce7f39aee1d27ab3cb8917d0c Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Mon, 16 Mar 2020 00:56:03 +0000 Subject: [PATCH] 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 | 1 - netwerk/ipc/DocumentLoadListener.cpp | 17 ++++------ netwerk/protocol/http/HttpBaseChannel.cpp | 41 +++++++++++------------ netwerk/protocol/http/HttpBaseChannel.h | 4 +-- 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/dom/ipc/DOMTypes.ipdlh b/dom/ipc/DOMTypes.ipdlh index d97fdec324a6..62678f32b6f7 100644 --- a/dom/ipc/DOMTypes.ipdlh +++ b/dom/ipc/DOMTypes.ipdlh @@ -309,7 +309,6 @@ struct TimedChannelInfo struct ReplacementChannelConfigInit { - uint32_t loadFlags; uint32_t redirectFlags; uint32_t classOfService; bool? privateBrowsing; diff --git a/netwerk/ipc/DocumentLoadListener.cpp b/netwerk/ipc/DocumentLoadListener.cpp index c695ca5ad543..d92ab0da54ed 100644 --- a/netwerk/ipc/DocumentLoadListener.cpp +++ b/netwerk/ipc/DocumentLoadListener.cpp @@ -805,17 +805,12 @@ void DocumentLoadListener::SerializeRedirectData( } if (baseChannel) { - uint32_t loadFlags = 0; - if (!aIsCrossProcess) { - loadFlags |= nsIChannel::LOAD_REPLACE; - } - - aArgs.init() = Some( - baseChannel - ->CloneReplacementChannelConfig( - true, aRedirectFlags, - HttpBaseChannel::ReplacementReason::DocumentChannel, loadFlags) - .Serialize()); + aArgs.init() = + Some(baseChannel + ->CloneReplacementChannelConfig( + true, aRedirectFlags, + HttpBaseChannel::ReplacementReason::DocumentChannel) + .Serialize()); } uint32_t contentDispositionTemp; diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 805054177ffc..527004eab4da 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/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 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 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. diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index bc39589ebeb9..d9878650cfaa 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/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 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&,