diff --git a/netwerk/ipc/DocumentChannelChild.cpp b/netwerk/ipc/DocumentChannelChild.cpp index c498d3c0698f..17247693bf3b 100644 --- a/netwerk/ipc/DocumentChannelChild.cpp +++ b/netwerk/ipc/DocumentChannelChild.cpp @@ -123,8 +123,15 @@ IPCResult DocumentChannelChild::RecvFailedAsyncOpen( } IPCResult DocumentChannelChild::RecvDisconnectChildListeners( - const nsresult& aStatus, const nsresult& aLoadGroupStatus) { - DisconnectChildListeners(aStatus, aLoadGroupStatus); + const nsresult& aStatus, const nsresult& aLoadGroupStatus, + bool aSwitchedProcess) { + // If this is a normal failure, then we want to disconnect our listeners and + // notify them of the failure. If this is a process switch, then we can just + // ignore it silently, and trust that the switch will shut down our docshell + // and cancel us when it's ready. + if (!aSwitchedProcess) { + DisconnectChildListeners(aStatus, aLoadGroupStatus); + } return IPC_OK(); } diff --git a/netwerk/ipc/DocumentChannelChild.h b/netwerk/ipc/DocumentChannelChild.h index dd40cab9cf0d..717a8bbf0d06 100644 --- a/netwerk/ipc/DocumentChannelChild.h +++ b/netwerk/ipc/DocumentChannelChild.h @@ -38,7 +38,8 @@ class DocumentChannelChild final : public DocumentChannel, mozilla::ipc::IPCResult RecvFailedAsyncOpen(const nsresult& aStatusCode); mozilla::ipc::IPCResult RecvDisconnectChildListeners( - const nsresult& aStatus, const nsresult& aLoadGroupStatus); + const nsresult& aStatus, const nsresult& aLoadGroupStatus, + bool aSwitchedProcess); mozilla::ipc::IPCResult RecvDeleteSelf(); diff --git a/netwerk/ipc/DocumentChannelParent.cpp b/netwerk/ipc/DocumentChannelParent.cpp index c33c8e885a68..9eccb0ee9ed7 100644 --- a/netwerk/ipc/DocumentChannelParent.cpp +++ b/netwerk/ipc/DocumentChannelParent.cpp @@ -79,11 +79,11 @@ bool DocumentChannelParent::Init(dom::CanonicalBrowsingContext* aContext, self->mDocumentLoadListener = nullptr; }, [self](DocumentLoadListener::OpenPromiseFailedType&& aRejectValue) { - if (!self->CanSend()) { - return; + if (self->CanSend()) { + Unused << self->SendDisconnectChildListeners( + aRejectValue.mStatus, aRejectValue.mLoadGroupStatus, + aRejectValue.mSwitchedProcess); } - Unused << self->SendDisconnectChildListeners( - aRejectValue.mStatus, aRejectValue.mLoadGroupStatus); self->mDocumentLoadListener = nullptr; }); diff --git a/netwerk/ipc/DocumentLoadListener.cpp b/netwerk/ipc/DocumentLoadListener.cpp index a97ca4b04e3c..42d8af220e5d 100644 --- a/netwerk/ipc/DocumentLoadListener.cpp +++ b/netwerk/ipc/DocumentLoadListener.cpp @@ -14,6 +14,7 @@ #include "mozilla/StaticPrefs_extensions.h" #include "mozilla/StaticPrefs_fission.h" #include "mozilla/StaticPrefs_security.h" +#include "mozilla/dom/BrowserParent.h" #include "mozilla/dom/BrowsingContextGroup.h" #include "mozilla/dom/CanonicalBrowsingContext.h" #include "mozilla/dom/ChildProcessChannelListener.h" @@ -800,15 +801,9 @@ void DocumentLoadListener::Cancel(const nsresult& aStatusCode) { ("DocumentLoadListener Cancel [this=%p, " "aStatusCode=%" PRIx32 " ]", this, static_cast(aStatusCode))); - mCancelled = true; - - if (mDoingProcessSwitch) { - // If we've already initiated process-switching - // then we can no longer be cancelled and we'll - // disconnect the old listeners when done. + if (mOpenPromiseResolved) { return; } - if (mChannel) { mChannel->Cancel(aStatusCode); } @@ -829,12 +824,16 @@ void DocumentLoadListener::DisconnectListeners(nsresult aStatus, Disconnect(); - // If we're not going to send anything else to the content process, and - // we haven't yet consumed a stream filter promise, then we're never going - // to. - // TODO: This might be because we retargeted the stream to the download - // handler or similar. Do we need to attach a stream filter to that? - mStreamFilterRequests.Clear(); + if (!aSwitchedProcess) { + // If we're not going to send anything else to the content process, and + // we haven't yet consumed a stream filter promise, then we're never going + // to. If we're disconnecting the old content process due to a proces + // switch, then we can rely on FinishReplacementChannelSetup being called + // (even if the switch failed), so we clear at that point instead. + // TODO: This might be because we retargeted the stream to the download + // handler or similar. Do we need to attach a stream filter to that? + mStreamFilterRequests.Clear(); + } } void DocumentLoadListener::RedirectToRealChannelFinished(nsresult aRv) { @@ -890,11 +889,7 @@ void DocumentLoadListener::FinishReplacementChannelSetup(nsresult aResult) { ctx->EndDocumentLoad(false); } }); - - if (mDoingProcessSwitch) { - DisconnectListeners(NS_BINDING_ABORTED, NS_BINDING_ABORTED, - NS_SUCCEEDED(aResult)); - } + mStreamFilterRequests.Clear(); nsCOMPtr registrar = RedirectChannelRegistrar::GetOrCreate(); @@ -1433,7 +1428,8 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch( // Get information about the current document loaded in our BrowsingContext. nsCOMPtr currentPrincipal; - if (auto* wgp = browsingContext->GetCurrentWindowGlobal()) { + RefPtr wgp = browsingContext->GetCurrentWindowGlobal(); + if (wgp) { currentPrincipal = wgp->DocumentPrincipal(); } @@ -1474,7 +1470,18 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch( LOG(("Process Switch: Changing Remoteness from '%s' to '%s'", currentRemoteType.get(), remoteType.get())); + // We're now committing to a process switch, so we can disconnect from + // the listeners in the old process. mDoingProcessSwitch = true; + if (wgp && wgp->IsProcessRoot()) { + if (RefPtr browserParent = wgp->GetBrowserParent()) { + // This load has already started, so we want to filter out any 'stop' + // progress events coming from the old process as a result of us + // disconnecting from it. + browserParent->SuspendProgressEventsUntilAfterNextLoadStarts(); + } + } + DisconnectListeners(NS_BINDING_ABORTED, NS_BINDING_ABORTED, true); LOG(("Process Switch: Calling ChangeRemoteness")); browsingContext diff --git a/netwerk/ipc/DocumentLoadListener.h b/netwerk/ipc/DocumentLoadListener.h index a8639a87935d..f7e130da88f4 100644 --- a/netwerk/ipc/DocumentLoadListener.h +++ b/netwerk/ipc/DocumentLoadListener.h @@ -474,9 +474,6 @@ class DocumentLoadListener : public nsIInterfaceRequestor, // passed to the childChannel in order to identify it in the new process. uint64_t mLoadIdentifier = 0; - // True if cancelled. - bool mCancelled = false; - Maybe mOriginalUriString; bool mSupportsRedirectToRealChannel = true; diff --git a/netwerk/ipc/PDocumentChannel.ipdl b/netwerk/ipc/PDocumentChannel.ipdl index c8bcb3420b8c..afb5a0f4c80b 100644 --- a/netwerk/ipc/PDocumentChannel.ipdl +++ b/netwerk/ipc/PDocumentChannel.ipdl @@ -46,7 +46,7 @@ child: // the loadgroup (but aStatus is passed as the parameter to RemoveRequest). // We do this so we can remove using NS_BINDING_RETARGETED, but still have the // channel not be in an error state. - async DisconnectChildListeners(nsresult aStatus, nsresult aLoadGroupReason); + async DisconnectChildListeners(nsresult aStatus, nsresult aLoadGroupReason, bool aSwitchedProcess); // Triggers replacing this DocumentChannel with a 'real' channel (like PHttpChannel), // and notifies the listener via a redirect to the new channel. diff --git a/netwerk/ipc/ParentProcessDocumentChannel.cpp b/netwerk/ipc/ParentProcessDocumentChannel.cpp index 03137f189052..11ba7c97b30d 100644 --- a/netwerk/ipc/ParentProcessDocumentChannel.cpp +++ b/netwerk/ipc/ParentProcessDocumentChannel.cpp @@ -204,8 +204,14 @@ NS_IMETHODIMP ParentProcessDocumentChannel::AsyncOpen( p->ChainTo(aResolveValue.mPromise.forget(), __func__); }, [self](DocumentLoadListener::OpenPromiseFailedType&& aRejectValue) { - self->DisconnectChildListeners(aRejectValue.mStatus, - aRejectValue.mLoadGroupStatus); + // If this is a normal failure, then we want to disconnect our listeners + // and notify them of the failure. If this is a process switch, then we + // can just ignore it silently, and trust that the switch will shut down + // our docshell and cancel us when it's ready. + if (!aRejectValue.mSwitchedProcess) { + self->DisconnectChildListeners(aRejectValue.mStatus, + aRejectValue.mLoadGroupStatus); + } self->RemoveObserver(); }); return NS_OK;