Skip to content

Commit

Permalink
Bug 1667006 - document.open should set the url of the session history…
Browse files Browse the repository at this point in the history
… entry to the entry document's. r=smaug

document.open replaces the session history entry with a new one, the url of
which needs to point to the entry document. nsDocShell::UpdateActiveEntry wasn't
resetting the url and the originalURL in the replace case. This patch always
creates a new SessionHistoryInfo so we share more code between replace and
non-replace, to avoid missing some fields.

Differential Revision: https://phabricator.services.mozilla.com/D91273
  • Loading branch information
petervanderbeken committed Sep 24, 2020
1 parent 27d2ca4 commit 4a5a36d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 49 deletions.
93 changes: 48 additions & 45 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1146,13 +1146,13 @@ void nsDocShell::FirePageHideNotificationInternal(
if (rootSH) {
MOZ_LOG(
gSHLog, LogLevel::Debug,
("document %p unloading, remove dynamic subframe entries", this));
("nsDocShell %p unloading, remove dynamic subframe entries", this));
if (StaticPrefs::fission_sessionHistoryInParent()) {
if (mActiveEntry) {
mBrowsingContext->RemoveDynEntriesFromActiveSessionHistoryEntry();
}
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p unloading, no active entries", this));
("nsDocShell %p unloading, no active entries", this));
} else if (mOSHE) {
int32_t index = rootSH->Index();
rootSH->LegacySHistory()->RemoveDynEntries(index, mOSHE);
Expand Down Expand Up @@ -1409,10 +1409,10 @@ bool nsDocShell::SetCurrentURI(nsIURI* aURI, nsIRequest* aRequest,
if (mLoadingEntry) {
isSubFrame = mLoadingEntry->mInfo.IsSubFrame();
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p SetCurrentURI, isSubFrame=%d", this, isSubFrame));
("nsDocShell %p SetCurrentURI, isSubFrame=%d", this, isSubFrame));
}
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p SetCurrentURI, no mLoadingEntry", this));
("nsDocShell %p SetCurrentURI, no mLoadingEntry", this));
} else {
if (mLSHE) {
isSubFrame = mLSHE->GetIsSubFrame();
Expand Down Expand Up @@ -4018,7 +4018,7 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
// reload
RefPtr<ChildSHistory> rootSH = GetRootSessionHistory();
if (StaticPrefs::fission_sessionHistoryInParent()) {
MOZ_LOG(gSHLog, LogLevel::Debug, ("document %p Reload", this));
MOZ_LOG(gSHLog, LogLevel::Debug, ("nsDocShell %p Reload", this));
bool forceReload = IsForceReloadType(loadType);
if (!XRE_IsParentProcess()) {
RefPtr<nsDocShell> docShell(this);
Expand All @@ -4044,12 +4044,12 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
if (loadState.isSome()) {
MOZ_LOG(
gSHLog, LogLevel::Debug,
("document %p Reload - LoadHistoryEntry", docShell.get()));
("nsDocShell %p Reload - LoadHistoryEntry", docShell.get()));
docShell->LoadHistoryEntry(loadState.ref(), loadType,
reloadingActiveEntry.ref());
} else {
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p ReloadDocument", docShell.get()));
("nsDocShell %p ReloadDocument", docShell.get()));
ReloadDocument(docShell, doc, loadType, browsingContext,
currentURI, referrerInfo);
}
Expand All @@ -4067,12 +4067,12 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
if (canReload) {
if (loadState.isSome()) {
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p Reload - LoadHistoryEntry", this));
("nsDocShell %p Reload - LoadHistoryEntry", this));
LoadHistoryEntry(loadState.ref(), loadType,
reloadingActiveEntry.ref());
} else {
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p ReloadDocument", this));
("nsDocShell %p ReloadDocument", this));
ReloadDocument(this, GetDocument(), loadType, mBrowsingContext,
mCurrentURI, mReferrerInfo);
}
Expand Down Expand Up @@ -8199,7 +8199,7 @@ void nsDocShell::SetDocCurrentStateObj(nsISHEntry* aShEntry,
scContainer = aInfo->GetStateData();
}
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p SetCurrentDocState %p", this, scContainer.get()));
("nsDocShell %p SetCurrentDocState %p", this, scContainer.get()));
} else {
if (aShEntry) {
scContainer = aShEntry->GetStateData();
Expand Down Expand Up @@ -8627,7 +8627,7 @@ bool nsDocShell::IsSameDocumentNavigation(nsDocShellLoadState* aLoadState,
aLoadState->GetLoadingSessionHistoryInfo()->mInfo);
}
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p NavBetweenSameDoc=%d", this,
("nsDocShell %p NavBetweenSameDoc=%d", this,
aState.mHistoryNavBetweenSameDoc));
} else {
if (mOSHE && aLoadState->LoadIsFromSessionHistory()) {
Expand Down Expand Up @@ -8672,7 +8672,7 @@ bool nsDocShell::IsSameDocumentNavigation(nsDocShellLoadState* aLoadState,
aState.mSameExceptHashes && aState.mNewURIHasRef);

MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p NavBetweenSameDoc=%d is same doc = %d", this,
("nsDocShell %p NavBetweenSameDoc=%d is same doc = %d", this,
aState.mHistoryNavBetweenSameDoc, doSameDocumentNavigation));
return doSameDocumentNavigation;
}
Expand Down Expand Up @@ -11034,7 +11034,7 @@ nsresult nsDocShell::UpdateURLAndHistory(Document* aDocument, nsIURI* aNewURI,

if (StaticPrefs::fission_sessionHistoryInParent()) {
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p UpdateActiveEntry replace", this));
("nsDocShell %p UpdateActiveEntry (not replacing)", this));
nsString title(mActiveEntry->GetTitle());
UpdateActiveEntry(false,
/* aPreviousScrollPos = */ Some(scrollPos), aNewURI,
Expand Down Expand Up @@ -11074,7 +11074,10 @@ nsresult nsDocShell::UpdateURLAndHistory(Document* aDocument, nsIURI* aNewURI,
}
} else if (StaticPrefs::fission_sessionHistoryInParent()) {
MOZ_LOG(gSHLog, LogLevel::Debug,
("document %p UpdateActiveEntry non-replace", this));
("nsDocShell %p UpdateActiveEntry (replacing)", this));
// Setting the resultPrincipalURI to nullptr is fine here: it will cause
// NS_GetFinalChannelURI to use the originalURI as the URI, which is aNewURI
// in our case. We could also set it to aNewURI, with the same result.
UpdateActiveEntry(
true, /* aPreviousScrollPos = */ Nothing(), aNewURI, aNewURI,
aDocument->NodePrincipal(), aDocument->GetCsp(), u""_ns,
Expand All @@ -11083,7 +11086,7 @@ nsresult nsDocShell::UpdateURLAndHistory(Document* aDocument, nsIURI* aNewURI,
// Step 3.
newSHEntry = mOSHE;

MOZ_LOG(gSHLog, LogLevel::Debug, ("document %p step 3", this));
MOZ_LOG(gSHLog, LogLevel::Debug, ("nsDocShell %p step 3", this));
// Since we're not changing which page we have loaded, pass
// true for aCloneChildren.
if (!newSHEntry) {
Expand Down Expand Up @@ -11504,25 +11507,36 @@ void nsDocShell::UpdateActiveEntry(
"This code only deals with pushState");
MOZ_ASSERT_IF(aPreviousScrollPos.isSome(), !aReplace);

if (!aReplace || !mActiveEntry) {
MOZ_LOG(gSHLog, LogLevel::Debug,
("Creating an active entry on nsDocShell %p to %s", this,
aURI->GetSpecOrDefault().get()));
if (mActiveEntry) {
// Link this entry to the previous active entry.
mActiveEntry =
MakeUnique<SessionHistoryInfo>(*mActiveEntry, aURI, HistoryID());
} else {
mActiveEntry = MakeUnique<SessionHistoryInfo>(
aURI, HistoryID(), aTriggeringPrincipal, aCsp, mContentTypeHint);
}
mActiveEntry->SetTitle(aTitle);
mActiveEntry->SetStateData(static_cast<nsStructuredCloneContainer*>(aData));
mActiveEntry->SetURIWasModified(aURIWasModified);
if (aScrollRestorationIsManual.isSome()) {
mActiveEntry->SetScrollRestorationIsManual(
aScrollRestorationIsManual.value());
}
MOZ_LOG(gSHLog, LogLevel::Debug,
("Creating an active entry on nsDocShell %p to %s", this,
aURI->GetSpecOrDefault().get()));

// Even if we're replacing an existing entry we create new a
// SessionHistoryInfo. In the parent process we'll keep the existing
// SessionHistoryEntry, but just replace its SessionHistoryInfo, that way the
// entry keeps identity but its data is replaced.
bool replace = aReplace && mActiveEntry;
if (mActiveEntry) {
// Link this entry to the previous active entry.
mActiveEntry = MakeUnique<SessionHistoryInfo>(*mActiveEntry, aURI);
// FIXME Assert that mTriggeringPrincipal/mContentType/mDocShellID/
// mDynamicallyCreated are correct?
} else {
mActiveEntry = MakeUnique<SessionHistoryInfo>(
aURI, HistoryID(), aTriggeringPrincipal, aCsp, mContentTypeHint);
}
mActiveEntry->SetOriginalURI(aOriginalURI);
mActiveEntry->SetTitle(aTitle);
mActiveEntry->SetStateData(static_cast<nsStructuredCloneContainer*>(aData));
mActiveEntry->SetURIWasModified(aURIWasModified);
if (aScrollRestorationIsManual.isSome()) {
mActiveEntry->SetScrollRestorationIsManual(
aScrollRestorationIsManual.value());
}

if (replace) {
mBrowsingContext->ReplaceActiveSessionHistoryEntry(mActiveEntry.get());
} else {
if (mBrowsingContext->IsTop()) {
mBrowsingContext->SetActiveSessionHistoryEntryForTop(
aPreviousScrollPos, mActiveEntry.get(), mLoadType);
Expand All @@ -11533,17 +11547,6 @@ void nsDocShell::UpdateActiveEntry(
mBrowsingContext->SetActiveSessionHistoryEntryForFrame(
aPreviousScrollPos, mActiveEntry.get(), mChildOffset);
}
} else {
mActiveEntry->SetResultPrincipalURI(nullptr);
mActiveEntry->SetLoadReplace(false);
mActiveEntry->SetStateData(static_cast<nsStructuredCloneContainer*>(aData));
mActiveEntry->SetPostData(nullptr);
mActiveEntry->SetURIWasModified(aURIWasModified);
if (aScrollRestorationIsManual.isSome()) {
mActiveEntry->SetScrollRestorationIsManual(
aScrollRestorationIsManual.value());
}
mBrowsingContext->ReplaceActiveSessionHistoryEntry(mActiveEntry.get());
}
}

Expand Down
3 changes: 1 addition & 2 deletions docshell/shistory/SessionHistoryEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ SessionHistoryInfo::SessionHistoryInfo(nsDocShellLoadState* aLoadState,
}

SessionHistoryInfo::SessionHistoryInfo(
const SessionHistoryInfo& aSharedStateFrom, nsIURI* aURI,
const nsID& aDocShellID)
const SessionHistoryInfo& aSharedStateFrom, nsIURI* aURI)
: mURI(aURI), mSharedState(aSharedStateFrom.mSharedState) {
MaybeUpdateTitleFromURI();
}
Expand Down
3 changes: 1 addition & 2 deletions docshell/shistory/SessionHistoryEntry.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ class SessionHistoryInfo {
SessionHistoryInfo() = default;
SessionHistoryInfo(const SessionHistoryInfo& aInfo) = default;
SessionHistoryInfo(nsDocShellLoadState* aLoadState, nsIChannel* aChannel);
SessionHistoryInfo(const SessionHistoryInfo& aSharedStateFrom, nsIURI* aURI,
const nsID& aDocShellID);
SessionHistoryInfo(const SessionHistoryInfo& aSharedStateFrom, nsIURI* aURI);
SessionHistoryInfo(nsIURI* aURI, const nsID& aDocShellID,
nsIPrincipal* aTriggeringPrincipal,
nsIContentSecurityPolicy* aCsp,
Expand Down

0 comments on commit 4a5a36d

Please sign in to comment.