From cb6768b9eff43f26b49a420cdd685faef33ba489 Mon Sep 17 00:00:00 2001 From: Kashav Madan Date: Wed, 16 Jun 2021 14:05:53 +0000 Subject: [PATCH] Bug 1716444 - Don't do final tab state flush for replaced contexts, r=farre,annyG If we're destroying the frame loader of a replaced browsing context we'll end up firing browser-shutdown-tabstate-updated for a tab that wasn't actually closed. This results in us cleaning up Session Store state earlier than expected, which means we drop future updates to SessionStoreInternal._closedTabs. Fixes browser_sessionHistory.js, browser_async_remove_tab.js, and possibly browser_491168.js for SHIP+BFCache. Differential Revision: https://phabricator.services.mozilla.com/D117944 --- browser/components/sessionstore/SessionStore.jsm | 10 +--------- docshell/base/CanonicalBrowsingContext.cpp | 5 +++++ docshell/base/CanonicalBrowsingContext.h | 4 ++++ dom/base/nsFrameLoader.cpp | 2 +- dom/chrome-webidl/BrowsingContext.webidl | 2 ++ 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/browser/components/sessionstore/SessionStore.jsm b/browser/components/sessionstore/SessionStore.jsm index 99166829f4a8..3f65f7a03245 100644 --- a/browser/components/sessionstore/SessionStore.jsm +++ b/browser/components/sessionstore/SessionStore.jsm @@ -1336,15 +1336,7 @@ var SessionStoreInternal = { return; } - // XXX(farre): This could possibly happen for replaced contexts, - // both for when aBrowsingContext is itself a top level context - // and when it is a sub-frame. This check should really be covered - // by the previous epoch check, but we don't currently trust the - // epochs to fix that. - if ( - aBrowser.browsingContext && - aBrowser.browsingContext.top != aBrowsingContext.top - ) { + if (aBrowsingContext.isReplaced) { return; } diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index c82c8569d89f..b09e690d7617 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -224,10 +224,15 @@ void CanonicalBrowsingContext::ReplacedBy( MOZ_ASSERT(!aNewContext->mWebProgress); MOZ_ASSERT(!aNewContext->mSessionHistory); MOZ_ASSERT(IsTop() && aNewContext->IsTop()); + + mIsReplaced = true; + aNewContext->mIsReplaced = false; + if (mStatusFilter) { mStatusFilter->RemoveProgressListener(mDocShellProgressBridge); mStatusFilter = nullptr; } + mWebProgress->ContextReplaced(aNewContext); aNewContext->mWebProgress = std::move(mWebProgress); diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index e21d16da0535..cc30d65654d2 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -325,6 +325,8 @@ class CanonicalBrowsingContext final : public BrowsingContext { void SetTouchEventsOverride(dom::TouchEventsOverride, ErrorResult& aRv); + bool IsReplaced() const { return mIsReplaced; } + protected: // Called when the browsing context is being discarded. void CanonicalDiscard(); @@ -478,6 +480,8 @@ class CanonicalBrowsingContext final : public BrowsingContext { bool mPriorityActive = false; nsCOMPtr mSessionStoreSessionStorageUpdateTimer; + + bool mIsReplaced = false; }; } // namespace dom diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index c4f645b32e63..762bc4c6effd 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -3239,7 +3239,7 @@ already_AddRefed nsFrameLoader::RequestTabStateFlush( void nsFrameLoader::RequestFinalTabStateFlush() { BrowsingContext* context = GetExtantBrowsingContext(); - if (!context || !context->IsTop()) { + if (!context || !context->IsTop() || context->Canonical()->IsReplaced()) { return; } diff --git a/dom/chrome-webidl/BrowsingContext.webidl b/dom/chrome-webidl/BrowsingContext.webidl index d2f226388764..4b98e382f0d2 100644 --- a/dom/chrome-webidl/BrowsingContext.webidl +++ b/dom/chrome-webidl/BrowsingContext.webidl @@ -305,6 +305,8 @@ interface CanonicalBrowsingContext : BrowsingContext { * are available in a specific BrowsingContext and its descendents. */ [SetterThrows] inherit attribute TouchEventsOverride touchEventsOverride; + + readonly attribute boolean isReplaced; }; [Exposed=Window, ChromeOnly]