Bug 1816234 - Fix meta refresh with SHIP. r=smaug

BrowsingContext::ShouldAddEntryForRefresh expects to be passed the current URI,
to determine if it's the same as the URI for the new load (we shouldn't add a
new entry if they are the same). Before this patch, in the child process we call
BrowsingContext::ShouldAddEntryForRefresh from nsDocShell::MoveLoadingToActiveEntry
(nsDocShell::CreateContentViewer -> Embed -> MoveLoadingToActiveEntry), passing
nsDocShell's mCurrentURI. However, by that point nsDocShell::CreateContentViewer
has already set the mCurrentURI to the newly loading URI (through
nsDocShell::OnNewURI). In the parent process we do pass the correct URI to
BrowsingContext::ShouldAddEntryForRefresh, so the session history ends up being
correct, but depending on timing of the IPC messages there's a brief moment where
index and length are incorrect in the child process (child process and parent
process get a different answer from BrowsingContext::ShouldAddEntryForRefresh).
This causes intermittent failures in the test at
docshell/test/mochitest/test_bug1742865.html.

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

docshell/base/BrowsingContext.cpp

@@ -3605,13 +3605,13 @@ bool BrowsingContext::IsPopupAllowed() {
 
 /* static */
 bool BrowsingContext::ShouldAddEntryForRefresh(
-    nsIURI* aCurrentURI, const SessionHistoryInfo& aInfo) {
-  return ShouldAddEntryForRefresh(aCurrentURI, aInfo.GetURI(),
+    nsIURI* aPreviousURI, const SessionHistoryInfo& aInfo) {
+  return ShouldAddEntryForRefresh(aPreviousURI, aInfo.GetURI(),
                                   aInfo.HasPostData());
 }
 
 /* static */
-bool BrowsingContext::ShouldAddEntryForRefresh(nsIURI* aCurrentURI,
+bool BrowsingContext::ShouldAddEntryForRefresh(nsIURI* aPreviousURI,
                                                nsIURI* aNewURI,
                                                bool aHasPostData) {
   if (aHasPostData) {
@@ -3619,15 +3619,15 @@ bool BrowsingContext::ShouldAddEntryForRefresh(nsIURI* aCurrentURI,
   }
 
   bool equalsURI = false;
-  if (aCurrentURI) {
-    aCurrentURI->Equals(aNewURI, &equalsURI);
+  if (aPreviousURI) {
+    aPreviousURI->Equals(aNewURI, &equalsURI);
   }
   return !equalsURI;
 }
 
 void BrowsingContext::SessionHistoryCommit(
     const LoadingSessionHistoryInfo& aInfo, uint32_t aLoadType,
-    nsIURI* aCurrentURI, bool aHadActiveEntry, bool aPersist,
+    nsIURI* aPreviousURI, bool aHadActiveEntry, bool aPersist,
     bool aCloneEntryChildren, bool aChannelExpired, uint32_t aCacheKey) {
   nsID changeID = {};
   if (XRE_IsContentProcess()) {
@@ -3648,7 +3648,7 @@ void BrowsingContext::SessionHistoryCommit(
             ShouldUpdateSessionHistory(aLoadType) &&
             (!LOAD_TYPE_HAS_FLAGS(aLoadType,
                                   nsIWebNavigation::LOAD_FLAGS_IS_REFRESH) ||
-             ShouldAddEntryForRefresh(aCurrentURI, aInfo.mInfo))) {
+             ShouldAddEntryForRefresh(aPreviousURI, aInfo.mInfo))) {
           changeID = rootSH->AddPendingHistoryChange();
         }
       } else {

docshell/base/nsDocShell.cpp

@@ -5708,7 +5708,7 @@ static bool IsFollowupPartOfMultipart(nsIRequest* aRequest) {
 nsresult nsDocShell::Embed(nsIContentViewer* aContentViewer,
                            WindowGlobalChild* aWindowActor,
                            bool aIsTransientAboutBlank, bool aPersist,
-                           nsIRequest* aRequest) {
+                           nsIRequest* aRequest, nsIURI* aPreviousURI) {
   // Save the LayoutHistoryState of the previous document, before
   // setting up new document
   PersistLayoutHistoryState();
@@ -5759,7 +5759,7 @@ nsresult nsDocShell::Embed(nsIContentViewer* aContentViewer,
     }
 
     MOZ_LOG(gSHLog, LogLevel::Debug, ("document %p Embed", this));
-    MoveLoadingToActiveEntry(aPersist, expired, cacheKey);
+    MoveLoadingToActiveEntry(aPersist, expired, cacheKey, aPreviousURI);
   }
 
   bool updateHistory = true;
@@ -6812,7 +6812,7 @@ nsresult nsDocShell::CreateAboutBlankContentViewer(
       // hook 'em up
       if (viewer) {
         viewer->SetContainer(this);
-        rv = Embed(viewer, aActor, true, false, nullptr);
+        rv = Embed(viewer, aActor, true, false, nullptr, mCurrentURI);
         NS_ENSURE_SUCCESS(rv, rv);
 
         SetCurrentURI(blankDoc->GetDocumentURI(), nullptr,
@@ -7899,6 +7899,11 @@ nsresult nsDocShell::CreateContentViewer(const nsACString& aContentType,
   if (aOpenedChannel) {
     aOpenedChannel->GetURI(getter_AddRefs(mLoadingURI));
   }
+
+  // Grab the current URI, we need to pass it to Embed, and OnNewURI will reset
+  // it before we do call Embed.
+  nsCOMPtr<nsIURI> previousURI = mCurrentURI;
+
   FirePageHideNotification(!mSavingOldViewer);
   if (mIsBeingDestroyed) {
     // Force to stop the newly created orphaned viewer.
@@ -8076,7 +8081,7 @@ nsresult nsDocShell::CreateContentViewer(const nsACString& aContentType,
 
   NS_ENSURE_SUCCESS(Embed(viewer, nullptr, false,
                           ShouldAddToSessionHistory(finalURI, aOpenedChannel),
-                          aOpenedChannel),
+                          aOpenedChannel, previousURI),
                     NS_ERROR_FAILURE);
 
   if (!mBrowsingContext->GetHasLoadedNonInitialDocument()) {
@@ -13725,7 +13730,8 @@ void nsDocShell::SetLoadingSessionHistoryInfo(
 }
 
 void nsDocShell::MoveLoadingToActiveEntry(bool aPersist, bool aExpired,
-                                          uint32_t aCacheKey) {
+                                          uint32_t aCacheKey,
+                                          nsIURI* aPreviousURI) {
   MOZ_ASSERT(mozilla::SessionHistoryInParent());
 
   MOZ_LOG(gSHLog, LogLevel::Debug,
@@ -13768,8 +13774,8 @@ void nsDocShell::MoveLoadingToActiveEntry(bool aPersist, bool aExpired,
       // does require a non-null uri if this is for a refresh load of the same
       // URI, but in that case mCurrentURI won't be null here.
       mBrowsingContext->SessionHistoryCommit(
-          *loadingEntry, loadType, mCurrentURI, hadActiveEntry, aPersist, false,
-          aExpired, aCacheKey);
+          *loadingEntry, loadType, aPreviousURI, hadActiveEntry, aPersist,
+          false, aExpired, aCacheKey);
     }
   }
 }

docshell/base/nsDocShell.h

@@ -1006,7 +1006,7 @@ class nsDocShell final : public nsDocLoader,
   nsresult Embed(nsIContentViewer* aContentViewer,
                  mozilla::dom::WindowGlobalChild* aWindowActor,
                  bool aIsTransientAboutBlank, bool aPersist,
-                 nsIRequest* aRequest);
+                 nsIRequest* aRequest, nsIURI* aPreviousURI);
   nsPresContext* GetEldestPresContext();
   nsresult CheckLoadingPermissions();
   nsresult LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType,
@@ -1104,8 +1104,10 @@ class nsDocShell final : public nsDocLoader,
   // case a new session history entry is added to the session history.
   // aExpired is true if the relevant nsIChannel has its cache token expired.
   // aCacheKey is the channel's cache key.
+  // aPreviousURI should be the URI that was previously loaded into the
+  // nsDocshell
   void MoveLoadingToActiveEntry(bool aPersist, bool aExpired,
-                                uint32_t aCacheKey);
+                                uint32_t aCacheKey, nsIURI* aPreviousURI);
 
   void ActivenessMaybeChanged();