Bug 1735613 - Part 2: When redirecting in Document Channel, use the correct URI, r=nika

If we enable pref browser.tabs.documentchannel.parent-controlled and remove the
requirement for needing SHIP for parent-initiated loads, test
NavigationDelegateTest#redirectIntentLoad times out. It times out because when
redirecting in Document Channel, we were using the URI of the redirect that
failed.

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

netwerk/base/LoadInfo.cpp

@@ -522,6 +522,7 @@ LoadInfo::LoadInfo(const LoadInfo& rhs)
       mPrincipalToInherit(rhs.mPrincipalToInherit),
       mTopLevelPrincipal(rhs.mTopLevelPrincipal),
       mResultPrincipalURI(rhs.mResultPrincipalURI),
+      mChannelCreationOriginalURI(rhs.mChannelCreationOriginalURI),
       mCookieJarSettings(rhs.mCookieJarSettings),
       mCspToInherit(rhs.mCspToInherit),
       mSandboxedNullPrincipalID(rhs.mSandboxedNullPrincipalID),
@@ -1778,6 +1779,18 @@ LoadInfo::SetResultPrincipalURI(nsIURI* aURI) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+LoadInfo::GetChannelCreationOriginalURI(nsIURI** aURI) {
+  *aURI = do_AddRef(mChannelCreationOriginalURI).take();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+LoadInfo::SetChannelCreationOriginalURI(nsIURI* aURI) {
+  mChannelCreationOriginalURI = aURI;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 LoadInfo::SetRequestBlockingReason(uint32_t aReason) {
   mRequestBlockingReason = aReason;

netwerk/base/LoadInfo.h

@@ -264,6 +264,7 @@ class LoadInfo final : public nsILoadInfo {
   nsCOMPtr<nsIPrincipal> mPrincipalToInherit;
   nsCOMPtr<nsIPrincipal> mTopLevelPrincipal;
   nsCOMPtr<nsIURI> mResultPrincipalURI;
+  nsCOMPtr<nsIURI> mChannelCreationOriginalURI;
   nsCOMPtr<nsICSPEventListener> mCSPEventListener;
   nsCOMPtr<nsICookieJarSettings> mCookieJarSettings;
   nsCOMPtr<nsIContentSecurityPolicy> mCspToInherit;

netwerk/base/TRRLoadInfo.cpp

@@ -577,6 +577,16 @@ TRRLoadInfo::SetResultPrincipalURI(nsIURI* aURI) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+TRRLoadInfo::GetChannelCreationOriginalURI(nsIURI** aURI) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
+TRRLoadInfo::SetChannelCreationOriginalURI(nsIURI* aURI) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 NS_IMETHODIMP
 TRRLoadInfo::SetRequestBlockingReason(uint32_t aReason) {
   return NS_ERROR_NOT_IMPLEMENTED;

netwerk/base/nsILoadInfo.idl

@@ -1030,6 +1030,16 @@ interface nsILoadInfo : nsISupports
    */
   attribute nsIURI resultPrincipalURI;
 
+  /**
+   * This is the URI used to create the most recent channel in the load's
+   * redirect chain, if it's different from channel's `originalURI`.
+   * This is always null for loads not handled by DocumentLoadListener. If
+   * non-null, channelCreationOriginalURI will be used instead of channel's
+   * originalURI to re-create the channel in the final content process selected
+   * to perform the load.
+   */
+  attribute nsIURI channelCreationOriginalURI;
+
   /**
    * Returns a unique nsID used to construct the null principal for the
    * resulting resource if the SANDBOXED_ORIGIN flag is set.  This is used by

netwerk/ipc/DocumentLoadListener.cpp

@@ -748,8 +748,10 @@ auto DocumentLoadListener::Open(nsDocShellLoadState* aLoadState,
   // after opening the document channel we have to kick off countermeasures.
   nsHTTPSOnlyUtils::PotentiallyFireHttpRequestToShortenTimout(this);
 
+  nsCOMPtr<nsILoadInfo> loadInfo = mChannel->LoadInfo();
+  loadInfo->SetChannelCreationOriginalURI(aLoadState->URI());
+
   mContentParent = aContentParent;
-  mChannelCreationURI = aLoadState->URI();
   mLoadStateExternalLoadFlags = aLoadState->LoadFlags();
   mLoadStateInternalLoadFlags = aLoadState->InternalLoadFlags();
   mLoadStateLoadType = aLoadState->LoadType();
@@ -1385,13 +1387,7 @@ void DocumentLoadListener::SerializeRedirectData(
     RedirectToRealChannelArgs& aArgs, bool aIsCrossProcess,
     uint32_t aRedirectFlags, uint32_t aLoadFlags,
     ContentParent* aParent) const {
-  // Use the original URI of the current channel, as this is what
-  // we'll use to construct the channel in the content process.
-  aArgs.uri() = mChannelCreationURI;
-  if (!aArgs.uri()) {
-    mChannel->GetOriginalURI(getter_AddRefs(aArgs.uri()));
-  }
-
+  aArgs.uri() = GetChannelCreationURI();
   aArgs.loadIdentifier() = mLoadIdentifier;
 
   // I previously used HttpBaseChannel::CloneLoadInfoForRedirect, but that
@@ -1581,9 +1577,9 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch(
   MOZ_DIAGNOSTIC_ASSERT(aWillSwitchToRemote);
 
   MOZ_LOG(gProcessIsolationLog, LogLevel::Verbose,
-          ("DocumentLoadListener MaybeTriggerProcessSwitch [this=%p, uri=%s, "
+          ("DocumentLoadListener MaybeTriggerProcessSwitch [this=%p, uri=%s"
            "browserid=%" PRIx64 "]",
-           this, mChannelCreationURI->GetSpecOrDefault().get(),
+           this, GetChannelCreationURI()->GetSpecOrDefault().get(),
            GetLoadingBrowsingContext()->Top()->BrowserId()));
 
   // If we're doing an <object>/<embed> load, we may be doing a document load at
@@ -1638,7 +1634,7 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch(
   }
 
   auto optionsResult = IsolationOptionsForNavigation(
-      browsingContext->Top(), parentWindow, mChannelCreationURI, mChannel,
+      browsingContext->Top(), parentWindow, GetChannelCreationURI(), mChannel,
       currentRemoteType, HasCrossOriginOpenerPolicyMismatch(),
       mLoadStateLoadType, mDocumentChannelId, mRemoteTypeOverride);
   if (optionsResult.isErr()) {
@@ -2056,7 +2052,7 @@ bool DocumentLoadListener::DocShellWillDisplayContent(nsresult aStatus) {
             ("Skipping process switch, as DocShell will not display content "
              "(status: %s) %s",
              GetStaticErrorName(aStatus),
-             mChannelCreationURI->GetSpecOrDefault().get()));
+             GetChannelCreationURI()->GetSpecOrDefault().get()));
   }
 
   // If filtering returned a failure code, then an error page will
@@ -2477,6 +2473,17 @@ DocumentLoadListener::AsyncOnChannelRedirect(
   // so just update our channel and tell the callback that we're good to go.
   mChannel = aNewChannel;
 
+  // We need the original URI of the current channel to use to open the real
+  // channel in the content process. Unfortunately we overwrite the original
+  // uri of the new channel with the original pre-redirect URI, so grab
+  // a copy of it now and save it on the loadInfo corresponding to the
+  // new channel.
+  nsCOMPtr<nsILoadInfo> loadInfoFromChannel = mChannel->LoadInfo();
+  MOZ_ASSERT(loadInfoFromChannel);
+  nsCOMPtr<nsIURI> uri;
+  mChannel->GetOriginalURI(getter_AddRefs(uri));
+  loadInfoFromChannel->SetChannelCreationOriginalURI(uri);
+
   // Since we're redirecting away from aOldChannel, we should check if it
   // had a COOP mismatch, since we want the final result for this to
   // include the state of all channels we redirected through.
@@ -2525,12 +2532,6 @@ DocumentLoadListener::AsyncOnChannelRedirect(
        "mHaveVisibleRedirect=%c",
        this, mHaveVisibleRedirect ? 'T' : 'F'));
 
-  // We need the original URI of the current channel to use to open the real
-  // channel in the content process. Unfortunately we overwrite the original
-  // uri of the new channel with the original pre-redirect URI, so grab
-  // a copy of it now.
-  aNewChannel->GetOriginalURI(getter_AddRefs(mChannelCreationURI));
-
   // Clear out our nsIParentChannel functions, since a normal parent
   // channel would actually redirect and not have those values on the new one.
   // We expect the URI classifier to run on the redirected channel with
@@ -2581,6 +2582,23 @@ DocumentLoadListener::AsyncOnChannelRedirect(
   return NS_OK;
 }
 
+nsIURI* DocumentLoadListener::GetChannelCreationURI() const {
+  nsCOMPtr<nsILoadInfo> channelLoadInfo = mChannel->LoadInfo();
+
+  nsCOMPtr<nsIURI> uri;
+  channelLoadInfo->GetChannelCreationOriginalURI(getter_AddRefs(uri));
+  if (uri) {
+    // See channelCreationOriginalURI for more info. We use this instead of the
+    // originalURI of the channel to help us avoid the situation when we use
+    // the URI of a redirect that has failed to happen.
+    return uri;
+  }
+
+  // Otherwise, get the original URI from the channel.
+  mChannel->GetOriginalURI(getter_AddRefs(uri));
+  return uri;
+}
+
 // This method returns the cached result of running the Cross-Origin-Opener
 // policy compare algorithm by calling ComputeCrossOriginOpenerPolicyMismatch
 bool DocumentLoadListener::HasCrossOriginOpenerPolicyMismatch() const {

netwerk/ipc/DocumentLoadListener.h

@@ -503,11 +503,9 @@ class DocumentLoadListener : public nsIInterfaceRequestor,
   // replaces us.
   RefPtr<ParentChannelListener> mParentChannelListener;
 
-  // The original URI of the current channel. If there are redirects,
-  // then the value on the channel gets overwritten with the original
-  // URI of the first channel in the redirect chain, so we cache the
-  // value we need here.
-  nsCOMPtr<nsIURI> mChannelCreationURI;
+  // Get the channel creation URI for constructing the channel in the content
+  // process. See the function for more details.
+  nsIURI* GetChannelCreationURI() const;
 
   // The original navigation timing information containing various timestamps
   // such as when the original load started.