Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-17 07:15:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1725680, requested index should be used only by the nsSHistory (and related code in CanonicalBrowsingContext), r=peterv

Browse Source 
Using requestedIndex on the child side is hard, because there are race conditions when a session history load is triggered
and at the same time a non-session history load commits a new active entry.

Differential Revision: https://phabricator.services.mozilla.com/D126619
This commit is contained in:
Olli Pettay 2021-09-29 13:22:34 +00:00
parent 6901d2f440
commit 0ca720d7a3
18 changed files with 150 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
docshell/base/BrowsingContext.cpp
View File

@ -3426,10 +3426,8 @@ void BrowsingContext::SessionHistoryCommit(
changeID = rootSH->AddPendingHistoryChange();
}
} else {
// This is a load from session history, so we can update
// index and length immediately.
rootSH->SetIndexAndLength(aInfo.mRequestedIndex,
aInfo.mSessionHistoryLength, changeID);
// History load doesn't change the length, only index.
changeID = rootSH->AddPendingHistoryChange(aInfo.mOffset, 0);
}
}
ContentChild* cc = ContentChild::GetSingleton();

37
docshell/base/CanonicalBrowsingContext.cpp
View File

@ -487,11 +487,7 @@ void CanonicalBrowsingContext::AddLoadingSessionHistoryEntry(
}
void CanonicalBrowsingContext::GetLoadingSessionHistoryInfoFromParent(
Maybe<LoadingSessionHistoryInfo>& aLoadingInfo, int32_t* aRequestedIndex,
int32_t* aLength) {
*aRequestedIndex = -1;
*aLength = 0;
Maybe<LoadingSessionHistoryInfo>& aLoadingInfo) {
nsISHistory* shistory = GetSessionHistory();
if (!shistory || !GetParent()) {
return;
@ -512,8 +508,6 @@ void CanonicalBrowsingContext::GetLoadingSessionHistoryInfoFromParent(
aLoadingInfo.emplace(she);
mLoadingEntries.AppendElement(LoadingSessionHistoryEntry{
aLoadingInfo.value().mLoadId, she.get()});
*aRequestedIndex = shistory->GetRequestedIndex();
*aLength = shistory->GetCount();
Unused << SetHistoryID(she->DocshellID());
}
break;
@ -758,8 +752,6 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
return;
}
CallerWillNotifyHistoryIndexAndLengthChanges caller(shistory);
RefPtr<SessionHistoryEntry> newActiveEntry = mLoadingEntries[i].mEntry;
bool loadFromSessionHistory = !newActiveEntry->ForInitialLoad();
@ -767,6 +759,18 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
SessionHistoryEntry::RemoveLoadId(aLoadId);
mLoadingEntries.RemoveElementAt(i);
int32_t indexOfHistoryLoad = -1;
if (loadFromSessionHistory) {
nsCOMPtr<nsISHEntry> root = nsSHistory::GetRootSHEntry(newActiveEntry);
indexOfHistoryLoad = shistory->GetIndexOfEntry(root);
if (indexOfHistoryLoad < 0) {
// Entry has been removed from the session history.
return;
}
}
CallerWillNotifyHistoryIndexAndLengthChanges caller(shistory);
// If there is a name in the new entry, clear the name of all contiguous
// entries. This is for https://html.spec.whatwg.org/#history-traversal
// Step 4.4.2.
@ -806,6 +810,7 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
if (loadFromSessionHistory) {
// XXX Synchronize browsing context tree and session history tree?
shistory->InternalSetRequestedIndex(indexOfHistoryLoad);
shistory->UpdateIndex();
} else if (addEntry) {
shistory->AddEntry(mActiveEntry, aPersist);
@ -824,6 +829,8 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
this);
}
mActiveEntry = newActiveEntry;
shistory->InternalSetRequestedIndex(indexOfHistoryLoad);
// FIXME UpdateIndex() here may update index too early (but even the
// old implementation seems to have similar issues).
shistory->UpdateIndex();
@ -921,15 +928,9 @@ void CanonicalBrowsingContext::NotifyOnHistoryReload(
}
if (aLoadState) {
int32_t index = 0;
int32_t requestedIndex = -1;
int32_t length = 0;
shistory->GetIndex(&index);
shistory->GetRequestedIndex(&requestedIndex);
shistory->GetCount(&length);
aLoadState.ref()->SetLoadIsFromSessionHistory(
requestedIndex >= 0 ? requestedIndex : index, length,
aReloadActiveEntry.value());
// Use 0 as the offset, since aLoadState will be be used for reload.
aLoadState.ref()->SetLoadIsFromSessionHistory(0,
aReloadActiveEntry.value());
}
// If we don't have an active entry and we don't have a loading entry then
// the nsDocShell will create a load state based on its document.

