Bug 1673702 - Wrong page loaded when reloading after navigating back across an eTLD+1 boundary. r=smaug
authorPeter Van der Beken <peterv@propagandism.org>
Fri, 30 Oct 2020 23:49:14 +0000
changeset 555325 e9fceec56dc8c28e304e79a17b82ac054069d7ca
parent 555324 1ee5fddc1caf51575d9c31bfd65d37873ae0050f
child 555326 767e71a480eb5915ef392f5835368afe3989701c
push id129897
push userpvanderbeken@mozilla.com
push dateFri, 30 Oct 2020 23:52:03 +0000
treeherderautoland@e9fceec56dc8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1673702
milestone84.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 1673702 - Wrong page loaded when reloading after navigating back across an eTLD+1 boundary. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D95408
docshell/base/CanonicalBrowsingContext.cpp
docshell/base/nsDocShellLoadState.cpp
docshell/shistory/ChildSHistory.cpp
docshell/shistory/SessionHistoryEntry.cpp
docshell/test/browser/browser.ini
docshell/test/browser/browser_bug1673702.js
docshell/test/browser/file_bug1673702.json
docshell/test/browser/file_bug1673702.json^headers^
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -426,16 +426,19 @@ CanonicalBrowsingContext::ReplaceLoading
   }
 
   return MakeUnique<LoadingSessionHistoryInfo>(newEntry, aInfo->mLoadId);
 }
 
 void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
                                                     const nsID& aChangeID,
                                                     uint32_t aLoadType) {
+  MOZ_LOG(gSHLog, LogLevel::Verbose,
+          ("CanonicalBrowsingContext::SessionHistoryCommit %p %" PRIu64, this,
+           aLoadId));
   for (size_t i = 0; i < mLoadingEntries.Length(); ++i) {
     if (mLoadingEntries[i].mLoadId == aLoadId) {
       nsSHistory* shistory = static_cast<nsSHistory*>(GetSessionHistory());
       if (!shistory) {
         SessionHistoryEntry::RemoveLoadId(aLoadId);
         mLoadingEntries.RemoveElementAt(i);
         return;
       }
--- a/docshell/base/nsDocShellLoadState.cpp
+++ b/docshell/base/nsDocShellLoadState.cpp
@@ -113,17 +113,22 @@ nsDocShellLoadState::nsDocShellLoadState
       mHasValidUserGestureActivation(aOther.mHasValidUserGestureActivation),
       mTypeHint(aOther.mTypeHint),
       mFileName(aOther.mFileName),
       mIsFromProcessingFrameAttributes(aOther.mIsFromProcessingFrameAttributes),
       mPendingRedirectedChannel(aOther.mPendingRedirectedChannel),
       mOriginalURIString(aOther.mOriginalURIString),
       mCancelContentJSEpoch(aOther.mCancelContentJSEpoch),
       mLoadIdentifier(aOther.mLoadIdentifier),
-      mChannelInitialized(aOther.mChannelInitialized) {}
+      mChannelInitialized(aOther.mChannelInitialized) {
+  if (aOther.mLoadingSessionHistoryInfo) {
+    mLoadingSessionHistoryInfo = MakeUnique<LoadingSessionHistoryInfo>(
+        *aOther.mLoadingSessionHistoryInfo);
+  }
+}
 
 nsDocShellLoadState::nsDocShellLoadState(nsIURI* aURI, uint64_t aLoadIdentifier)
     : mURI(aURI),
       mResultPrincipalURIIsSome(false),
       mTriggeringSandboxFlags(0),
       mKeepResultPrincipalURIIfSet(false),
       mLoadReplace(false),
       mInheritPrincipal(false),
--- a/docshell/shistory/ChildSHistory.cpp
+++ b/docshell/shistory/ChildSHistory.cpp
@@ -132,18 +132,19 @@ bool ChildSHistory::CanGo(int32_t aOffse
     return false;
   }
   return index.value() < Count() && index.value() >= 0;
 }
 
 void ChildSHistory::Go(int32_t aOffset, bool aRequireUserInteraction,
                        ErrorResult& aRv) {
   CheckedInt<int32_t> index = Index();
-  MOZ_LOG(gSHLog, LogLevel::Debug,
-          ("nsHistory::Go(%d), current index = %d", aOffset, index.value()));
+  MOZ_LOG(
+      gSHLog, LogLevel::Debug,
+      ("ChildSHistory::Go(%d), current index = %d", aOffset, index.value()));
   if (aRequireUserInteraction && aOffset != -1 && aOffset != 1) {
     NS_ERROR(
         "aRequireUserInteraction may only be used with an offset of -1 or 1");
     aRv.Throw(NS_ERROR_INVALID_ARG);
     return;
   }
 
   while (true) {
@@ -177,36 +178,36 @@ void ChildSHistory::Go(int32_t aOffset, 
   }
 
   GotoIndex(index.value(), aOffset, aRv);
 }
 
 void ChildSHistory::AsyncGo(int32_t aOffset, bool aRequireUserInteraction,
                             CallerType aCallerType, ErrorResult& aRv) {
   CheckedInt<int32_t> index = Index();
-  MOZ_LOG(
-      gSHLog, LogLevel::Debug,
-      ("nsHistory::AsyncGo(%d), current index = %d", aOffset, index.value()));
+  MOZ_LOG(gSHLog, LogLevel::Debug,
+          ("ChildSHistory::AsyncGo(%d), current index = %d", aOffset,
+           index.value()));
   nsresult rv = mBrowsingContext->CheckLocationChangeRateLimit(aCallerType);
   if (NS_FAILED(rv)) {
     MOZ_LOG(gSHLog, LogLevel::Debug, ("Rejected"));
     aRv.Throw(rv);
     return;
   }
 
   RefPtr<PendingAsyncHistoryNavigation> asyncNav =
       new PendingAsyncHistoryNavigation(this, aOffset, aRequireUserInteraction);
   mPendingNavigations.insertBack(asyncNav);
   NS_DispatchToCurrentThread(asyncNav.forget());
 }
 
 void ChildSHistory::GotoIndex(int32_t aIndex, int32_t aOffset,
                               ErrorResult& aRv) {
   MOZ_LOG(gSHLog, LogLevel::Debug,
-          ("nsHistory::GotoIndex(%d, %d), epoch %" PRIu64, aIndex, aOffset,
+          ("ChildSHistory::GotoIndex(%d, %d), epoch %" PRIu64, aIndex, aOffset,
            mHistoryEpoch));
   if (mozilla::SessionHistoryInParent()) {
     nsCOMPtr<nsISHistory> shistory = mHistory;
     mBrowsingContext->HistoryGo(
         aOffset, mHistoryEpoch, [shistory](int32_t&& aRequestedIndex) {
           // FIXME Should probably only do this for non-fission.
           if (shistory) {
             shistory->InternalSetRequestedIndex(aRequestedIndex);
@@ -218,19 +219,19 @@ void ChildSHistory::GotoIndex(int32_t aI
 }
 
 void ChildSHistory::RemovePendingHistoryNavigations() {
   // Per the spec, this generally shouldn't remove all navigations - it
   // depends if they're in the same document family or not.  We don't do
   // that.  Also with SessionHistoryInParent, this can only abort AsyncGo's
   // that have not yet been sent to the parent - see discussion of point
   // 2.2 in comments in nsDocShell::UpdateURLAndHistory()
-  MOZ_LOG(
-      gSHLog, LogLevel::Debug,
-      ("RemovePendingHistoryNavigations: %zu", mPendingNavigations.length()));
+  MOZ_LOG(gSHLog, LogLevel::Debug,
+          ("ChildSHistory::RemovePendingHistoryNavigations: %zu",
+           mPendingNavigations.length()));
   mPendingNavigations.clear();
 }
 
 void ChildSHistory::EvictLocalContentViewers() {
   if (!mozilla::SessionHistoryInParent()) {
     mHistory->EvictAllContentViewers();
   }
 }
--- a/docshell/shistory/SessionHistoryEntry.cpp
+++ b/docshell/shistory/SessionHistoryEntry.cpp
@@ -356,17 +356,17 @@ void SessionHistoryEntry::SetByLoadId(ui
                                       SessionHistoryEntry* aEntry) {
   if (!sLoadIdToEntry) {
     sLoadIdToEntry =
         new nsDataHashtable<nsUint64HashKey, SessionHistoryEntry*>();
   }
 
   MOZ_LOG(
       gSHLog, LogLevel::Verbose,
-      ("SessionHistoryEntry::SetByLoadId(%" PRIu64 " - %p", aLoadId, aEntry));
+      ("SessionHistoryEntry::SetByLoadId(%" PRIu64 " - %p)", aLoadId, aEntry));
   sLoadIdToEntry->Put(aLoadId, aEntry);
 }
 
 void SessionHistoryEntry::RemoveLoadId(uint64_t aLoadId) {
   MOZ_ASSERT(XRE_IsParentProcess());
   if (!sLoadIdToEntry) {
     return;
   }
--- a/docshell/test/browser/browser.ini
+++ b/docshell/test/browser/browser.ini
@@ -86,16 +86,20 @@ skip-if = fission
 [browser_bug1347823.js]
 skip-if = fission
 [browser_bug134911.js]
 [browser_bug1415918_beforeunload_options.js]
 [browser_bug1622420.js]
 support-files =
   file_bug1622420.html
   Bug1622420Child.jsm
+[browser_bug1673702.js]
+support-files =
+  file_bug1673702.json
+  file_bug1673702.json^headers^
 [browser_bug234628-1.js]
 [browser_bug234628-10.js]
 [browser_bug234628-11.js]
 [browser_bug234628-2.js]
 [browser_bug234628-3.js]
 [browser_bug234628-4.js]
 [browser_bug234628-5.js]
 [browser_bug234628-6.js]
new file mode 100644
--- /dev/null
+++ b/docshell/test/browser/browser_bug1673702.js
@@ -0,0 +1,24 @@
+const DUMMY =
+  "http://example.org/browser/docshell/test/browser/dummy_page.html";
+const JSON =
+  "http://example.com/browser/docshell/test/browser/file_bug1673702.json";
+
+add_task(async function test_backAndReload() {
+  await BrowserTestUtils.withNewTab({ gBrowser, url: DUMMY }, async function(
+    browser
+  ) {
+    info("Start JSON load.");
+    BrowserTestUtils.loadURI(browser, JSON);
+    await BrowserTestUtils.waitForLocationChange(gBrowser, JSON);
+
+    info("JSON load has started, go back.");
+    browser.goBack();
+    await BrowserTestUtils.browserStopped(browser);
+
+    info("Reload.");
+    BrowserReload();
+    await BrowserTestUtils.waitForLocationChange(gBrowser);
+
+    is(browser.documentURI.spec, DUMMY);
+  });
+});
new file mode 100644
--- /dev/null
+++ b/docshell/test/browser/file_bug1673702.json
@@ -0,0 +1,1 @@
+{ "version": 1, }
new file mode 100644
--- /dev/null
+++ b/docshell/test/browser/file_bug1673702.json^headers^
@@ -0,0 +1,1 @@
+Content-Type: application/json; charset=utf-8