From 4d366e18615a7f3344372937c96b88571f6e6969 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Mon, 30 Aug 2021 19:50:05 +0000 Subject: [PATCH] Bug 1723282, let nonbfcacheable page update layout history state when the new page is coming from the bfcache, r=peterv The fix let's ContentParent::RecvSynchronizeLayoutHistoryState update the layout history state. Using an existing test to launch a subtest for this. Hopefully the description of the test helps with reviewing it. (These BroadcastChannel based tests can be hard to follow.) Differential Revision: https://phabricator.services.mozilla.com/D122376 --- docshell/shistory/nsSHistory.cpp | 11 +++- ...rollRestoration_bfcache_and_nobfcache.html | 30 +++++++++ ...storation_bfcache_and_nobfcache_part2.html | 35 +++++++++++ ..._bfcache_and_nobfcache_part2.html^headers^ | 1 + docshell/test/navigation/mochitest.ini | 3 + .../navigation/test_scrollRestoration.html | 61 ++++++++++++++++++- 6 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache.html create mode 100644 docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html create mode 100644 docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html^headers^ diff --git a/docshell/shistory/nsSHistory.cpp b/docshell/shistory/nsSHistory.cpp index 2bdbf305252f..95623f6618e5 100644 --- a/docshell/shistory/nsSHistory.cpp +++ b/docshell/shistory/nsSHistory.cpp @@ -1227,9 +1227,10 @@ static void FinishRestore(CanonicalBrowsingContext* aBrowsingContext, frameLoaderOwner->GetFrameLoader(); // The current page can be bfcached, store the // nsFrameLoader in the current SessionHistoryEntry. - if (aCanSave && aBrowsingContext->GetActiveSessionHistoryEntry()) { - aBrowsingContext->GetActiveSessionHistoryEntry()->SetFrameLoader( - currentFrameLoader); + RefPtr currentSHEntry = + aBrowsingContext->GetActiveSessionHistoryEntry(); + if (aCanSave && currentSHEntry) { + currentSHEntry->SetFrameLoader(currentFrameLoader); Unused << aBrowsingContext->SetIsInBFCache(true); } @@ -1262,6 +1263,10 @@ static void FinishRestore(CanonicalBrowsingContext* aBrowsingContext, // The old page can't be stored in the bfcache, // destroy the nsFrameLoader. if (!aCanSave && currentFrameLoader) { + // Since destroying the browsing context may need to update layout + // history state, the browsing context needs to have still access to the + // correct entry. + aBrowsingContext->SetActiveSessionHistoryEntry(currentSHEntry); currentFrameLoader->Destroy(); } diff --git a/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache.html b/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache.html new file mode 100644 index 000000000000..fec51f821dc4 --- /dev/null +++ b/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache.html @@ -0,0 +1,30 @@ + + + + + + + diff --git a/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html b/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html new file mode 100644 index 000000000000..40e0578515be --- /dev/null +++ b/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html @@ -0,0 +1,35 @@ + + + + + +
content
+ + diff --git a/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html^headers^ b/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html^headers^ new file mode 100644 index 000000000000..59ba296103c7 --- /dev/null +++ b/docshell/test/navigation/file_scrollRestoration_bfcache_and_nobfcache_part2.html^headers^ @@ -0,0 +1 @@ +Cache-control: no-store diff --git a/docshell/test/navigation/mochitest.ini b/docshell/test/navigation/mochitest.ini index 586f67ebd115..1f3ba51a5be9 100644 --- a/docshell/test/navigation/mochitest.ini +++ b/docshell/test/navigation/mochitest.ini @@ -60,6 +60,9 @@ support-files = test_bug145971.html file_bug1609475.html file_bug1379762-1.html + file_scrollRestoration_bfcache_and_nobfcache.html + file_scrollRestoration_bfcache_and_nobfcache_part2.html + file_scrollRestoration_bfcache_and_nobfcache_part2.html^headers^ file_scrollRestoration_navigate.html file_scrollRestoration_part1_nobfcache.html file_scrollRestoration_part1_nobfcache.html^headers^ diff --git a/docshell/test/navigation/test_scrollRestoration.html b/docshell/test/navigation/test_scrollRestoration.html index 005e3afff3f9..d31598f391fb 100644 --- a/docshell/test/navigation/test_scrollRestoration.html +++ b/docshell/test/navigation/test_scrollRestoration.html @@ -127,7 +127,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id= currentCase++; } else if (command == "finishing") { bc3.close(); - SimpleTest.finish(); + test4(); } } @@ -142,6 +142,65 @@ https://bugzilla.mozilla.org/show_bug.cgi?id= window.open("file_scrollRestoration_part3_nobfcache.html", "", "width=360,height=480,noopener"); } + // test4 opens a new page which can enter bfcache. That page then loads + // another page which can't enter bfcache. That second page then scrolls + // down. History API is then used to navigate back and forward. When the + // second page loads again, it should scroll down automatically. + var bc4a, bc4b; + var scrollYCounter = 0; + function test4() { + currentCase = 0; + bc4a = new BroadcastChannel("bfcached"); + bc4a.onmessage = (msgEvent) => { + var msg = msgEvent.data; + var command = msg.command; + if (command == "pageshow") { + ++currentCase; + if (currentCase == 1) { + ok(!msg.persisted, "The first page should not be persisted initially."); + bc4a.postMessage("loadNext"); + } else if (currentCase == 3) { + ok(msg.persisted, "The first page should be persisted."); + bc4a.postMessage("forward"); + bc4a.close(); + } + } + } + + bc4b = new BroadcastChannel("notbfcached"); + bc4b.onmessage = (event) => { + var msg = event.data; + var command = msg.command; + if (command == "pageshow") { + ++currentCase; + if (currentCase == 2) { + ok(!msg.persisted, "The second page should not be persisted."); + bc4b.postMessage("getScrollY"); + bc4b.postMessage("scroll"); + bc4b.postMessage("getScrollY"); + bc4b.postMessage("back"); + } else if (currentCase == 4) { + ok(!msg.persisted, "The second page should not be persisted."); + bc4b.postMessage("getScrollY"); + } + } else if (msg == "closed") { + bc4b.close(); + SimpleTest.finish(); + } else if ("scrollY" in msg) { + ++scrollYCounter; + if (scrollYCounter == 1) { + is(msg.scrollY, 0, "The page should be initially scrolled to top."); + } else if (scrollYCounter == 2) { + isnot(msg.scrollY, 0, "The page should be then scrolled down."); + } else if (scrollYCounter == 3) { + isnot(msg.scrollY, 0, "The page should be scrolled down after being restored from the session history."); + bc4b.postMessage("close"); + } + } + } + window.open("file_scrollRestoration_bfcache_and_nobfcache.html", "", "width=360,height=480,noopener"); + } + function runTest() { // If Fission is disabled, the pref is no-op. SpecialPowers.pushPrefEnv({set: [["fission.bfcacheInParent", true]]}, () => {