tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 78c28fa0ca195f6223f4f65b9b57bb832f7ca0c1
parent 47a1818c6a30cb6192b2289993cf63c9bb419df7
Author: Adam Vandolder <avandolder@mozilla.com>
Date:   Thu, 16 Oct 2025 16:10:17 +0000

Bug 1993456 - Recreate contiguous entries array when replacing loading info after a redirect. r=farre,dom-core

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

Diffstat:
Mdocshell/base/CanonicalBrowsingContext.cpp | 105+++++++++++++++++++++++++++++++++++++++++++++----------------------------------
Mdocshell/base/CanonicalBrowsingContext.h | 3+++
Mdocshell/shistory/SessionHistoryEntry.cpp | 2++
Atesting/web-platform/tests/navigation-api/navigation-history-entry/entries-after-server-redirect.html | 21+++++++++++++++++++++
4 files changed, 86 insertions(+), 45 deletions(-)

diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp @@ -672,38 +672,19 @@ CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad( } } - nsCOMPtr<nsIURI> uri = - mActiveEntry ? mActiveEntry->GetURIOrInheritedForAboutBlank() : nullptr; - nsCOMPtr<nsIURI> targetURI = entry->GetURIOrInheritedForAboutBlank(); - bool sameOrigin = - NS_SUCCEEDED(nsContentUtils::GetSecurityManager()->CheckSameOriginURI( - targetURI, uri, false, false)); - if (entry->isInList() || - (mActiveEntry && mActiveEntry->isInList() && sameOrigin)) { - nsSHistory::WalkContiguousEntriesInOrder( - entry->isInList() ? entry : mActiveEntry, - [activeEntry = mActiveEntry, - entries = &loadingInfo->mContiguousEntries, - navigationType = *navigationType](auto* aEntry) { - nsCOMPtr<SessionHistoryEntry> entry = do_QueryObject(aEntry); - MOZ_ASSERT(entry); - if (navigationType == NavigationType::Replace && - entry == activeEntry) { - // In the case of a replace navigation, we end up dropping the - // active entry and all following entries. - return false; - } - entries->AppendElement(entry->Info()); - // In the case of a push navigation, we end up keeping the - // current active entry but drop all following entries. - return !(navigationType == NavigationType::Push && - entry == activeEntry); - }); - } + loadingInfo->mTriggeringEntry = + mActiveEntry ? Some(mActiveEntry->Info()) : Nothing(); + MOZ_LOG_FMT(gNavigationAPILog, LogLevel::Verbose, + "Triggering entry was {}.", + fmt::ptr(loadingInfo->mTriggeringEntry + .map([](auto& entry) { return &entry; }) + .valueOr(nullptr))); - if (!sessionHistoryLoad || !sameOrigin) { - loadingInfo->mContiguousEntries.AppendElement(entry->Info()); - } + loadingInfo->mTriggeringNavigationType = navigationType; + MOZ_LOG_FMT(gNavigationAPILog, LogLevel::Verbose, + "Triggering navigation type was {}.", *navigationType); + + GetContiguousEntriesForLoad(*loadingInfo, entry); if (MOZ_LOG_TEST(gNavigationAPILog, LogLevel::Debug)) { int32_t index = 0; @@ -721,18 +702,6 @@ CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad( } } - loadingInfo->mTriggeringEntry = - mActiveEntry ? Some(mActiveEntry->Info()) : Nothing(); - MOZ_LOG_FMT(gNavigationAPILog, LogLevel::Verbose, - "Triggering entry was {}.", - fmt::ptr(loadingInfo->mTriggeringEntry - .map([](auto& entry) { return &entry; }) - .valueOr(nullptr))); - - loadingInfo->mTriggeringNavigationType = navigationType; - MOZ_LOG_FMT(gNavigationAPILog, LogLevel::Verbose, - "Triggering navigation type was {}.", *navigationType); - [[maybe_unused]] auto pred = [&](auto& entry) { return entry.NavigationKey() == loadingInfo->mInfo.NavigationKey(); }; @@ -775,7 +744,7 @@ CanonicalBrowsingContext::ReplaceLoadingSessionHistoryEntryForLoad( loadingEntry->SetInfo(&newInfo); if (IsTop()) { - // Only top level pages care about Get/SetPersist. + // Only top level pages care about Get/SetTransient. nsCOMPtr<nsIURI> uri; aNewChannel->GetURI(getter_AddRefs(uri)); if (!nsDocShell::ShouldAddToSessionHistory(uri, aNewChannel)) { @@ -786,14 +755,60 @@ CanonicalBrowsingContext::ReplaceLoadingSessionHistoryEntryForLoad( } loadingEntry->SetDocshellID(GetHistoryID()); loadingEntry->SetIsDynamicallyAdded(CreatedDynamically()); + auto result = MakeUnique<LoadingSessionHistoryInfo>(loadingEntry, aInfo); - result->mContiguousEntries = aInfo->mContiguousEntries; + MOZ_LOG_FMT( + gNavigationAPILog, LogLevel::Debug, + "CanonicalBrowsingContext::ReplaceLoadingSessionHistoryEntryForLoad: " + "Recreating the contiguous entries list after redirected navigation " + "to {}.", + ToMaybeRef(result->mInfo.GetURI()) + .map(std::mem_fn(&nsIURI::GetSpecOrDefault)) + .valueOr("(null URI)."_ns)); + GetContiguousEntriesForLoad(*result, loadingEntry); return result; } } return nullptr; } +void CanonicalBrowsingContext::GetContiguousEntriesForLoad( + LoadingSessionHistoryInfo& aLoadingInfo, + const RefPtr<SessionHistoryEntry>& aEntry) { + nsCOMPtr<nsIURI> uri = + mActiveEntry ? mActiveEntry->GetURIOrInheritedForAboutBlank() : nullptr; + nsCOMPtr<nsIURI> targetURI = aEntry->GetURIOrInheritedForAboutBlank(); + bool sameOrigin = + NS_SUCCEEDED(nsContentUtils::GetSecurityManager()->CheckSameOriginURI( + targetURI, uri, false, false)); + if (aEntry->isInList() || + (mActiveEntry && mActiveEntry->isInList() && sameOrigin)) { + nsSHistory::WalkContiguousEntriesInOrder( + aEntry->isInList() ? aEntry : mActiveEntry, + [activeEntry = mActiveEntry, entries = &aLoadingInfo.mContiguousEntries, + navigationType = + *aLoadingInfo.mTriggeringNavigationType](auto* aEntry) { + nsCOMPtr<SessionHistoryEntry> entry = do_QueryObject(aEntry); + MOZ_ASSERT(entry); + if (navigationType == NavigationType::Replace && + entry == activeEntry) { + // In the case of a replace navigation, we end up dropping the + // active entry and all following entries. + return false; + } + entries->AppendElement(entry->Info()); + // In the case of a push navigation, we end up keeping the + // current active entry but drop all following entries. + return !(navigationType == NavigationType::Push && + entry == activeEntry); + }); + } + + if (!aLoadingInfo.mLoadIsFromSessionHistory || !sameOrigin) { + aLoadingInfo.mContiguousEntries.AppendElement(aEntry->Info()); + } +} + using PrintPromise = CanonicalBrowsingContext::PrintPromise; #ifdef NS_PRINTING // Clients must call StaticCloneForPrintingCreated or diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h @@ -582,6 +582,9 @@ class CanonicalBrowsingContext final : public BrowsingContext { already_AddRefed<nsDocShellLoadState> CreateLoadInfo( SessionHistoryEntry* aEntry); + void GetContiguousEntriesForLoad(LoadingSessionHistoryInfo& aLoadingInfo, + const RefPtr<SessionHistoryEntry>& aEntry); + // XXX(farre): Store a ContentParent pointer here rather than mProcessId? // Indicates which process owns the docshell. uint64_t mProcessId; diff --git a/docshell/shistory/SessionHistoryEntry.cpp b/docshell/shistory/SessionHistoryEntry.cpp @@ -420,6 +420,8 @@ LoadingSessionHistoryInfo::LoadingSessionHistoryInfo( LoadingSessionHistoryInfo::LoadingSessionHistoryInfo( SessionHistoryEntry* aEntry, const LoadingSessionHistoryInfo* aInfo) : mInfo(aEntry->Info()), + mTriggeringEntry(aInfo->mTriggeringEntry), + mTriggeringNavigationType(aInfo->mTriggeringNavigationType), mLoadId(aInfo->mLoadId), mLoadIsFromSessionHistory(aInfo->mLoadIsFromSessionHistory), mOffset(aInfo->mOffset), diff --git a/testing/web-platform/tests/navigation-api/navigation-history-entry/entries-after-server-redirect.html b/testing/web-platform/tests/navigation-api/navigation-history-entry/entries-after-server-redirect.html @@ -0,0 +1,21 @@ +<!doctype html> +<body> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<iframe id="i" src="/common/blank.html"></iframe> +<script> +promise_test(async t => { + // Wait for after the load event so that the navigation doesn't get converted + // into a replace navigation. + await new Promise(resolve => window.onload = () => t.step_timeout(resolve, 0)); + + // Do a redirected cross-document push navigation in the iframe. + i.contentWindow.navigation.navigate("/common/redirect.py?location=/common/blank.html?redirected"); + await new Promise(resolve => i.onload = resolve); + + assert_equals(i.contentWindow.navigation.entries().length, 2); + assert_equals(i.contentWindow.navigation.entries()[1], i.contentWindow.navigation.currentEntry); + assert_equals(i.contentWindow.navigation.currentEntry.url, i.contentWindow.navigation.entries()[0].url + "?redirected"); +}, "A navigation to a redirected page ends up on the page redirected to"); +</script> +</body>