3
docshell/base/CanonicalBrowsingContext.h
View File

@ -274,8 +274,7 @@ class CanonicalBrowsingContext final : public BrowsingContext {
SessionHistoryEntry* aEntry);
void GetLoadingSessionHistoryInfoFromParent(
Maybe<LoadingSessionHistoryInfo>& aLoadingInfo, int32_t* aRequestedIndex,
int32_t* aLength);
Maybe<LoadingSessionHistoryInfo>& aLoadingInfo);
void HistoryCommitIndexAndLength();

23
docshell/base/nsDocShell.cpp
View File

@ -919,15 +919,15 @@ bool nsDocShell::MaybeHandleSubframeHistory(
auto resolve =
[currentLoadIdentifier, browsingContext, parentDoc, loadState,
isNavigating](Tuple<mozilla::Maybe<LoadingSessionHistoryInfo>,
int32_t, int32_t>&& aResult) {
isNavigating](
mozilla::Maybe<LoadingSessionHistoryInfo>&& aResult) {
if (currentLoadIdentifier ==
browsingContext->GetCurrentLoadIdentifier() &&
Get<0>(aResult).isSome()) {
loadState->SetLoadingSessionHistoryInfo(
Get<0>(aResult).value());
loadState->SetLoadIsFromSessionHistory(
Get<1>(aResult), Get<2>(aResult), false);
aResult.isSome()) {
loadState->SetLoadingSessionHistoryInfo(aResult.value());
// This is an initial subframe load from the session
// history, index doesn't need to be updated.
loadState->SetLoadIsFromSessionHistory(0, false);
}
RefPtr<nsDocShell> docShell =
static_cast<nsDocShell*>(browsingContext->GetDocShell());
@ -947,14 +947,13 @@ bool nsDocShell::MaybeHandleSubframeHistory(
}
} else {
Maybe<LoadingSessionHistoryInfo> info;
int32_t requestedIndex = -1;
int32_t sessionHistoryLength = 0;
mBrowsingContext->Canonical()->GetLoadingSessionHistoryInfoFromParent(
info, &requestedIndex, &sessionHistoryLength);
info);
if (info.isSome()) {
aLoadState->SetLoadingSessionHistoryInfo(info.value());
aLoadState->SetLoadIsFromSessionHistory(
requestedIndex, sessionHistoryLength, false);
// This is an initial subframe load from the session
// history, index doesn't need to be updated.
aLoadState->SetLoadIsFromSessionHistory(0, false);
}
}
}

6
docshell/base/nsDocShellLoadState.cpp
View File

