Bug 1732250, use the original URI from the old channel when updating session history entry, r=peterv,necko-reviewers,valentin

https://searchfox.org/mozilla-central/rev/1df999af9999ccb436512cfece57a68d94d36e08/netwerk/protocol/http/nsHttpChannel.cpp#2876
makes original uri handling in the channel rather magical. The value of it on the new channel is bogus during
AsyncOnChannelRedirect call, and nsIChannel.idl doesn't hint about that behavior.

browser_getNavigationHistory.js can work as a testcase once it is enabled for Fission.

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

docshell/base/CanonicalBrowsingContext.cpp

@@ -568,12 +568,13 @@ CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad(
 
 UniquePtr<LoadingSessionHistoryInfo>
 CanonicalBrowsingContext::ReplaceLoadingSessionHistoryEntryForLoad(
-    LoadingSessionHistoryInfo* aInfo, nsIChannel* aChannel) {
+    LoadingSessionHistoryInfo* aInfo, nsIChannel* aOldChannel,
+    nsIChannel* aNewChannel) {
   MOZ_ASSERT(aInfo);
-  MOZ_ASSERT(aChannel);
+  MOZ_ASSERT(aNewChannel);
 
   SessionHistoryInfo newInfo = SessionHistoryInfo(
-      aChannel, aInfo->mInfo.LoadType(),
+      aOldChannel, aNewChannel, aInfo->mInfo.LoadType(),
       aInfo->mInfo.GetPartitionedPrincipalToInherit(), aInfo->mInfo.GetCsp());
 
   for (size_t i = 0; i < mLoadingEntries.Length(); ++i) {
@@ -584,9 +585,9 @@ CanonicalBrowsingContext::ReplaceLoadingSessionHistoryEntryForLoad(
       if (IsTop()) {
         // Only top level pages care about Get/SetPersist.
         nsCOMPtr<nsIURI> uri;
-        aChannel->GetURI(getter_AddRefs(uri));
+        aNewChannel->GetURI(getter_AddRefs(uri));
         loadingEntry->SetPersist(
-            nsDocShell::ShouldAddToSessionHistory(uri, aChannel));
+            nsDocShell::ShouldAddToSessionHistory(uri, aNewChannel));
       } else {
         loadingEntry->SetIsSubFrame(aInfo->mInfo.IsSubFrame());
       }

docshell/base/CanonicalBrowsingContext.h

@@ -125,7 +125,8 @@ class CanonicalBrowsingContext final : public BrowsingContext {
       nsDocShellLoadState* aLoadState, nsIChannel* aChannel);
 
   UniquePtr<LoadingSessionHistoryInfo> ReplaceLoadingSessionHistoryEntryForLoad(
-      LoadingSessionHistoryInfo* aInfo, nsIChannel* aChannel);
+      LoadingSessionHistoryInfo* aInfo, nsIChannel* aOldChannel,
+      nsIChannel* aNewChannel);
 
   already_AddRefed<Promise> Print(nsIPrintSettings* aPrintSettings,
                                   ErrorResult& aRv);

docshell/shistory/SessionHistoryEntry.cpp

@@ -81,14 +81,14 @@ SessionHistoryInfo::SessionHistoryInfo(
 }
 
 SessionHistoryInfo::SessionHistoryInfo(
-    nsIChannel* aChannel, uint32_t aLoadType,
+    nsIChannel* aOldChannel, nsIChannel* aNewChannel, uint32_t aLoadType,
     nsIPrincipal* aPartitionedPrincipalToInherit,
     nsIContentSecurityPolicy* aCsp) {
-  aChannel->GetURI(getter_AddRefs(mURI));
+  aNewChannel->GetURI(getter_AddRefs(mURI));
   mLoadType = aLoadType;
 
   nsCOMPtr<nsILoadInfo> loadInfo;
-  aChannel->GetLoadInfo(getter_AddRefs(loadInfo));
+  aNewChannel->GetLoadInfo(getter_AddRefs(loadInfo));
 
   loadInfo->GetResultPrincipalURI(getter_AddRefs(mResultPrincipalURI));
   loadInfo->GetTriggeringPrincipal(
@@ -99,16 +99,16 @@ SessionHistoryInfo::SessionHistoryInfo(
   mSharedState.Get()->mPartitionedPrincipalToInherit =
       aPartitionedPrincipalToInherit;
   mSharedState.Get()->mCsp = aCsp;
-  aChannel->GetContentType(mSharedState.Get()->mContentType);
-  aChannel->GetOriginalURI(getter_AddRefs(mOriginalURI));
+  aNewChannel->GetContentType(mSharedState.Get()->mContentType);
+  aOldChannel->GetOriginalURI(getter_AddRefs(mOriginalURI));
 
   uint32_t loadFlags;
-  aChannel->GetLoadFlags(&loadFlags);
+  aNewChannel->GetLoadFlags(&loadFlags);
   mLoadReplace = !!(loadFlags & nsIChannel::LOAD_REPLACE);
 
   MaybeUpdateTitleFromURI();
 
-  if (nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel)) {
+  if (nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aNewChannel)) {
     mReferrerInfo = httpChannel->GetReferrerInfo();
   }
 }

docshell/shistory/SessionHistoryEntry.h

@@ -47,7 +47,8 @@ class SessionHistoryInfo {
                      nsIPrincipal* aPartitionedPrincipalToInherit,
                      nsIContentSecurityPolicy* aCsp,
                      const nsACString& aContentType);
-  SessionHistoryInfo(nsIChannel* aChannel, uint32_t aLoadType,
+  SessionHistoryInfo(nsIChannel* aOldChannel, nsIChannel* aNewChannel,
+                     uint32_t aLoadType,
                      nsIPrincipal* aPartitionedPrincipalToInherit,
                      nsIContentSecurityPolicy* aCsp);
 

netwerk/base/nsIChannel.idl

@@ -42,6 +42,9 @@ interface nsIChannel : nsIRequest
      * NOTE: this is distinctly different from the http Referer (referring URI),
      * which is typically the page that contained the original URI (accessible
      * from nsIHttpChannel).
+     *
+     * NOTE: originalURI isn't yet set on the new channel when
+     * asyncOnChannelRedirect is called.
      */
     attribute nsIURI originalURI;
 

netwerk/base/nsIChannelEventSink.idl

@@ -77,6 +77,9 @@ interface nsIChannelEventSink : nsISupports
      * the redirect is vetoed and no callback must be done. Repeat: No
      * callback must be done if this method throws!
      *
+     * NOTE: originalURI isn't yet set on the new channel when
+     * asyncOnChannelRedirect is called.
+     *
      * @see nsIAsyncVerifyRedirectCallback::onRedirectVerifyCallback()
      *
      * @param oldChannel

netwerk/ipc/DocumentLoadListener.cpp

@@ -2474,7 +2474,7 @@ DocumentLoadListener::AsyncOnChannelRedirect(
       mLoadingSessionHistoryInfo =
           GetDocumentBrowsingContext()
               ->ReplaceLoadingSessionHistoryEntryForLoad(
-                  mLoadingSessionHistoryInfo.get(), aNewChannel);
+                  mLoadingSessionHistoryInfo.get(), aOldChannel, aNewChannel);
     }
     if (!net::ChannelIsPost(aOldChannel)) {
       AddURIVisit(aOldChannel, 0);