corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests 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 1af8c5c2f6
commit 7ecc9afd1d
18 changed files with 150 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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.