From 096f64cd95ccd77d704122137ec5426c61987c7f Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Mon, 14 Dec 2020 14:37:25 +0000 Subject: [PATCH] Bug 1671839 - [Fission] Fix and re-enable back.py and navigate.py for Fission, r=peterv I think there could be still other issues with persist handling (or at least it could be simplified), but this should be pretty much the minimal patch to fix the issue when about:newtab url is changed to about:blank (without a redirect, but magical about: handling). So we need to check persisted handling later than currently. I'd prefer to land something like this first and then consider if there are better ways to handle both about: url changes and proper redirects. Depends on D93899 Differential Revision: https://phabricator.services.mozilla.com/D98871 --- docshell/base/CanonicalBrowsingContext.cpp | 5 ++-- docshell/base/CanonicalBrowsingContext.h | 2 +- docshell/base/nsDocShell.cpp | 32 ++++++++++++++-------- docshell/base/nsDocShell.h | 11 ++++---- dom/ipc/ContentParent.cpp | 4 +-- dom/ipc/ContentParent.h | 2 +- dom/ipc/PContent.ipdl | 3 +- 7 files changed, 34 insertions(+), 25 deletions(-) diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index 001461ebfac8..03f0356d874b 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -469,7 +469,8 @@ void CanonicalBrowsingContext::CallOnAllTopDescendants( void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId, const nsID& aChangeID, - uint32_t aLoadType) { + uint32_t aLoadType, + bool aPersist) { MOZ_LOG(gSHLog, LogLevel::Verbose, ("CanonicalBrowsingContext::SessionHistoryCommit %p %" PRIu64, this, aLoadId)); @@ -523,7 +524,7 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId, } if (addEntry) { - shistory->AddEntry(mActiveEntry, mActiveEntry->GetPersist()); + shistory->AddEntry(mActiveEntry, aPersist); } } } else { diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index 1ee6a4326002..b221d11e7a1a 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -125,7 +125,7 @@ class CanonicalBrowsingContext final : public BrowsingContext { aCallback); void SessionHistoryCommit(uint64_t aLoadId, const nsID& aChangeID, - uint32_t aLoadType); + uint32_t aLoadType, bool aPersist); // Calls the session history listeners' OnHistoryReload, storing the result in // aCanReload. If aCanReload is set to true and we have an active or a loading diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 512ea9dbd97b..8289fc74a66f 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -5606,7 +5606,7 @@ nsresult nsDocShell::RefreshURIFromQueue() { nsresult nsDocShell::Embed(nsIContentViewer* aContentViewer, WindowGlobalChild* aWindowActor, - bool aIsTransientAboutBlank) { + bool aIsTransientAboutBlank, bool aPersist) { // Save the LayoutHistoryState of the previous document, before // setting up new document PersistLayoutHistoryState(); @@ -5632,7 +5632,7 @@ nsresult nsDocShell::Embed(nsIContentViewer* aContentViewer, if (!aIsTransientAboutBlank && mozilla::SessionHistoryInParent()) { MOZ_LOG(gSHLog, LogLevel::Debug, ("document %p Embed", this)); - MoveLoadingToActiveEntry(); + MoveLoadingToActiveEntry(aPersist); } bool updateHistory = true; @@ -6679,7 +6679,7 @@ nsresult nsDocShell::CreateAboutBlankContentViewer( // hook 'em up if (viewer) { viewer->SetContainer(this); - rv = Embed(viewer, aActor, true); + rv = Embed(viewer, aActor, true, false); NS_ENSURE_SUCCESS(rv, rv); SetCurrentURI(blankDoc->GetDocumentURI(), nullptr, true, 0); @@ -7916,7 +7916,9 @@ nsresult nsDocShell::CreateContentViewer(const nsACString& aContentType, } } - NS_ENSURE_SUCCESS(Embed(viewer), NS_ERROR_FAILURE); + NS_ENSURE_SUCCESS(Embed(viewer, nullptr, false, + ShouldAddToSessionHistory(finalURI, aOpenedChannel)), + NS_ERROR_FAILURE); if (!mBrowsingContext->GetHasLoadedNonInitialDocument()) { MOZ_ALWAYS_SUCCEEDS(mBrowsingContext->SetHasLoadedNonInitialDocument(true)); @@ -8844,8 +8846,10 @@ nsresult nsDocShell::HandleSameDocumentNavigation( mActiveEntry = MakeUnique(mLoadingEntry->mInfo); nsID changeID = {}; if (XRE_IsParentProcess()) { + // Persist value for session history commit doesn't matter here + // since the load is from session history. mBrowsingContext->Canonical()->SessionHistoryCommit( - mLoadingEntry->mLoadId, changeID, mLoadType); + mLoadingEntry->mLoadId, changeID, mLoadType, true); } else { RefPtr rootSH = GetRootSessionHistory(); if (rootSH) { @@ -8856,8 +8860,11 @@ nsresult nsDocShell::HandleSameDocumentNavigation( changeID); } ContentChild* cc = ContentChild::GetSingleton(); - mozilla::Unused << cc->SendHistoryCommit( - mBrowsingContext, mLoadingEntry->mLoadId, changeID, mLoadType); + // Persist value for session history commit doesn't matter here + // since the load is from session history. + mozilla::Unused << cc->SendHistoryCommit(mBrowsingContext, + mLoadingEntry->mLoadId, + changeID, mLoadType, true); } // FIXME Need to set postdata. SetCacheKeyOnHistoryEntry(nullptr, cacheKey); @@ -13231,7 +13238,7 @@ void nsDocShell::SetLoadingSessionHistoryInfo( mLoadingEntry = MakeUnique(aLoadingInfo); } -void nsDocShell::MoveLoadingToActiveEntry() { +void nsDocShell::MoveLoadingToActiveEntry(bool aPersist) { MOZ_ASSERT(mozilla::SessionHistoryInParent()); MOZ_LOG(gSHLog, LogLevel::Debug, @@ -13257,8 +13264,8 @@ void nsDocShell::MoveLoadingToActiveEntry() { uint32_t loadType = mLoadType == LOAD_ERROR_PAGE ? mFailedLoadType : mLoadType; if (XRE_IsParentProcess()) { - mBrowsingContext->Canonical()->SessionHistoryCommit(loadingEntry->mLoadId, - changeID, loadType); + mBrowsingContext->Canonical()->SessionHistoryCommit( + loadingEntry->mLoadId, changeID, loadType, aPersist); } else { RefPtr rootSH = GetRootSessionHistory(); if (rootSH) { @@ -13285,8 +13292,9 @@ void nsDocShell::MoveLoadingToActiveEntry() { } } ContentChild* cc = ContentChild::GetSingleton(); - mozilla::Unused << cc->SendHistoryCommit( - mBrowsingContext, loadingEntry->mLoadId, changeID, loadType); + mozilla::Unused << cc->SendHistoryCommit(mBrowsingContext, + loadingEntry->mLoadId, changeID, + loadType, aPersist); } } } diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 780ea98730bb..e00f41b5bd46 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -960,8 +960,8 @@ class nsDocShell final : public nsDocLoader, nsresult EnsureCommandHandler(); nsresult RefreshURIFromQueue(); nsresult Embed(nsIContentViewer* aContentViewer, - mozilla::dom::WindowGlobalChild* aWindowActor = nullptr, - bool aIsTransientAboutBlank = false); + mozilla::dom::WindowGlobalChild* aWindowActor, + bool aIsTransientAboutBlank, bool aPersist); nsPresContext* GetEldestPresContext(); nsresult CheckLoadingPermissions(); nsresult LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType); @@ -1047,10 +1047,9 @@ class nsDocShell final : public nsDocLoader, nsresult LoadURI(nsDocShellLoadState* aLoadState, bool aSetNavigating, bool aContinueHandlingSubframeHistory); - // Sets the active entry to the current loading entry. If aCommit is true then - // SessionHistoryCommit will be called on the CanonicalBrowsingContext - // (directly or over IPC). - void MoveLoadingToActiveEntry(); + // Sets the active entry to the current loading entry. aPersist is used in the + // case a new session history entry is added to the session history. + void MoveLoadingToActiveEntry(bool aPersist); void ActivenessMaybeChanged(); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 25b693c2f9e0..b52b43eb8505 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -6994,10 +6994,10 @@ mozilla::ipc::IPCResult ContentParent::RecvNotifyOnHistoryReload( mozilla::ipc::IPCResult ContentParent::RecvHistoryCommit( const MaybeDiscarded& aContext, const uint64_t& aLoadID, - const nsID& aChangeID, const uint32_t& aLoadType) { + const nsID& aChangeID, const uint32_t& aLoadType, const bool& aPersist) { if (!aContext.IsDiscarded()) { aContext.get_canonical()->SessionHistoryCommit(aLoadID, aChangeID, - aLoadType); + aLoadType, aPersist); } return IPC_OK(); diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 17cca75a5bcb..34616690bb79 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1334,7 +1334,7 @@ class ContentParent final mozilla::ipc::IPCResult RecvHistoryCommit( const MaybeDiscarded& aContext, const uint64_t& aLoadID, - const nsID& aChangeID, const uint32_t& aLoadType); + const nsID& aChangeID, const uint32_t& aLoadType, const bool& aPersist); mozilla::ipc::IPCResult RecvHistoryGo( const MaybeDiscarded& aContext, int32_t aOffset, diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index d332cf83f648..036cbdaf2bfe 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1691,7 +1691,8 @@ parent: bool? reloadActiveEntry); async HistoryCommit(MaybeDiscardedBrowsingContext aContext, - uint64_t aLoadID, nsID aChangeID, uint32_t aLoadType); + uint64_t aLoadID, nsID aChangeID, uint32_t aLoadType, + bool aPersist); async HistoryGo(MaybeDiscardedBrowsingContext aContext, int32_t aOffset, uint64_t aHistoryEpoch, bool aRequireUserInteraction) returns(int32_t requestedIndex);