@ -556,12 +556,10 @@ nsDocShellLoadState::GetLoadingSessionHistoryInfo() const {
}
void nsDocShellLoadState::SetLoadIsFromSessionHistory(
int32_t aRequestedIndex, int32_t aSessionHistoryLength,
bool aLoadingFromActiveEntry) {
int32_t aOffset, bool aLoadingFromActiveEntry) {
if (mLoadingSessionHistoryInfo) {
mLoadingSessionHistoryInfo->mLoadIsFromSessionHistory = true;
mLoadingSessionHistoryInfo->mRequestedIndex = aRequestedIndex;
mLoadingSessionHistoryInfo->mSessionHistoryLength = aSessionHistoryLength;
mLoadingSessionHistoryInfo->mOffset = aOffset;
mLoadingSessionHistoryInfo->mLoadingCurrentActiveEntry =
aLoadingFromActiveEntry;
}

3
docshell/base/nsDocShellLoadState.h
View File

@ -326,8 +326,7 @@ class nsDocShellLoadState final {
mozilla::dom::DocShellLoadStateInit Serialize();
void SetLoadIsFromSessionHistory(int32_t aRequestedIndex,
int32_t aSessionHistoryLength,
void SetLoadIsFromSessionHistory(int32_t aOffset,
bool aLoadingFromActiveEntry);
void ClearLoadIsFromSessionHistory();

9
docshell/shistory/SessionHistoryEntry.cpp
View File

@ -349,8 +349,7 @@ LoadingSessionHistoryInfo::LoadingSessionHistoryInfo(
: mInfo(aEntry->Info()),
mLoadId(aInfo->mLoadId),
mLoadIsFromSessionHistory(aInfo->mLoadIsFromSessionHistory),
mRequestedIndex(aInfo->mRequestedIndex),
mSessionHistoryLength(aInfo->mSessionHistoryLength),
mOffset(aInfo->mOffset),
mLoadingCurrentActiveEntry(aInfo->mLoadingCurrentActiveEntry) {
MOZ_ASSERT(SessionHistoryEntry::sLoadIdToEntry &&
SessionHistoryEntry::sLoadIdToEntry->Get(mLoadId) == aEntry);
@ -1586,8 +1585,7 @@ void IPDLParamTraits<dom::LoadingSessionHistoryInfo>::Write(
WriteIPDLParam(aMsg, aActor, aParam.mInfo);
WriteIPDLParam(aMsg, aActor, aParam.mLoadId);
WriteIPDLParam(aMsg, aActor, aParam.mLoadIsFromSessionHistory);
WriteIPDLParam(aMsg, aActor, aParam.mRequestedIndex);
WriteIPDLParam(aMsg, aActor, aParam.mSessionHistoryLength);
WriteIPDLParam(aMsg, aActor, aParam.mOffset);
WriteIPDLParam(aMsg, aActor, aParam.mLoadingCurrentActiveEntry);
WriteIPDLParam(aMsg, aActor, aParam.mForceMaybeResetName);
}
@ -1599,8 +1597,7 @@ bool IPDLParamTraits<dom::LoadingSessionHistoryInfo>::Read(
!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mLoadId) ||
!ReadIPDLParam(aMsg, aIter, aActor,
&aResult->mLoadIsFromSessionHistory) ||
!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mRequestedIndex) ||
!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mSessionHistoryLength) ||
!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mOffset) ||
!ReadIPDLParam(aMsg, aIter, aActor,
&aResult->mLoadingCurrentActiveEntry) ||
!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mForceMaybeResetName)) {

7
docshell/shistory/SessionHistoryEntry.h
View File

@ -230,10 +230,9 @@ struct LoadingSessionHistoryInfo {
// but session-history-in-parent needs to pass needed information explicitly
// to the relevant child process.
bool mLoadIsFromSessionHistory = false;
// mRequestedIndex, mSessionHistoryLength and mLoadingCurrentActiveEntry are
// relevant only if mLoadIsFromSessionHistory is true.
int32_t mRequestedIndex = -1;
int32_t mSessionHistoryLength = 0;
// mOffset and mLoadingCurrentActiveEntry are relevant only if
// mLoadIsFromSessionHistory is true.
int32_t mOffset = 0;
// If we're loading from the current active entry we want to treat it as not
// a same-document navigation (see nsDocShell::IsSameDocumentNavigation).
bool mLoadingCurrentActiveEntry = false;

29
docshell/shistory/nsSHistory.cpp
View File

@ -2028,6 +2028,10 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,
return NS_ERROR_FAILURE;
}
int32_t originalRequestedIndex = mRequestedIndex;
int32_t previousRequest = mRequestedIndex > -1 ? mRequestedIndex : mIndex;
int32_t requestedOffset = aIndex - previousRequest;
// This is a normal local history navigation.
nsCOMPtr<nsISHEntry> prevEntry;
@ -2085,14 +2089,20 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,
if (mRequestedIndex == mIndex) {
// Possibly a reload case
InitiateLoad(nextEntry, mRootBC, aLoadType, aLoadResults, aUserActivation);
InitiateLoad(nextEntry, mRootBC, aLoadType, aLoadResults, aUserActivation,
requestedOffset);
return NS_OK;
}
// Going back or forward.
bool differenceFound = LoadDifferingEntries(
prevEntry, nextEntry, mRootBC, aLoadType, aLoadResults, aUserActivation);
bool differenceFound =
LoadDifferingEntries(prevEntry, nextEntry, mRootBC, aLoadType,
aLoadResults, aUserActivation, requestedOffset);
if (!differenceFound) {
// LoadNextPossibleEntry will change the offset by one, and in order
// to keep track of the requestedOffset, need to reset mRequestedIndex to
// the value it had initially.
mRequestedIndex = originalRequestedIndex;
// We did not find any differences. Go further in the history.
return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd, aLoadResults,
aUserActivation);
@ -2105,7 +2115,7 @@ bool nsSHistory::LoadDifferingEntries(nsISHEntry* aPrevEntry,
nsISHEntry* aNextEntry,
BrowsingContext* aParent, long aLoadType,
nsTArray<LoadEntryResult>& aLoadResults,
bool aUserActivation) {
bool aUserActivation, int32_t aOffset) {
MOZ_ASSERT(aPrevEntry && aNextEntry && aParent);
uint32_t prevID = aPrevEntry->GetID();
@ -2115,7 +2125,8 @@ bool nsSHistory::LoadDifferingEntries(nsISHEntry* aPrevEntry,
if (prevID != nextID) {
// Set the Subframe flag if not navigating the root docshell.
aNextEntry->SetIsSubFrame(aParent != mRootBC);
InitiateLoad(aNextEntry, aParent, aLoadType, aLoadResults, aUserActivation);
InitiateLoad(aNextEntry, aParent, aLoadType, aLoadResults, aUserActivation,
aOffset);
return true;
}
@ -2174,7 +2185,7 @@ bool nsSHistory::LoadDifferingEntries(nsISHEntry* aPrevEntry,
// This will either load a new page to shell or some subshell or
// do nothing.
if (LoadDifferingEntries(pChild, nChild, bcChild, aLoadType, aLoadResults,
aUserActivation)) {
aUserActivation, aOffset)) {
differenceFound = true;
}
}
@ -2184,7 +2195,7 @@ bool nsSHistory::LoadDifferingEntries(nsISHEntry* aPrevEntry,
void nsSHistory::InitiateLoad(nsISHEntry* aFrameEntry,
BrowsingContext* aFrameBC, long aLoadType,
nsTArray<LoadEntryResult>& aLoadResults,
bool aUserActivation) {
bool aUserActivation, int32_t aOffset) {
MOZ_ASSERT(aFrameBC && aFrameEntry);
LoadEntryResult* loadResult = aLoadResults.AppendElement();
@ -2217,8 +2228,8 @@ void nsSHistory::InitiateLoad(nsISHEntry* aFrameEntry,
aFrameBC->GetDocShell() &&
nsDocShell::Cast(aFrameBC->GetDocShell())->IsOSHE(aFrameEntry);
}
loadState->SetLoadIsFromSessionHistory(mRequestedIndex, Length(),
loadingFromActiveEntry);
loadState->SetLoadIsFromSessionHistory(aOffset, loadingFromActiveEntry);
if (mozilla::SessionHistoryInParent()) {
nsCOMPtr<SessionHistoryEntry> she = do_QueryInterface(aFrameEntry);

4
docshell/shistory/nsSHistory.h
View File

@ -222,11 +222,11 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
mozilla::dom::BrowsingContext* aParent,
long aLoadType,
nsTArray<LoadEntryResult>& aLoadResults,
bool aUserActivation);
bool aUserActivation, int32_t aOffset);
void InitiateLoad(nsISHEntry* aFrameEntry,
mozilla::dom::BrowsingContext* aFrameBC, long aLoadType,
nsTArray<LoadEntryResult>& aLoadResult,
bool aUserActivation);
bool aUserActivation, int32_t aOffset);
nsresult LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd,
nsTArray<LoadEntryResult>& aLoadResults,

5
docshell/test/navigation/file_new_shentry_during_history_navigation_1.html Normal file
View File

@ -0,0 +1,5 @@
<script>
onload = function() {
opener.postMessage("load");
}
</script>

1
docshell/test/navigation/file_new_shentry_during_history_navigation_1.html^headers^ Normal file
View File

@ -0,0 +1 @@
Cache-control: no-store

10
docshell/test/navigation/file_new_shentry_during_history_navigation_2.html Normal file
View File

@ -0,0 +1,10 @@
<script>
onload = function() {
opener.postMessage("load");
}
onbeforeunload = function() {
opener.postMessage("beforeunload");
history.pushState("data1", "", "?push1");
}
</script>

1
docshell/test/navigation/file_new_shentry_during_history_navigation_2.html^headers^ Normal file
View File

@ -0,0 +1 @@
Cache-control: no-store

6
docshell/test/navigation/mochitest.ini
View File

@ -127,6 +127,12 @@ support-files = file_evict_from_bfcache.html
support-files = file_meta_refresh.html
[test_navigation_type.html]
support-files = file_navigation_type.html
[test_new_shentry_during_history_navigation.html]
support-files =
file_new_shentry_during_history_navigation_1.html
file_new_shentry_during_history_navigation_1.html^headers^
file_new_shentry_during_history_navigation_2.html
file_new_shentry_during_history_navigation_2.html^headers^
[test_not-opener.html]
[test_online_offline_bfcache.html]
support-files = file_online_offline_bfcache.html

60
docshell/test/navigation/test_new_shentry_during_history_navigation.html Normal file
View File

@ -0,0 +1,60 @@
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<title>Test adding new session history entries while navigating to another one</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
<script>
SimpleTest.waitForExplicitFinish();
var win;
function waitForMessage(msg) {
return new Promise(
function(resolve) {
window.addEventListener("message",
function(event) {
is(event.data, msg, "Got the expected message " + msg);
resolve();
}, { once: true }
);
}
);
}
async function test() {
let loadPromise = waitForMessage("load");
win = window.open("file_new_shentry_during_history_navigation_1.html");
await loadPromise;
loadPromise = waitForMessage("load");
win.location.href = "file_new_shentry_during_history_navigation_2.html";
await loadPromise;
let beforeunloadPromise = waitForMessage("beforeunload");
win.history.back();
await beforeunloadPromise;
await waitForMessage("load");
win.history.back();
SimpleTest.requestFlakyTimeout("Test that history.back() does not work on the initial entry.");
setTimeout(function() {
win.onmessage = null;
win.close();
SimpleTest.finish();
}, 500);
window.onmessage = function(event) {
ok(false, "Should not get a message " + event.data);
}
}
</script>
</head>
<body onload="test()">
<p id="display"></p>
<div id="content" style="display: none"></div>
<pre id="test"></pre>
</body>
</html>

9
dom/ipc/ContentParent.cpp
View File

@ -7353,13 +7353,8 @@ ContentParent::RecvGetLoadingSessionHistoryInfoFromParent(
}
Maybe<LoadingSessionHistoryInfo> info;
int32_t requestedIndex = -1;
int32_t sessionHistoryLength = 0;
aContext.get_canonical()->GetLoadingSessionHistoryInfoFromParent(
info, &requestedIndex, &sessionHistoryLength);
aResolver(
Tuple<const mozilla::Maybe<LoadingSessionHistoryInfo>&, const int32_t&,
const int32_t&>(info, requestedIndex, sessionHistoryLength));
aContext.get_canonical()->GetLoadingSessionHistoryInfoFromParent(info);
aResolver(info);
return IPC_OK();
}

2
dom/ipc/PContent.ipdl
View File

@ -1015,7 +1015,7 @@ parent:
nsString aName);
async GetLoadingSessionHistoryInfoFromParent(MaybeDiscardedBrowsingContext aContext)
returns (LoadingSessionHistoryInfo? aLoadingInfo, int32_t aRequestedIndex, int32_t aLength);
returns (LoadingSessionHistoryInfo? aLoadingInfo);
async RemoveFromBFCache(MaybeDiscardedBrowsingContext aContext);