Bug 1665322 - Error page loads should add entries for the original lo…

…ad with session history in the parent. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D90393
iaxat · Sep 20, 2020 · 45dc5ef · 45dc5ef
1 parent df104cb
commit 45dc5ef
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 34 deletions.
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -3996,6 +3996,13 @@ nsresult nsDocShell::LoadErrorPage(nsIURI* aErrorURI, nsIURI* aFailedURI,
     // identifier, the error page won't persist.
     mLSHE->AbandonBFCacheEntry();
   }
+  if (StaticPrefs::fission_sessionHistoryInParent()) {
+    // Commit the loading entry for the real load here, Embed will not commit
+    // the loading entry for the error page. History will then contain an entry
+    // for the real load, and the error page won't persist if we try loading
+    // that entry again.
+    MoveLoadingToActiveEntry(true);
+  }
 
   RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aErrorURI);
   loadState->SetTriggeringPrincipal(nsContentUtils::GetSystemPrincipal());
@@ -5705,38 +5712,7 @@ nsresult nsDocShell::Embed(nsIContentViewer* aContentViewer,
 
   if (StaticPrefs::fission_sessionHistoryInParent()) {
     MOZ_LOG(gSHLog, LogLevel::Debug, ("document %p Embed", this));
-    mActiveEntry = nullptr;
-    mozilla::UniquePtr<mozilla::dom::LoadingSessionHistoryInfo> loadingEntry;
-    mActiveEntryIsLoadingFromSessionHistory =
-        mLoadingEntry && mLoadingEntry->mLoadIsFromSessionHistory;
-    if (mLoadingEntry) {
-      mActiveEntry = MakeUnique<SessionHistoryInfo>(mLoadingEntry->mInfo);
-      mLoadingEntry.swap(loadingEntry);
-    }
-    if (mActiveEntry) {
-      MOZ_ASSERT(loadingEntry);
-      nsID changeID = {};
-      if (XRE_IsParentProcess()) {
-        mBrowsingContext->Canonical()->SessionHistoryCommit(
-            loadingEntry->mLoadId, changeID, mLoadType);
-      } else {
-        RefPtr<ChildSHistory> rootSH = GetRootSessionHistory();
-        if (rootSH) {
-          if (!loadingEntry->mLoadIsFromSessionHistory) {
-            changeID = rootSH->AddPendingHistoryChange();
-          } else {
-            // This is a load from session history, so we can update
-            // index and length immediately.
-            rootSH->SetIndexAndLength(loadingEntry->mRequestedIndex,
-                                      loadingEntry->mSessionHistoryLength,
-                                      changeID);
-          }
-        }
-        ContentChild* cc = ContentChild::GetSingleton();
-        mozilla::Unused << cc->SendHistoryCommit(
-            mBrowsingContext, loadingEntry->mLoadId, changeID, mLoadType);
-      }
-    }
+    MoveLoadingToActiveEntry(mLoadType != LOAD_ERROR_PAGE);
   }
 
   bool updateHistory = true;
@@ -8896,6 +8872,10 @@ nsresult nsDocShell::HandleSameDocumentNavigation(
     }
   }
   if (StaticPrefs::fission_sessionHistoryInParent() && mLoadingEntry) {
+    MOZ_LOG(
+        gSHLog, LogLevel::Debug,
+        ("Moving the loading entry to the active entry on nsDocShell %p to %s",
+         this, mLoadingEntry->mInfo.GetURI()->GetSpecOrDefault().get()));
     mActiveEntry = MakeUnique<SessionHistoryInfo>(mLoadingEntry->mInfo);
     nsID changeID = {};
     if (XRE_IsParentProcess()) {
@@ -9961,8 +9941,7 @@ nsresult nsDocShell::DoURILoad(nsDocShellLoadState* aLoadState,
   //       DocumentLoadListener, so probably need to create session history info
   //       in more places.
   if (aLoadState->GetLoadingSessionHistoryInfo()) {
-    mLoadingEntry = MakeUnique<LoadingSessionHistoryInfo>(
-        *aLoadState->GetLoadingSessionHistoryInfo());
+    SetLoadingSessionHistoryInfo(*aLoadState->GetLoadingSessionHistoryInfo());
   }
 
   // open a channel for the url
@@ -11537,6 +11516,9 @@ void nsDocShell::UpdateActiveEntry(
   MOZ_ASSERT_IF(aPreviousScrollPos.isSome(), !aReplace);
 
   if (!aReplace || !mActiveEntry) {
+    MOZ_LOG(gSHLog, LogLevel::Debug,
+            ("Creating an active entry on nsDocShell %p to %s", this,
+             aURI->GetSpecOrDefault().get()));
     if (mActiveEntry) {
       // Link this entry to the previous active entry.
       mActiveEntry =
@@ -13217,5 +13199,52 @@ void nsDocShell::SetLoadingSessionHistoryInfo(
     const mozilla::dom::LoadingSessionHistoryInfo& aLoadingInfo) {
   // FIXME Would like to assert this, but can't yet.
   // MOZ_ASSERT(!mLoadingEntry);
+  MOZ_LOG(gSHLog, LogLevel::Debug,
+          ("Setting the loading entry on nsDocShell %p to %s", this,
+           aLoadingInfo.mInfo.GetURI()->GetSpecOrDefault().get()));
   mLoadingEntry = MakeUnique<LoadingSessionHistoryInfo>(aLoadingInfo);
 }
+
+void nsDocShell::MoveLoadingToActiveEntry(bool aCommit) {
+  MOZ_ASSERT(StaticPrefs::fission_sessionHistoryInParent());
+
+  MOZ_LOG(gSHLog, LogLevel::Debug,
+          ("nsDocShell %p MoveLoadingToActiveEntry", this));
+
+  mActiveEntry = nullptr;
+  mozilla::UniquePtr<mozilla::dom::LoadingSessionHistoryInfo> loadingEntry;
+  mActiveEntryIsLoadingFromSessionHistory = !!mLoadingEntry;
+  if (mLoadingEntry) {
+    MOZ_LOG(gSHLog, LogLevel::Debug,
+            ("Moving the loading entry to the active entry on nsDocShell %p "
+             "to %s",
+             this, mLoadingEntry->mInfo.GetURI()->GetSpecOrDefault().get()));
+    mActiveEntry = MakeUnique<SessionHistoryInfo>(mLoadingEntry->mInfo);
+    mLoadingEntry.swap(loadingEntry);
+  }
+
+  if (mActiveEntry && aCommit) {
+    MOZ_ASSERT(loadingEntry);
+    nsID changeID = {};
+    if (XRE_IsParentProcess()) {
+      mBrowsingContext->Canonical()->SessionHistoryCommit(loadingEntry->mLoadId,
+                                                          changeID, mLoadType);
+    } else {
+      RefPtr<ChildSHistory> rootSH = GetRootSessionHistory();
+      if (rootSH) {
+        if (!loadingEntry->mLoadIsFromSessionHistory) {
+          changeID = rootSH->AddPendingHistoryChange();
+        } else {
+          // This is a load from session history, so we can update
+          // index and length immediately.
+          rootSH->SetIndexAndLength(loadingEntry->mRequestedIndex,
+                                    loadingEntry->mSessionHistoryLength,
+                                    changeID);
+        }
+      }
+      ContentChild* cc = ContentChild::GetSingleton();
+      mozilla::Unused << cc->SendHistoryCommit(
+          mBrowsingContext, loadingEntry->mLoadId, changeID, mLoadType);
+    }
+  }
+}
diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h
@@ -1095,6 +1095,11 @@ 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(bool aCommit);
+
  private:  // data members
   nsID mHistoryID;
   nsString mTitle;