Skip to content

Commit

Permalink
Bug 1843968 - Hold some strong references. r=mccr8, a=dmeehan
Browse files Browse the repository at this point in the history
  • Loading branch information
petervanderbeken committed Aug 1, 2023
1 parent d1b8fca commit e1cb7db
Show file tree
Hide file tree
Showing 20 changed files with 101 additions and 64 deletions.
16 changes: 10 additions & 6 deletions docshell/base/BrowsingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1933,7 +1933,8 @@ nsresult BrowsingContext::LoadURI(nsDocShellLoadState* aLoadState,
"Targeting occurs in InternalLoad");

if (mDocShell) {
return mDocShell->LoadURI(aLoadState, aSetNavigating);
nsCOMPtr<nsIDocShell> docShell = mDocShell;
return docShell->LoadURI(aLoadState, aSetNavigating);
}

// Note: We do this check both here and in `nsDocShell::InternalLoad`, since
Expand Down Expand Up @@ -2029,7 +2030,8 @@ nsresult BrowsingContext::InternalLoad(nsDocShellLoadState* aLoadState) {
"must be targeting this BrowsingContext");

if (mDocShell) {
return nsDocShell::Cast(mDocShell)->InternalLoad(aLoadState);
RefPtr<nsDocShell> docShell = nsDocShell::Cast(mDocShell);
return docShell->InternalLoad(aLoadState);
}

