Bug 1720733 - Try to ensure SessionHistoryEntry::mFrameLoader isn't replaced accidentally, r=peterv
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Sat, 17 Jul 2021 09:34:46 +0000
changeset 585777 b68a422ad6202e5356d0cef5d9b86db5f37f2d88
parent 585776 27ac62ded6e0cae32684e67e6b16192e4c43092d
child 585778 047c36f483388bf15a636805c1387b1af815b021
push id38620
push usercsabou@mozilla.com
push dateSun, 18 Jul 2021 09:08:29 +0000
treeherdermozilla-central@cc4e5ea0c986 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1720733
milestone92.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1720733 - Try to ensure SessionHistoryEntry::mFrameLoader isn't replaced accidentally, r=peterv I don't have a test for this, since the race condition is super hard to trigger. The patch is based on code auditing. The patch just prevents bfcaching if we're in middle of another load. Differential Revision: https://phabricator.services.mozilla.com/D119992
docshell/shistory/SessionHistoryEntry.cpp
docshell/shistory/nsSHistory.cpp
--- a/docshell/shistory/SessionHistoryEntry.cpp
+++ b/docshell/shistory/SessionHistoryEntry.cpp
@@ -1366,17 +1366,17 @@ void SessionHistoryEntry::ReplaceWith(co
   mChildren.Clear();
 }
 
 SHEntrySharedParentState* SessionHistoryEntry::SharedInfo() const {
   return static_cast<SHEntrySharedParentState*>(mInfo->mSharedState.Get());
 }
 
 void SessionHistoryEntry::SetFrameLoader(nsFrameLoader* aFrameLoader) {
-  MOZ_ASSERT_IF(aFrameLoader, !SharedInfo()->mFrameLoader);
+  MOZ_DIAGNOSTIC_ASSERT(!aFrameLoader || !SharedInfo()->mFrameLoader);
   // If the pref is disabled, we still allow evicting the existing entries.
   MOZ_RELEASE_ASSERT(!aFrameLoader || mozilla::BFCacheInParent());
   SharedInfo()->SetFrameLoader(aFrameLoader);
   if (aFrameLoader) {
     if (BrowserParent* bp = aFrameLoader->GetBrowserParent()) {
       bp->Deactivated();
     }
 
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -1251,17 +1251,18 @@ void nsSHistory::LoadURIOrBFCache(LoadEn
     RefPtr<CanonicalBrowsingContext> canonicalBC =
         aLoadEntry.mBrowsingContext->Canonical();
     nsCOMPtr<SessionHistoryEntry> she = do_QueryInterface(loadState->SHEntry());
     nsCOMPtr<SessionHistoryEntry> currentShe =
         canonicalBC->GetActiveSessionHistoryEntry();
     MOZ_ASSERT(she);
     RefPtr<nsFrameLoader> frameLoader = she->GetFrameLoader();
     if (frameLoader &&
-        (!currentShe || she->SharedInfo() != currentShe->SharedInfo())) {
+        (!currentShe || (she->SharedInfo() != currentShe->SharedInfo() &&
+                         !currentShe->GetFrameLoader()))) {
       bool canSave = (!currentShe || currentShe->GetSaveLayoutStateFlag()) &&
                      canonicalBC->AllowedInBFCache(Nothing());
 
       MOZ_LOG(gSHIPBFCacheLog, LogLevel::Debug,
               ("nsSHistory::LoadURIOrBFCache "
                "saving presentation=%i",
                canSave));