// Note: We do this check both here and in `nsDocShell::InternalLoad`, since
Expand Down Expand Up @@ -2091,9 +2093,10 @@ void BrowsingContext::DisplayLoadError(const nsAString& aURI) {

if (mDocShell) {
bool didDisplayLoadError = false;
mDocShell->DisplayLoadError(NS_ERROR_MALFORMED_URI, nullptr,
PromiseFlatString(aURI).get(), nullptr,
&didDisplayLoadError);
nsCOMPtr<nsIDocShell> docShell = mDocShell;
docShell->DisplayLoadError(NS_ERROR_MALFORMED_URI, nullptr,
PromiseFlatString(aURI).get(), nullptr,
&didDisplayLoadError);
} else {
if (ContentParent* cp = Canonical()->GetContentParent()) {
Unused << cp->SendDisplayLoadError(this, PromiseFlatString(aURI));
Expand Down Expand Up @@ -3680,7 +3683,8 @@ void BrowsingContext::HistoryGo(
[](mozilla::ipc::
ResponseRejectReason) { /* FIXME Is ignoring this fine? */ });
} else {
aResolver(Canonical()->HistoryGo(
RefPtr<CanonicalBrowsingContext> self = Canonical();
aResolver(self->HistoryGo(
aOffset, aHistoryEpoch, aRequireUserInteraction, aUserActivation,
Canonical()->GetContentParent()
? Some(Canonical()->GetContentParent()->ChildID())
Expand Down
8 changes: 4 additions & 4 deletions docshell/base/CanonicalBrowsingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,7 @@ void CanonicalBrowsingContext::GoBack(
mCurrentLoad->Cancel(NS_BINDING_CANCELLED_OLD_LOAD, ""_ns);
}

if (nsDocShell* docShell = nsDocShell::Cast(GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(GetDocShell())) {
if (aCancelContentJSEpoch.WasPassed()) {
docShell->SetCancelContentJSEpoch(aCancelContentJSEpoch.Value());
}
Expand All @@ -1515,7 +1515,7 @@ void CanonicalBrowsingContext::GoForward(
mCurrentLoad->Cancel(NS_BINDING_CANCELLED_OLD_LOAD, ""_ns);
}

if (auto* docShell = nsDocShell::Cast(GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(GetDocShell())) {
if (aCancelContentJSEpoch.WasPassed()) {
docShell->SetCancelContentJSEpoch(aCancelContentJSEpoch.Value());
}
Expand All @@ -1541,7 +1541,7 @@ void CanonicalBrowsingContext::GoToIndex(
mCurrentLoad->Cancel(NS_BINDING_CANCELLED_OLD_LOAD, ""_ns);
}

if (auto* docShell = nsDocShell::Cast(GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(GetDocShell())) {
if (aCancelContentJSEpoch.WasPassed()) {
docShell->SetCancelContentJSEpoch(aCancelContentJSEpoch.Value());
}
Expand All @@ -1565,7 +1565,7 @@ void CanonicalBrowsingContext::Reload(uint32_t aReloadFlags) {
mCurrentLoad->Cancel(NS_BINDING_CANCELLED_OLD_LOAD, ""_ns);
}

if (auto* docShell = nsDocShell::Cast(GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(GetDocShell())) {
docShell->Reload(aReloadFlags);
} else if (ContentParent* cp = GetContentParent()) {
Unused << cp->SendReload(this, aReloadFlags);
Expand Down
26 changes: 18 additions & 8 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,8 @@ nsresult nsDocShell::LoadURI(nsDocShellLoadState* aLoadState,
("nsDocShell[%p]: loading from session history", this));

if (!mozilla::SessionHistoryInParent()) {
return LoadHistoryEntry(aLoadState->SHEntry(), aLoadState->LoadType(),
nsCOMPtr<nsISHEntry> entry = aLoadState->SHEntry();
return LoadHistoryEntry(entry, aLoadState->LoadType(),
aLoadState->HasValidUserGestureActivation());
}

Expand Down Expand Up @@ -4125,8 +4126,11 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
} else {
MOZ_LOG(gSHLog, LogLevel::Debug,
("nsDocShell %p ReloadDocument", this));
ReloadDocument(this, GetDocument(), loadType, mBrowsingContext,
mCurrentURI, mReferrerInfo);
RefPtr<Document> doc = GetDocument();
RefPtr<BrowsingContext> bc = mBrowsingContext;
nsCOMPtr<nsIURI> currentURI = mCurrentURI;
nsCOMPtr<nsIReferrerInfo> referrerInfo = mReferrerInfo;
ReloadDocument(this, doc, loadType, bc, currentURI, referrerInfo);
}
}
}
Expand All @@ -4144,19 +4148,24 @@ nsDocShell::Reload(uint32_t aReloadFlags) {

/* If you change this part of code, make sure bug 45297 does not re-occur */
if (mOSHE) {
nsCOMPtr<nsISHEntry> oshe = mOSHE;
return LoadHistoryEntry(
mOSHE, loadType,
oshe, loadType,
aReloadFlags & nsIWebNavigation::LOAD_FLAGS_USER_ACTIVATION);
}

if (mLSHE) { // In case a reload happened before the current load is done
nsCOMPtr<nsISHEntry> lshe = mLSHE;
return LoadHistoryEntry(
mLSHE, loadType,
lshe, loadType,
aReloadFlags & nsIWebNavigation::LOAD_FLAGS_USER_ACTIVATION);
}

return ReloadDocument(this, GetDocument(), loadType, mBrowsingContext,
mCurrentURI, mReferrerInfo);
RefPtr<Document> doc = GetDocument();
RefPtr<BrowsingContext> bc = mBrowsingContext;
nsCOMPtr<nsIURI> currentURI = mCurrentURI;
nsCOMPtr<nsIReferrerInfo> referrerInfo = mReferrerInfo;
return ReloadDocument(this, doc, loadType, bc, currentURI, referrerInfo);
}

/* static */
Expand Down Expand Up @@ -9644,7 +9653,8 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
if (NS_FAILED(rv)) {
nsCOMPtr<nsIChannel> chan(do_QueryInterface(req));
UnblockEmbedderLoadEventForFailure();
if (DisplayLoadError(rv, aLoadState->URI(), nullptr, chan) &&
nsCOMPtr<nsIURI> uri = aLoadState->URI();
if (DisplayLoadError(rv, uri, nullptr, chan) &&
// FIXME: At this point code was using internal load flags, but checking
// non-internal load flags?
aLoadState->HasLoadFlags(LOAD_FLAGS_ERROR_LOAD_CHANGES_RV)) {
Expand Down
3 changes: 2 additions & 1 deletion docshell/base/nsDocShellTreeOwner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,8 @@ nsDocShellTreeOwner::HandleEvent(Event* aEvent) {
aEvent->PreventDefault();
}
} else if (eventType.EqualsLiteral("drop")) {
nsIWebNavigation* webnav = static_cast<nsIWebNavigation*>(mWebBrowser);
nsCOMPtr<nsIWebNavigation> webnav =
static_cast<nsIWebNavigation*>(mWebBrowser);

// The page might have cancelled the dragover event itself, so check to
// make sure that the link can be dropped first.
Expand Down
11 changes: 7 additions & 4 deletions docshell/shistory/ChildSHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void ChildSHistory::SetIndexAndLength(uint32_t aIndex, uint32_t aLength,
void ChildSHistory::Reload(uint32_t aReloadFlags, ErrorResult& aRv) {
if (mozilla::SessionHistoryInParent()) {
if (XRE_IsParentProcess()) {
nsISHistory* shistory =
nsCOMPtr<nsISHistory> shistory =
mBrowsingContext->Canonical()->GetSessionHistory();
if (shistory) {
aRv = shistory->Reload(aReloadFlags);
Expand All @@ -120,7 +120,8 @@ void ChildSHistory::Reload(uint32_t aReloadFlags, ErrorResult& aRv) {

return;
}
aRv = mHistory->Reload(aReloadFlags);
nsCOMPtr<nsISHistory> shistory = mHistory;
aRv = shistory->Reload(aReloadFlags);
}

bool ChildSHistory::CanGo(int32_t aOffset) {
Expand Down Expand Up @@ -207,7 +208,8 @@ void ChildSHistory::GotoIndex(int32_t aIndex, int32_t aOffset,
}

nsCOMPtr<nsISHistory> shistory = mHistory;
mBrowsingContext->HistoryGo(
RefPtr<BrowsingContext> bc = mBrowsingContext;
bc->HistoryGo(
aOffset, mHistoryEpoch, aRequireUserInteraction, aUserActivation,
[shistory](Maybe<int32_t>&& aRequestedIndex) {
// FIXME Should probably only do this for non-fission.
Expand All @@ -216,7 +218,8 @@ void ChildSHistory::GotoIndex(int32_t aIndex, int32_t aOffset,
}
});
} else {
aRv = mHistory->GotoIndex(aIndex, aUserActivation);
nsCOMPtr<nsISHistory> shistory = mHistory;
aRv = shistory->GotoIndex(aIndex, aUserActivation);
}
}

Expand Down
4 changes: 3 additions & 1 deletion docshell/shistory/nsSHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1398,7 +1398,9 @@ void nsSHistory::LoadURIOrBFCache(LoadEntryResult& aLoadEntry) {
}
}

aLoadEntry.mBrowsingContext->LoadURI(aLoadEntry.mLoadState, false);
RefPtr<BrowsingContext> bc = aLoadEntry.mBrowsingContext;
RefPtr<nsDocShellLoadState> loadState = aLoadEntry.mLoadState;
bc->LoadURI(loadState, false);
}

/* static */
Expand Down
4 changes: 2 additions & 2 deletions dom/base/Location.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ void Location::Reload(bool aForceget, nsIPrincipal& aSubjectPrincipal,
return;
}

nsCOMPtr<nsIDocShell> docShell(GetDocShell());
RefPtr<nsDocShell> docShell(GetDocShell().downcast<nsDocShell>());
if (!docShell) {
return aRv.Throw(NS_ERROR_FAILURE);
}
Expand Down Expand Up @@ -595,7 +595,7 @@ void Location::Reload(bool aForceget, nsIPrincipal& aSubjectPrincipal,
nsIWebNavigation::LOAD_FLAGS_BYPASS_PROXY;
}

rv = nsDocShell::Cast(docShell)->Reload(reloadFlags);
rv = docShell->Reload(reloadFlags);
if (NS_FAILED(rv) && rv != NS_BINDING_ABORTED) {
// NS_BINDING_ABORTED is returned when we attempt to reload a POST result
// and the user says no at the "do you want to reload?" prompt. Don't
Expand Down
3 changes: 2 additions & 1 deletion dom/base/nsFrameLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,8 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() {
bool tmpState = mNeedsAsyncDestroy;
mNeedsAsyncDestroy = true;

rv = GetDocShell()->LoadURI(loadState, false);
RefPtr<nsDocShell> docShell = GetDocShell();
rv = docShell->LoadURI(loadState, false);
mNeedsAsyncDestroy = tmpState;
mURIToLoad = nullptr;
NS_ENSURE_SUCCESS(rv, rv);
Expand Down
11 changes: 6 additions & 5 deletions dom/html/HTMLFormElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,8 @@ nsresult HTMLFormElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
OwnerDoc()->WarnOnceAbout(
DeprecatedOperations::eFormSubmissionUntrustedEvent);
}
DoSubmit(aVisitor.mDOMEvent);
RefPtr<Event> event = aVisitor.mDOMEvent;
DoSubmit(event);
break;
}
default:
Expand Down Expand Up @@ -755,7 +756,8 @@ nsresult HTMLFormElement::SubmitSubmission(

// If there is no link handler, then we won't actually be able to submit.
Document* doc = GetComposedDoc();
nsCOMPtr<nsIDocShell> container = doc ? doc->GetDocShell() : nullptr;
RefPtr<nsDocShell> container =
doc ? nsDocShell::Cast(doc->GetDocShell()) : nullptr;
if (!container || IsEditable()) {
return NS_OK;
}
Expand Down Expand Up @@ -800,7 +802,6 @@ nsresult HTMLFormElement::SubmitSubmission(
//
// Submit
//
nsCOMPtr<nsIDocShell> docShell;
uint64_t currentLoadId = 0;

{
Expand All @@ -827,8 +828,8 @@ nsresult HTMLFormElement::SubmitSubmission(
loadState->SetCsp(GetCsp());
loadState->SetAllowFocusMove(UserActivation::IsHandlingUserInput());

rv = nsDocShell::Cast(container)->OnLinkClickSync(this, loadState, false,
NodePrincipal());
nsCOMPtr<nsIPrincipal> nodePrincipal = NodePrincipal();
rv = container->OnLinkClickSync(this, loadState, false, nodePrincipal);
NS_ENSURE_SUBMIT_SUCCESS(rv);

mTargetContext = loadState->TargetBrowsingContext().GetMaybeDiscarded();
Expand Down
14 changes: 7 additions & 7 deletions dom/ipc/ContentChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4361,7 +4361,7 @@ mozilla::ipc::IPCResult ContentChild::RecvLoadURI(
if (aContext.IsNullOrDiscarded()) {
return IPC_OK();
}
BrowsingContext* context = aContext.get();
RefPtr<BrowsingContext> context = aContext.get();
if (!context->IsInProcess()) {
// The DocShell has been torn down or the BrowsingContext has changed
// process in the middle of the load request. There's not much we can do at
Expand Down Expand Up @@ -4407,7 +4407,7 @@ mozilla::ipc::IPCResult ContentChild::RecvInternalLoad(
if (aLoadState->TargetBrowsingContext().IsDiscarded()) {
return IPC_OK();
}
BrowsingContext* context = aLoadState->TargetBrowsingContext().get();
RefPtr<BrowsingContext> context = aLoadState->TargetBrowsingContext().get();

context->InternalLoad(aLoadState);

Expand Down Expand Up @@ -4437,7 +4437,7 @@ mozilla::ipc::IPCResult ContentChild::RecvDisplayLoadError(
if (aContext.IsNullOrDiscarded()) {
return IPC_OK();
}
BrowsingContext* context = aContext.get();
RefPtr<BrowsingContext> context = aContext.get();

context->DisplayLoadError(aURI);

Expand Down Expand Up @@ -4550,7 +4550,7 @@ mozilla::ipc::IPCResult ContentChild::RecvGoBack(
}
BrowsingContext* bc = aContext.get();

if (auto* docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (aCancelContentJSEpoch) {
docShell->SetCancelContentJSEpoch(*aCancelContentJSEpoch);
}
Expand All @@ -4573,7 +4573,7 @@ mozilla::ipc::IPCResult ContentChild::RecvGoForward(
}
BrowsingContext* bc = aContext.get();

if (auto* docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (aCancelContentJSEpoch) {
docShell->SetCancelContentJSEpoch(*aCancelContentJSEpoch);
}
Expand All @@ -4595,7 +4595,7 @@ mozilla::ipc::IPCResult ContentChild::RecvGoToIndex(
}
BrowsingContext* bc = aContext.get();

if (auto* docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (aCancelContentJSEpoch) {
docShell->SetCancelContentJSEpoch(*aCancelContentJSEpoch);
}
Expand All @@ -4617,7 +4617,7 @@ mozilla::ipc::IPCResult ContentChild::RecvReload(
}
BrowsingContext* bc = aContext.get();

if (auto* docShell = nsDocShell::Cast(bc->GetDocShell())) {
if (RefPtr<nsDocShell> docShell = nsDocShell::Cast(bc->GetDocShell())) {
docShell->Reload(aReloadFlags);
}

Expand Down
10 changes: 6 additions & 4 deletions dom/ipc/ContentParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7808,9 +7808,10 @@ mozilla::ipc::IPCResult ContentParent::RecvHistoryGo(
uint64_t aHistoryEpoch, bool aRequireUserInteraction, bool aUserActivation,
HistoryGoResolver&& aResolveRequestedIndex) {
if (!aContext.IsDiscarded()) {
aResolveRequestedIndex(aContext.get_canonical()->HistoryGo(
aOffset, aHistoryEpoch, aRequireUserInteraction, aUserActivation,
Some(ChildID())));
RefPtr<CanonicalBrowsingContext> canonical = aContext.get_canonical();
aResolveRequestedIndex(
canonical->HistoryGo(aOffset, aHistoryEpoch, aRequireUserInteraction,
aUserActivation, Some(ChildID())));
}
return IPC_OK();
}
Expand Down Expand Up @@ -8033,7 +8034,8 @@ mozilla::ipc::IPCResult ContentParent::RecvHistoryReload(
const MaybeDiscarded<BrowsingContext>& aContext,
const uint32_t aReloadFlags) {
if (!aContext.IsDiscarded()) {
nsISHistory* shistory = aContext.get_canonical()->GetSessionHistory();
nsCOMPtr<nsISHistory> shistory =
aContext.get_canonical()->GetSessionHistory();
if (shistory) {
shistory->Reload(aReloadFlags);
}
Expand Down
7 changes: 4 additions & 3 deletions dom/ipc/WindowGlobalParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ mozilla::ipc::IPCResult WindowGlobalParent::RecvLoadURI(
return IPC_FAIL(this, "Illegal cross-process javascript: load attempt");
}

CanonicalBrowsingContext* targetBC = aTargetBC.get_canonical();
RefPtr<CanonicalBrowsingContext> targetBC = aTargetBC.get_canonical();

// FIXME: For cross-process loads, we should double check CanAccess() for the
// source browsing context in the parent process.
Expand Down Expand Up @@ -352,7 +352,7 @@ mozilla::ipc::IPCResult WindowGlobalParent::RecvInternalLoad(
return IPC_FAIL(this, "Illegal cross-process javascript: load attempt");
}

CanonicalBrowsingContext* targetBC =
RefPtr<CanonicalBrowsingContext> targetBC =
aLoadState->TargetBrowsingContext().get_canonical();

// FIXME: For cross-process loads, we should double check CanAccess() for the
Expand Down Expand Up @@ -1354,7 +1354,8 @@ mozilla::ipc::IPCResult WindowGlobalParent::RecvReloadWithHttpsOnlyException() {
loadState->SetTriggeringPrincipal(nsContentUtils::GetSystemPrincipal());
loadState->SetLoadType(LOAD_NORMAL_REPLACE);

BrowsingContext()->Top()->LoadURI(loadState, /* setNavigating */ true);
RefPtr<CanonicalBrowsingContext> topBC = BrowsingContext()->Top();
topBC->LoadURI(loadState, /* setNavigating */ true);

return IPC_OK();
}
Expand Down
Loading

0 comments on commit e1cb7db

Please sign in to comment.