From 2137f67a30cb918f78ca94fc88a5d6481613870d Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Fri, 12 Jun 2020 05:56:28 +0000 Subject: [PATCH] Bug 1641737 - P2. Use MozPromise with DocumentLoadListener and remove cycle. r=mattwoodrow. Differential Revision: https://phabricator.services.mozilla.com/D78179 --- netwerk/ipc/ADocumentChannelBridge.h | 60 ------ netwerk/ipc/DocumentChannelParent.cpp | 73 +++++-- netwerk/ipc/DocumentChannelParent.h | 40 +--- netwerk/ipc/DocumentLoadListener.cpp | 194 ++++++++++--------- netwerk/ipc/DocumentLoadListener.h | 109 +++++++---- netwerk/ipc/ParentProcessDocumentChannel.cpp | 76 +++++--- netwerk/ipc/ParentProcessDocumentChannel.h | 17 +- netwerk/ipc/moz.build | 1 - 8 files changed, 291 insertions(+), 279 deletions(-) delete mode 100644 netwerk/ipc/ADocumentChannelBridge.h diff --git a/netwerk/ipc/ADocumentChannelBridge.h b/netwerk/ipc/ADocumentChannelBridge.h deleted file mode 100644 index 0bfa2e0adc7b..000000000000 --- a/netwerk/ipc/ADocumentChannelBridge.h +++ /dev/null @@ -1,60 +0,0 @@ -/* vim: set sw=2 ts=8 et tw=80 : */ - -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef mozilla_net_ADocumentChannelBridge_h -#define mozilla_net_ADocumentChannelBridge_h - -#include "mozilla/net/PDocumentChannelParent.h" -#include "mozilla/dom/nsCSPContext.h" - -namespace mozilla { -namespace net { - -/** - * ADocumentChannelBridge is the interface for DocumentLoadListener to - * communicate with the nsIChannel placeholder in the docshell. It may be - * implemented over IPDL. - */ -class ADocumentChannelBridge { - public: - NS_INLINE_DECL_PURE_VIRTUAL_REFCOUNTING - - // Notify the destination docshell that we're not going to send - // a response to it (usually because we've redirected to a different - // process), and drop any references to the parent DocumentLoadListener. - // This should remove the nsIChannel from the loadgroup, and - // fire OnStart/StopRequest with aStatus. - // aLoadGroupStatus is used as mStatus when we remove the child channel - // from 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. - virtual void DisconnectChildListeners(nsresult aStatus, - nsresult aLoadGroupStatus) = 0; - - // Delete the bridge, and drop any refs to the DocumentLoadListener - virtual void Delete() = 0; - - // Initate a switch from the DocumentChannel to the protocol-specific - // real channel. - virtual RefPtr - RedirectToRealChannel( - nsTArray>&& - aStreamFilterEndpoints, - uint32_t aRedirectFlags, uint32_t aLoadFlags) = 0; - - // Returns the process id that this bridge is connected to. - // If 0 indicates that the load is started from the parent process. - virtual base::ProcessId OtherPid() const = 0; - - protected: - virtual ~ADocumentChannelBridge() = default; -}; - -} // namespace net -} // namespace mozilla - -#endif // mozilla_net_ADocumentChannelBridge_h diff --git a/netwerk/ipc/DocumentChannelParent.cpp b/netwerk/ipc/DocumentChannelParent.cpp index 4b8420fa2e01..3fe70cfb50b6 100644 --- a/netwerk/ipc/DocumentChannelParent.cpp +++ b/netwerk/ipc/DocumentChannelParent.cpp @@ -6,6 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "DocumentChannelParent.h" + #include "mozilla/dom/BrowserParent.h" #include "mozilla/dom/CanonicalBrowsingContext.h" #include "mozilla/dom/ClientInfo.h" @@ -34,28 +35,58 @@ bool DocumentChannelParent::Init(dom::CanonicalBrowsingContext* aContext, LOG(("DocumentChannelParent Init [this=%p, uri=%s]", this, loadState->URI()->GetSpecOrDefault().get())); + RefPtr promise; if (loadState->GetLoadIdentifier()) { - mParent = DocumentLoadListener::ClaimParentLoad( - loadState->GetLoadIdentifier(), this); - return !!mParent; + promise = DocumentLoadListener::ClaimParentLoad( + getter_AddRefs(mDocumentLoadListener), loadState->GetLoadIdentifier()); + if (!promise) { + return false; + } + } else { + mDocumentLoadListener = new DocumentLoadListener(aContext); + + Maybe clientInfo; + if (aArgs.initialClientInfo().isSome()) { + clientInfo.emplace(ClientInfo(aArgs.initialClientInfo().ref())); + } + + nsresult rv = NS_ERROR_UNEXPECTED; + promise = mDocumentLoadListener->Open( + loadState, aArgs.cacheKey(), Some(aArgs.channelId()), + aArgs.asyncOpenTime(), aArgs.timing().refOr(nullptr), + std::move(clientInfo), aArgs.outerWindowId(), + aArgs.hasValidTransientUserAction(), Some(aArgs.uriModified()), + Some(aArgs.isXFOError()), IProtocol::OtherPid(), &rv); + if (NS_FAILED(rv)) { + MOZ_ASSERT(!promise); + return SendFailedAsyncOpen(rv); + } } - mParent = new DocumentLoadListener(aContext, this); - - Maybe clientInfo; - if (aArgs.initialClientInfo().isSome()) { - clientInfo.emplace(ClientInfo(aArgs.initialClientInfo().ref())); - } - - nsresult rv = NS_ERROR_UNEXPECTED; - if (!mParent->Open(loadState, aArgs.cacheKey(), Some(aArgs.channelId()), - aArgs.asyncOpenTime(), aArgs.timing().refOr(nullptr), - std::move(clientInfo), aArgs.outerWindowId(), - aArgs.hasValidTransientUserAction(), - Some(aArgs.uriModified()), Some(aArgs.isXFOError()), - &rv)) { - return SendFailedAsyncOpen(rv); - } + RefPtr self = this; + promise->Then( + GetCurrentThreadSerialEventTarget(), __func__, + [self](DocumentLoadListener::OpenPromiseSucceededType&& aResolveValue) { + // The DLL is waiting for us to resolve the + // PDocumentChannel::RedirectToRealChannelPromise given as parameter. + auto promise = self->RedirectToRealChannel( + std::move(aResolveValue.mStreamFilterEndpoints), + aResolveValue.mRedirectFlags, aResolveValue.mLoadFlags); + // We chain the promise the DLL is waiting on to the one returned by + // RedirectToRealChannel. As soon as the promise returned is resolved + // or rejected, so will the DLL's promise. + promise->ChainTo(aResolveValue.mPromise.forget(), __func__); + self->mDocumentLoadListener = nullptr; + }, + [self](DocumentLoadListener::OpenPromiseFailedType&& aRejectValue) { + if (aRejectValue.mStatus == NS_ERROR_DOCSHELL_DYING) { + Unused << self->SendDeleteSelf(); + return; + } + Unused << self->SendDisconnectChildListeners( + aRejectValue.mStatus, aRejectValue.mLoadGroupStatus); + self->mDocumentLoadListener = nullptr; + }); return true; } @@ -65,12 +96,12 @@ DocumentChannelParent::RedirectToRealChannel( nsTArray>&& aStreamFilterEndpoints, uint32_t aRedirectFlags, uint32_t aLoadFlags) { - if (!CanSend() || !mParent) { + if (!CanSend()) { return PDocumentChannelParent::RedirectToRealChannelPromise:: CreateAndReject(ResponseRejectReason::ChannelClosed, __func__); } RedirectToRealChannelArgs args; - mParent->SerializeRedirectData( + mDocumentLoadListener->SerializeRedirectData( args, false, aRedirectFlags, aLoadFlags, static_cast(Manager()->Manager())); return SendRedirectToRealChannel(args, std::move(aStreamFilterEndpoints)); diff --git a/netwerk/ipc/DocumentChannelParent.h b/netwerk/ipc/DocumentChannelParent.h index 2cb5e5144c5f..4ae293eac2de 100644 --- a/netwerk/ipc/DocumentChannelParent.h +++ b/netwerk/ipc/DocumentChannelParent.h @@ -7,8 +7,8 @@ #ifndef mozilla_net_DocumentChannelParent_h #define mozilla_net_DocumentChannelParent_h -#include "mozilla/net/PDocumentChannelParent.h" #include "mozilla/net/DocumentLoadListener.h" +#include "mozilla/net/PDocumentChannelParent.h" namespace mozilla { namespace dom { @@ -17,12 +17,10 @@ class CanonicalBrowsingContext; namespace net { /** - * An implementation of ADocumentChannelBridge that forwards all changes across - * to DocumentChannelChild, the nsIChannel implementation owned by a content - * process docshell. + * An actor that forwards all changes across to DocumentChannelChild, the + * nsIChannel implementation owned by a content process docshell. */ -class DocumentChannelParent final : public ADocumentChannelBridge, - public PDocumentChannelParent { +class DocumentChannelParent final : public PDocumentChannelParent { public: NS_INLINE_DECL_REFCOUNTING(DocumentChannelParent, override); @@ -33,45 +31,27 @@ class DocumentChannelParent final : public ADocumentChannelBridge, // PDocumentChannelParent bool RecvCancel(const nsresult& aStatus) { - if (mParent) { - mParent->Cancel(aStatus); + if (mDocumentLoadListener) { + mDocumentLoadListener->Cancel(aStatus); } return true; } void ActorDestroy(ActorDestroyReason aWhy) override { - if (mParent) { - mParent->DocumentChannelBridgeDisconnected(); - mParent = nullptr; + if (mDocumentLoadListener) { + mDocumentLoadListener->Cancel(NS_BINDING_ABORTED); } } private: - // DocumentChannelListener - void DisconnectChildListeners(nsresult aStatus, - nsresult aLoadGroupStatus) override { - if (CanSend()) { - Unused << SendDisconnectChildListeners(aStatus, aLoadGroupStatus); - } - mParent = nullptr; - } - - void Delete() override { - if (CanSend()) { - Unused << SendDeleteSelf(); - } - } - - ProcessId OtherPid() const override { return IProtocol::OtherPid(); } - RefPtr RedirectToRealChannel( nsTArray>&& aStreamFilterEndpoints, - uint32_t aRedirectFlags, uint32_t aLoadFlags) override; + uint32_t aRedirectFlags, uint32_t aLoadFlags); virtual ~DocumentChannelParent(); - RefPtr mParent; + RefPtr mDocumentLoadListener; }; } // namespace net diff --git a/netwerk/ipc/DocumentLoadListener.cpp b/netwerk/ipc/DocumentLoadListener.cpp index 7ebf437402bf..0d64d4241b88 100644 --- a/netwerk/ipc/DocumentLoadListener.cpp +++ b/netwerk/ipc/DocumentLoadListener.cpp @@ -30,9 +30,9 @@ #include "nsDocShellLoadTypes.h" #include "nsExternalHelperAppService.h" #include "nsHttpChannel.h" -#include "nsIHttpChannelInternal.h" #include "nsIBrowser.h" #include "nsIE10SUtils.h" +#include "nsIHttpChannelInternal.h" #include "nsIStreamConverterService.h" #include "nsIViewSourceChannel.h" #include "nsImportModule.h" @@ -221,7 +221,7 @@ class ParentProcessDocumentOpenInfo final : public nsDocumentOpenInfo, // reference cycle. RefPtr doc = do_GetInterface(ToSupports(mListener)); MOZ_ASSERT(doc); - doc->DisconnectChildListeners(NS_BINDING_RETARGETED, NS_OK); + doc->DisconnectListeners(NS_BINDING_RETARGETED, NS_OK); mListener->SetListenerAfterRedirect(nullptr); } @@ -258,9 +258,7 @@ NS_INTERFACE_MAP_BEGIN(DocumentLoadListener) NS_INTERFACE_MAP_END DocumentLoadListener::DocumentLoadListener( - CanonicalBrowsingContext* aBrowsingContext, ADocumentChannelBridge* aBridge) - : mDocumentChannelBridge(aBridge) { - MOZ_ASSERT(aBridge); + CanonicalBrowsingContext* aBrowsingContext) { LOG(("DocumentLoadListener ctor [this=%p]", this)); mParentChannelListener = new ParentChannelListener( this, aBrowsingContext, aBrowsingContext->UsePrivateBrowsing()); @@ -373,12 +371,13 @@ CanonicalBrowsingContext* DocumentLoadListener::GetBrowsingContext() { return mParentChannelListener->GetBrowsingContext(); } -bool DocumentLoadListener::Open( +auto DocumentLoadListener::Open( nsDocShellLoadState* aLoadState, uint32_t aCacheKey, const Maybe& aChannelId, const TimeStamp& aAsyncOpenTime, nsDOMNavigationTiming* aTiming, Maybe&& aInfo, uint64_t aOuterWindowId, bool aHasGesture, Maybe aUriModified, - Maybe aIsXFOError, nsresult* aRv) { + Maybe aIsXFOError, base::ProcessId aPid, nsresult* aRv) + -> RefPtr { LOG(("DocumentLoadListener Open [this=%p, uri=%s]", this, aLoadState->URI()->GetSpecOrDefault().get())); RefPtr browsingContext = @@ -404,7 +403,7 @@ bool DocumentLoadListener::Open( nullptr, attrs, loadFlags, aCacheKey, *aRv, getter_AddRefs(mChannel))) { mParentChannelListener = nullptr; - return false; + return nullptr; } nsCOMPtr uriBeingLoaded = @@ -455,7 +454,7 @@ bool DocumentLoadListener::Open( // we want the original request so that we get different ones for // each part of a multipart channel. nsCOMPtr viewSourceChannel; - if (OtherPid() && (viewSourceChannel = do_QueryInterface(mChannel))) { + if (aPid && (viewSourceChannel = do_QueryInterface(mChannel))) { viewSourceChannel->SetReplaceRequest(false); } @@ -501,12 +500,12 @@ bool DocumentLoadListener::Open( if (aValue.IsResolve()) { bool handled = aValue.ResolveValue(); if (handled) { - self->DisconnectChildListeners(NS_ERROR_ABORT, NS_ERROR_ABORT); + self->DisconnectListeners(NS_ERROR_ABORT, NS_ERROR_ABORT); mParentChannelListener = nullptr; } else { nsresult rv = mChannel->AsyncOpen(openInfo); if (NS_FAILED(rv)) { - self->DisconnectChildListeners(rv, rv); + self->DisconnectListeners(rv, rv); mParentChannelListener = nullptr; } } @@ -518,10 +517,11 @@ bool DocumentLoadListener::Open( *aRv = mChannel->AsyncOpen(openInfo); if (NS_FAILED(*aRv)) { mParentChannelListener = nullptr; - return false; + return nullptr; } } + mOtherPid = aPid; mChannelCreationURI = aLoadState->URI(); mLoadStateLoadFlags = aLoadState->LoadFlags(); mLoadStateLoadType = aLoadState->LoadType(); @@ -537,7 +537,9 @@ bool DocumentLoadListener::Open( if (auto* ctx = GetBrowsingContext()) { ctx->StartDocumentLoad(this); } - return true; + + *aRv = NS_OK; + return mOpenPromise.Ensure(__func__); } /* static */ @@ -609,11 +611,12 @@ bool DocumentLoadListener::OpenFromParent( aBrowsingContext, aBrowsingContext->GetContentParent()->OtherPid()); nsresult rv; - bool result = + auto promise = listener->Open(loadState, cacheKey, channelId, TimeStamp::Now(), timing, std::move(initialClientInfo), aOuterWindowId, false, - Nothing(), Nothing(), &rv); - if (result) { + Nothing(), Nothing(), 0, &rv); + if (promise) { + MOZ_ASSERT(NS_SUCCEEDED(rv)); // Create an entry in the redirect channel registrar to // allocate an identifier for this load. nsCOMPtr registrar = @@ -624,7 +627,7 @@ bool DocumentLoadListener::OpenFromParent( rv = registrar->LinkChannels(*aOutIdent, listener, nullptr); MOZ_ASSERT(NS_SUCCEEDED(rv)); } - return result; + return !!promise; } void DocumentLoadListener::CleanupParentLoadAttempt(uint32_t aLoadIdent) { @@ -644,8 +647,9 @@ void DocumentLoadListener::CleanupParentLoadAttempt(uint32_t aLoadIdent) { registrar->DeregisterChannels(aLoadIdent); } -already_AddRefed DocumentLoadListener::ClaimParentLoad( - uint32_t aLoadIdent, ADocumentChannelBridge* aBridge) { +auto DocumentLoadListener::ClaimParentLoad(DocumentLoadListener** aListener, + uint32_t aLoadIdent) + -> RefPtr { nsCOMPtr registrar = RedirectChannelRegistrar::GetOrCreate(); @@ -654,46 +658,44 @@ already_AddRefed DocumentLoadListener::ClaimParentLoad( RefPtr loadListener = do_QueryObject(parentChannel); registrar->DeregisterChannels(aLoadIdent); - MOZ_ASSERT(loadListener); + MOZ_ASSERT(loadListener && !loadListener->mOpenPromise.IsEmpty()); if (loadListener) { - loadListener->NotifyBridgeConnected(aBridge); + loadListener->NotifyBridgeConnected(); } - return loadListener.forget(); + + RefPtr p = loadListener->mOpenPromise.Ensure(__func__); + loadListener.forget(aListener); + + return p; } -void DocumentLoadListener::NotifyBridgeConnected( - ADocumentChannelBridge* aBridge) { +void DocumentLoadListener::NotifyBridgeConnected() { LOG(("DocumentLoadListener NotifyBridgeConnected [this=%p]", this)); - MOZ_ASSERT(!mDocumentChannelBridge); MOZ_ASSERT(mPendingDocumentChannelBridgeProcess); - MOZ_ASSERT(aBridge->OtherPid() == *mPendingDocumentChannelBridgeProcess); - mDocumentChannelBridge = aBridge; + mOtherPid = *mPendingDocumentChannelBridgeProcess; mPendingDocumentChannelBridgeProcess.reset(); - mBridgePromise.ResolveIfExists(aBridge, __func__); + mBridgePromise.ResolveIfExists(true, __func__); } void DocumentLoadListener::NotifyBridgeFailed() { LOG(("DocumentLoadListener NotifyBridgeFailed [this=%p]", this)); - MOZ_ASSERT(!mDocumentChannelBridge); MOZ_ASSERT(mPendingDocumentChannelBridgeProcess); mPendingDocumentChannelBridgeProcess.reset(); Cancel(NS_BINDING_ABORTED); - mBridgePromise.RejectIfExists(false, __func__); + mBridgePromise.RejectIfExists(NS_BINDING_ABORTED, __func__); } -void DocumentLoadListener::DocumentChannelBridgeDisconnected() { - LOG(("DocumentLoadListener DocumentChannelBridgeDisconnected [this=%p]", - this)); +void DocumentLoadListener::Disconnect() { + LOG(("DocumentLoadListener Disconnect [this=%p]", this)); // The nsHttpChannel may have a reference to this parent, release it // to avoid circular references. RefPtr httpChannelImpl = do_QueryObject(mChannel); if (httpChannelImpl) { httpChannelImpl->SetWarningReporter(nullptr); } - mDocumentChannelBridge = nullptr; if (auto* ctx = GetBrowsingContext()) { ctx->EndDocumentLoad(this); @@ -718,32 +720,31 @@ void DocumentLoadListener::Cancel(const nsresult& aStatusCode) { mChannel->Cancel(aStatusCode); } - DisconnectChildListeners(aStatusCode, aStatusCode); + DisconnectListeners(aStatusCode, aStatusCode); } -void DocumentLoadListener::DisconnectChildListeners(nsresult aStatus, - nsresult aLoadGroupStatus) { +void DocumentLoadListener::DisconnectListeners(nsresult aStatus, + nsresult aLoadGroupStatus) { LOG( - ("DocumentLoadListener DisconnectChildListener [this=%p, " + ("DocumentLoadListener DisconnectListener [this=%p, " "aStatus=%" PRIx32 " aLoadGroupStatus=%" PRIx32 " ]", this, static_cast(aStatus), static_cast(aLoadGroupStatus))); - RefPtr keepAlive(this); - if (mDocumentChannelBridge) { - // This will drop the bridge's reference to us, so we use keepAlive to - // make sure we don't get deleted until we exit the function. - mDocumentChannelBridge->DisconnectChildListeners(aStatus, aLoadGroupStatus); - } else if (mPendingDocumentChannelBridgeProcess) { + if (mPendingDocumentChannelBridgeProcess) { + RefPtr self = this; EnsureBridge()->Then( GetCurrentThreadSerialEventTarget(), __func__, - [keepAlive, aStatus, - aLoadGroupStatus](ADocumentChannelBridge* aBridge) { - aBridge->DisconnectChildListeners(aStatus, aLoadGroupStatus); - keepAlive->mDocumentChannelBridge = nullptr; + [self, aStatus, aLoadGroupStatus](bool aDummy) { + self->RejectOpenPromiseIfExists(aStatus, aLoadGroupStatus, __func__); }, - [](bool aDummy) {}); + [self](nsresult aError) { + self->RejectOpenPromiseIfExists(aError, aError, __func__); + }); + } else { + RejectOpenPromiseIfExists(aStatus, aLoadGroupStatus, __func__); } - DocumentChannelBridgeDisconnected(); + + 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 @@ -801,20 +802,25 @@ void DocumentLoadListener::FinishReplacementChannelSetup(nsresult aResult) { "aResult=%x]", this, int(aResult))); + bool disconnected = false; if (mDoingProcessSwitch) { - DisconnectChildListeners(NS_BINDING_ABORTED, NS_BINDING_ABORTED); + DisconnectListeners(NS_BINDING_ABORTED, NS_BINDING_ABORTED); + disconnected = true; } if (!mRedirectChannelId) { if (NS_FAILED(aResult)) { mChannel->Cancel(aResult); mChannel->Resume(); - DisconnectChildListeners(aResult, aResult); + if (!disconnected) { + DisconnectListeners(aResult, aResult); + } return; } ApplyPendingFunctions(mChannel); - // ResumeSuspendedChannel will be called later as RedirectToRealChannel - // continues, so we can return early. + + // The channel has already been resumed by the ParentProcessDocumentChannel + // so we can return early. return; } @@ -859,7 +865,6 @@ void DocumentLoadListener::FinishReplacementChannelSetup(nsresult aResult) { MOZ_ASSERT( !SameCOMIdentity(redirectChannel, static_cast(this))); - Delete(); redirectChannel->SetParentListener(mParentChannelListener); ApplyPendingFunctions(redirectChannel); @@ -1370,7 +1375,7 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch( LOG(("Process Switch: Calling nsIBrowser::PerformProcessSwitch")); // We're switching a toplevel BrowsingContext's process. This has to be done // using nsIBrowser. - RefPtr domPromise; + RefPtr domPromise; browser->PerformProcessSwitch(remoteType, mCrossProcessRedirectIdentifier, isCOOPSwitch, getter_AddRefs(domPromise)); MOZ_DIAGNOSTIC_ASSERT(domPromise, @@ -1391,12 +1396,10 @@ bool DocumentLoadListener::MaybeTriggerProcessSwitch( return true; } -auto DocumentLoadListener::EnsureBridge() -> RefPtr { - MOZ_ASSERT(mDocumentChannelBridge || mPendingDocumentChannelBridgeProcess); - if (mDocumentChannelBridge) { +RefPtr DocumentLoadListener::EnsureBridge() { + if (!mPendingDocumentChannelBridgeProcess) { MOZ_ASSERT(mBridgePromise.IsEmpty()); - return EnsureBridgePromise::CreateAndResolve(mDocumentChannelBridge, - __func__); + return GenericPromise::CreateAndResolve(true, __func__); } return mBridgePromise.Ensure(__func__); @@ -1472,15 +1475,24 @@ DocumentLoadListener::RedirectToRealChannel( GetCurrentThreadSerialEventTarget(), __func__, [self = RefPtr(this), endpoints = std::move(aStreamFilterEndpoints), aRedirectFlags, - aLoadFlags](ADocumentChannelBridge* aBridge) mutable { - if (self->mCancelled) { + aLoadFlags](bool aDummy) mutable + -> RefPtr { + if (self->mCancelled || self->mOpenPromise.IsEmpty()) { return PDocumentChannelParent::RedirectToRealChannelPromise:: CreateAndResolve(NS_BINDING_ABORTED, __func__); } - return aBridge->RedirectToRealChannel(std::move(endpoints), - aRedirectFlags, aLoadFlags); + // This promise will be passed on the promise listener which will + // resolve this promise for us. + auto promise = MakeRefPtr< + PDocumentChannelParent::RedirectToRealChannelPromise::Private>( + __func__); + self->mOpenPromise.Resolve( + OpenPromiseSucceededType( + {std::move(endpoints), aRedirectFlags, aLoadFlags, promise}), + __func__); + return promise; }, - [](bool aDummy) { + [](nsresult aDummy) { return PDocumentChannelParent::RedirectToRealChannelPromise:: CreateAndReject(ipc::ResponseRejectReason::ActorDestroyed, __func__); @@ -1584,6 +1596,7 @@ void DocumentLoadListener::TriggerRedirectToRealChannel( NS_IMETHODIMP DocumentLoadListener::OnStartRequest(nsIRequest* aRequest) { LOG(("DocumentLoadListener OnStartRequest [this=%p]", this)); + nsCOMPtr multiPartChannel = do_QueryInterface(aRequest); if (multiPartChannel) { multiPartChannel->GetBaseChannel(getter_AddRefs(mChannel)); @@ -1591,11 +1604,8 @@ DocumentLoadListener::OnStartRequest(nsIRequest* aRequest) { mChannel = do_QueryInterface(aRequest); } MOZ_DIAGNOSTIC_ASSERT(mChannel); - RefPtr httpChannel = do_QueryObject(mChannel); - if (!mDocumentChannelBridge && !mPendingDocumentChannelBridgeProcess) { - return NS_ERROR_UNEXPECTED; - } + RefPtr httpChannel = do_QueryObject(mChannel); // Enforce CSP frame-ancestors and x-frame-options checks which // might cancel the channel. @@ -1610,21 +1620,23 @@ DocumentLoadListener::OnStartRequest(nsIRequest* aRequest) { nsresult status = NS_OK; aRequest->GetStatus(&status); if (status == NS_ERROR_NO_CONTENT) { - DisconnectChildListeners(status, status); + DisconnectListeners(status, status); return NS_OK; } mStreamListenerFunctions.AppendElement(StreamListenerFunction{ VariantIndex<0>{}, OnStartRequestParams{aRequest}}); - if (!mInitiatedRedirectToRealChannel) { - mChannel->Suspend(); - } else { - // This can be called multiple time if we have a multipart - // decoder. Since we've already added the reqest to - // mStreamListenerFunctions, we don't need to do anything else. + if (mOpenPromise.IsEmpty() || mInitiatedRedirectToRealChannel) { + // I we have already resolved the promise, there's no point to continue + // attempting a process switch or redirecting to the real channel. + // We can also have multiple calls to OnStartRequest when dealing with + // multi-part content, but only want to redirect once. return NS_OK; } + + mChannel->Suspend(); + mInitiatedRedirectToRealChannel = true; // Determine if a new process needs to be spawned. If it does, this will @@ -1715,7 +1727,7 @@ DocumentLoadListener::OnAfterLastPart(nsresult aStatus) { // channel, then it means we never got OnStartRequest (maybe a problem?) // and we retargeted everything. LOG(("DocumentLoadListener Disconnecting child")); - DisconnectChildListeners(NS_BINDING_RETARGETED, NS_OK); + DisconnectListeners(NS_BINDING_RETARGETED, NS_OK); return NS_OK; } mStreamListenerFunctions.AppendElement(StreamListenerFunction{ @@ -1724,13 +1736,6 @@ DocumentLoadListener::OnAfterLastPart(nsresult aStatus) { return NS_OK; } -NS_IMETHODIMP -DocumentLoadListener::SetParentListener( - mozilla::net::ParentChannelListener* listener) { - // We don't need this (do we?) - return NS_OK; -} - NS_IMETHODIMP DocumentLoadListener::GetInterface(const nsIID& aIID, void** result) { RefPtr browsingContext = @@ -1743,6 +1748,17 @@ DocumentLoadListener::GetInterface(const nsIID& aIID, void** result) { return QueryInterface(aIID, result); } +//////////////////////////////////////////////////////////////////////////////// +// nsIParentChannel +//////////////////////////////////////////////////////////////////////////////// + +NS_IMETHODIMP +DocumentLoadListener::SetParentListener( + mozilla::net::ParentChannelListener* listener) { + // We don't need this (do we?) + return NS_OK; +} + // Rather than forwarding all these nsIParentChannel functions to the child, // we cache a list of them, and then ask the 'real' channel to forward them // for us after it's created. @@ -1791,12 +1807,14 @@ DocumentLoadListener::NotifyClassificationFlags(uint32_t aClassificationFlags, NS_IMETHODIMP DocumentLoadListener::Delete() { - if (mDocumentChannelBridge) { - mDocumentChannelBridge->Delete(); - } + MOZ_ASSERT_UNREACHABLE("This method is unused"); return NS_OK; } +//////////////////////////////////////////////////////////////////////////////// +// nsIChannelEventSink +//////////////////////////////////////////////////////////////////////////////// + NS_IMETHODIMP DocumentLoadListener::AsyncOnChannelRedirect( nsIChannel* aOldChannel, nsIChannel* aNewChannel, uint32_t aFlags, diff --git a/netwerk/ipc/DocumentLoadListener.h b/netwerk/ipc/DocumentLoadListener.h index 0b57b3fb1abe..bae0f0fe74b4 100644 --- a/netwerk/ipc/DocumentLoadListener.h +++ b/netwerk/ipc/DocumentLoadListener.h @@ -9,20 +9,19 @@ #include "mozilla/MozPromise.h" #include "mozilla/Variant.h" +#include "mozilla/dom/SessionHistoryEntry.h" #include "mozilla/net/NeckoCommon.h" #include "mozilla/net/NeckoParent.h" #include "mozilla/net/PDocumentChannelParent.h" #include "mozilla/net/ParentChannelListener.h" -#include "mozilla/net/ADocumentChannelBridge.h" -#include "mozilla/dom/SessionHistoryEntry.h" #include "nsDOMNavigationTiming.h" +#include "nsIBrowser.h" #include "nsIInterfaceRequestor.h" +#include "nsIMultiPartChannel.h" #include "nsIParentChannel.h" #include "nsIParentRedirectingChannel.h" -#include "nsIRedirectResultListener.h" -#include "nsIMultiPartChannel.h" #include "nsIProgressEventSink.h" -#include "nsIBrowser.h" +#include "nsIRedirectResultListener.h" #define DOCUMENT_LOAD_LISTENER_IID \ { \ @@ -94,21 +93,48 @@ class DocumentLoadListener : public nsIInterfaceRequestor, public nsIMultiPartChannelListener, public nsIProgressEventSink { public: - explicit DocumentLoadListener(dom::CanonicalBrowsingContext* aBrowsingContext, - ADocumentChannelBridge* aBridge); + explicit DocumentLoadListener( + dom::CanonicalBrowsingContext* aBrowsingContext); + + struct OpenPromiseSucceededType { + nsTArray> + mStreamFilterEndpoints; + uint32_t mRedirectFlags; + uint32_t mLoadFlags; + RefPtr + mPromise; + }; + struct OpenPromiseFailedType { + nsresult mStatus; + nsresult mLoadGroupStatus; + }; + + typedef MozPromise + OpenPromise; // Creates the channel, and then calls AsyncOpen on it. - bool Open(nsDocShellLoadState* aLoadState, uint32_t aCacheKey, - const Maybe& aChannelId, const TimeStamp& aAsyncOpenTime, - nsDOMNavigationTiming* aTiming, Maybe&& aInfo, - uint64_t aOuterWindowId, bool aHasGesture, Maybe aUriModified, - Maybe aIsXFOError, nsresult* aRv); + // The DocumentLoadListener will require additional process from the consumer + // in order to complete the redirect to the end channel. This is done by + // returning a RedirectToRealChannelPromise and then waiting for it to be + // resolved or rejected accordingly. + // Once that promise is resolved; the consumer no longer needs to hold a + // reference to the DocumentLoadListener nor will the consumer required to be + // used again. + RefPtr Open(nsDocShellLoadState* aLoadState, uint32_t aCacheKey, + const Maybe& aChannelId, + const TimeStamp& aAsyncOpenTime, + nsDOMNavigationTiming* aTiming, + Maybe&& aInfo, + uint64_t aOuterWindowId, bool aHasGesture, + Maybe aUriModified, Maybe aIsXFOError, + base::ProcessId aPid, nsresult* aRv); // Creates a DocumentLoadListener directly in the parent process without // an associated DocumentChannelBridge. // If successful it registers a unique identifier (return in aOutIdent) to - // keep it alive until a future bridge can attach to it, or we fail and clean - // up. + // keep it alive until a future bridge can attach to it, or we fail and + // clean up. static bool OpenFromParent(dom::CanonicalBrowsingContext* aBrowsingContext, nsDocShellLoadState* aLoadState, uint64_t aOuterWindowId, uint32_t* aOutIdent); @@ -119,9 +145,12 @@ class DocumentLoadListener : public nsIInterfaceRequestor, static void CleanupParentLoadAttempt(uint32_t aLoadIdent); // Looks up aLoadIdent to find the associated, cleans up the registration - // and attaches aBridge as the listener. - static already_AddRefed ClaimParentLoad( - uint32_t aLoadIdent, ADocumentChannelBridge* aBridge); + static RefPtr ClaimParentLoad(DocumentLoadListener** aListener, + uint32_t aLoadIdent); + + // Called by the DocumentChannelParent if actor got destroyed or the parent + // channel got deleted. + void Abort(); NS_DECL_ISUPPORTS NS_DECL_NSIREQUESTOBSERVER @@ -143,6 +172,7 @@ class DocumentLoadListener : public nsIInterfaceRequestor, NS_DECLARE_STATIC_IID_ACCESSOR(DOCUMENT_LOAD_LISTENER_IID) + // Called by the DocumentChannel if cancelled. void Cancel(const nsresult& status); nsIChannel* GetChannel() const { return mChannel; } @@ -184,27 +214,16 @@ class DocumentLoadListener : public nsIInterfaceRequestor, return NS_OK; } - // Called by the bridge when it disconnects, so that we can drop - // our reference to it. - void DocumentChannelBridgeDisconnected(); - - void DisconnectChildListeners(nsresult aStatus, nsresult aLoadGroupStatus); - base::ProcessId OtherPid() const { - if (mDocumentChannelBridge) { - return mDocumentChannelBridge->OtherPid(); - } if (mPendingDocumentChannelBridgeProcess) { return *mPendingDocumentChannelBridgeProcess; } - return 0; + return mOtherPid; } [[nodiscard]] RefPtr AttachStreamFilter( base::ProcessId aChildProcessId); - using ParentEndpoint = ipc::Endpoint; - // Serializes all data needed to setup the new replacement channel // in the content process into the RedirectToRealChannelArgs struct. void SerializeRedirectData(RedirectToRealChannelArgs& aArgs, @@ -217,9 +236,13 @@ class DocumentLoadListener : public nsIInterfaceRequestor, base::ProcessId aPendingBridgeProcess); virtual ~DocumentLoadListener(); + private: + friend class ParentProcessDocumentOpenInfo; + // Will reject the promise to notify the DLL consumer that we are done. + void DisconnectListeners(nsresult aStatus, nsresult aLoadGroupStatus); // Called when we were created without a document channel bridge, // and now it has been created and attached. - void NotifyBridgeConnected(ADocumentChannelBridge* aBridge); + void NotifyBridgeConnected(); // Called when we were created without a document channel bridge, // and creation has failed, and won't ever be attached. @@ -229,9 +252,7 @@ class DocumentLoadListener : public nsIInterfaceRequestor, // waiting for a pending one if necessary. // If we've failed to create a bridge, or a bridge has already been // detached then rejects. - typedef MozPromise, bool, false> - EnsureBridgePromise; - RefPtr EnsureBridge(); + RefPtr EnsureBridge(); // Initiates the switch from DocumentChannel to the real protocol-specific // channel, and ensures that RedirectToRealChannelFinished is called when @@ -260,6 +281,7 @@ class DocumentLoadListener : public nsIInterfaceRequestor, // A helper for TriggerRedirectToRealChannel that abstracts over // the same-process and cross-process switch cases and returns // a single promise to wait on. + using ParentEndpoint = ipc::Endpoint; RefPtr RedirectToRealChannel(uint32_t aRedirectFlags, uint32_t aLoadFlags, const Maybe& aDestinationProcess, @@ -276,6 +298,8 @@ class DocumentLoadListener : public nsIInterfaceRequestor, bool HasCrossOriginOpenerPolicyMismatch() const; void ApplyPendingFunctions(nsISupports* aChannel) const; + void Disconnect(); + // This defines a variant that describes all the attribute setters (and their // parameters) from nsIParentChannel // @@ -374,12 +398,6 @@ class DocumentLoadListener : public nsIInterfaceRequestor, // replaces us. RefPtr mParentChannelListener; - // The bridge to the nsIChannel in the originating docshell. - // This reference forms a cycle with the bridge, and we expect - // the bridge to call DisonnectDocumentChannelBridge when it - // shuts down to break this. - RefPtr mDocumentChannelBridge; - // If we were created without a bridge, then this is set // to Some() with the process id of the content process // that will be creating our bridge soon. @@ -387,7 +405,7 @@ class DocumentLoadListener : public nsIInterfaceRequestor, // Holds a promise for callers that want to wait on the document // channel bridge becoming available. - MozPromiseHolder mBridgePromise; + MozPromiseHolder mBridgePromise; // The original URI of the current channel. If there are redirects, // then the value on the channel gets overwritten with the original @@ -447,6 +465,17 @@ class DocumentLoadListener : public nsIInterfaceRequestor, // True if cancelled. bool mCancelled = false; + + // The process id of the content process that we are being called from + // or 0 initiated from a parent process load. + base::ProcessId mOtherPid = 0; + + void RejectOpenPromiseIfExists(nsresult aStatus, nsresult aLoadGroupStatus, + const char* aLocation) { + mOpenPromise.RejectIfExists( + OpenPromiseFailedType({aStatus, aLoadGroupStatus}), aLocation); + } + MozPromiseHolder mOpenPromise; }; NS_DEFINE_STATIC_IID_ACCESSOR(DocumentLoadListener, DOCUMENT_LOAD_LISTENER_IID) diff --git a/netwerk/ipc/ParentProcessDocumentChannel.cpp b/netwerk/ipc/ParentProcessDocumentChannel.cpp index 6412bf3ef706..d5019d499e88 100644 --- a/netwerk/ipc/ParentProcessDocumentChannel.cpp +++ b/netwerk/ipc/ParentProcessDocumentChannel.cpp @@ -56,6 +56,7 @@ ParentProcessDocumentChannel::RedirectToRealChannel( msg.Insert( "Attempt to load a non-authorised load in the parent process: ", 0); NS_ASSERTION(false, msg.get()); + RemoveObserver(); return PDocumentChannelParent::RedirectToRealChannelPromise:: CreateAndResolve(NS_ERROR_CONTENT_BLOCKED, __func__); } @@ -68,6 +69,11 @@ ParentProcessDocumentChannel::RedirectToRealChannel( nsresult rv = gHttpHandler->AsyncOnChannelRedirect(this, channel, aRedirectFlags); if (NS_FAILED(rv)) { + LOG( + ("ParentProcessDocumentChannel RedirectToRealChannel " + "AsyncOnChannelRedirect failed [this=%p " + "aRv=%d]", + this, int(rv))); OnRedirectVerifyCallback(rv); } @@ -81,11 +87,11 @@ ParentProcessDocumentChannel::OnRedirectVerifyCallback(nsresult aResult) { "aResult=%d]", this, int(aResult))); - MOZ_ASSERT(mCanceled || mDocumentLoadListener); + MOZ_ASSERT(mDocumentLoadListener); if (NS_FAILED(aResult)) { Cancel(aResult); - } else if (mCanceled && NS_SUCCEEDED(aResult)) { + } else if (mCanceled) { aResult = NS_ERROR_ABORT; } else { const nsCOMPtr channel = mDocumentLoadListener->GetChannel(); @@ -113,7 +119,7 @@ ParentProcessDocumentChannel::OnRedirectVerifyCallback(nsresult aResult) { mLoadGroup = nullptr; mListener = nullptr; mCallbacks = nullptr; - DisconnectDocumentLoadListener(); + RemoveObserver(); mPromise.ResolveIfExists(aResult, __func__); @@ -124,7 +130,7 @@ NS_IMETHODIMP ParentProcessDocumentChannel::AsyncOpen( nsIStreamListener* aListener) { LOG(("ParentProcessDocumentChannel AsyncOpen [this=%p]", this)); mDocumentLoadListener = new DocumentLoadListener( - GetDocShell()->GetBrowsingContext()->Canonical(), this); + GetDocShell()->GetBrowsingContext()->Canonical()); LOG(("Created PPDocumentChannel with listener=%p", mDocumentLoadListener.get())); @@ -140,15 +146,17 @@ NS_IMETHODIMP ParentProcessDocumentChannel::AsyncOpen( nsresult rv = NS_OK; Maybe initialClientInfo = mInitialClientInfo; - if (!mDocumentLoadListener->Open( - mLoadState, mCacheKey, Some(mChannelId), mAsyncOpenTime, mTiming, - std::move(initialClientInfo), GetDocShell()->GetOuterWindowID(), - GetDocShell() - ->GetBrowsingContext() - ->HasValidTransientUserGestureActivation(), - Some(mUriModified), Some(mIsXFOError), &rv)) { - MOZ_ASSERT(NS_FAILED(rv)); - DisconnectDocumentLoadListener(); + auto promise = mDocumentLoadListener->Open( + mLoadState, mCacheKey, Some(mChannelId), mAsyncOpenTime, mTiming, + std::move(initialClientInfo), GetDocShell()->GetOuterWindowID(), + GetDocShell() + ->GetBrowsingContext() + ->HasValidTransientUserGestureActivation(), + Some(mUriModified), Some(mIsXFOError), 0 /* ProcessId */, &rv); + if (NS_FAILED(rv)) { + MOZ_ASSERT(!promise); + mDocumentLoadListener = nullptr; + RemoveObserver(); return rv; } @@ -156,6 +164,32 @@ NS_IMETHODIMP ParentProcessDocumentChannel::AsyncOpen( if (mLoadGroup) { mLoadGroup->AddRequest(this, nullptr); } + + RefPtr self = this; + promise + ->Then( + GetCurrentThreadSerialEventTarget(), __func__, + [self]( + DocumentLoadListener::OpenPromiseSucceededType&& aResolveValue) { + self->mPromiseRequest.Complete(); + // The DLL is waiting for us to resolve the + // PDocumentChannel::RedirectToRealChannelPromise given as + // parameter. + auto promise = self->RedirectToRealChannel( + std::move(aResolveValue.mStreamFilterEndpoints), + aResolveValue.mRedirectFlags, aResolveValue.mLoadFlags); + // We chain the promise the DLL is waiting on to the one returned by + // RedirectToRealChannel. As soon as the promise returned is + // resolved or rejected, so will the DLL's promise. + promise->ChainTo(aResolveValue.mPromise.forget(), __func__); + }, + [self](DocumentLoadListener::OpenPromiseFailedType&& aRejectValue) { + self->mPromiseRequest.Complete(); + self->DisconnectChildListeners(aRejectValue.mStatus, + aRejectValue.mLoadGroupStatus); + self->RemoveObserver(); + }) + ->Track(mPromiseRequest); return NS_OK; } @@ -167,19 +201,10 @@ NS_IMETHODIMP ParentProcessDocumentChannel::Cancel(nsresult aStatus) { mCanceled = true; mDocumentLoadListener->Cancel(aStatus); - - ShutdownListeners(aStatus); - - return NS_OK; -} - -void ParentProcessDocumentChannel::DisconnectDocumentLoadListener() { - if (!mDocumentLoadListener) { - return; + if (!mPromiseRequest.Exists()) { + DisconnectChildListeners(aStatus, aStatus); } - mDocumentLoadListener->DocumentChannelBridgeDisconnected(); - mDocumentLoadListener = nullptr; - RemoveObserver(); + return NS_OK; } void ParentProcessDocumentChannel::RemoveObserver() { @@ -191,6 +216,7 @@ void ParentProcessDocumentChannel::RemoveObserver() { //////////////////////////////////////////////////////////////////////////////// // nsIObserver +//////////////////////////////////////////////////////////////////////////////// NS_IMETHODIMP ParentProcessDocumentChannel::Observe(nsISupports* aSubject, const char* aTopic, diff --git a/netwerk/ipc/ParentProcessDocumentChannel.h b/netwerk/ipc/ParentProcessDocumentChannel.h index 825ed3fa2884..b41d59e28d7a 100644 --- a/netwerk/ipc/ParentProcessDocumentChannel.h +++ b/netwerk/ipc/ParentProcessDocumentChannel.h @@ -8,7 +8,6 @@ #define mozilla_net_ParentProcessDocumentChannel_h #include "ProtocolUtils.h" -#include "mozilla/net/ADocumentChannelBridge.h" #include "mozilla/net/DocumentChannel.h" #include "mozilla/net/DocumentLoadListener.h" #include "nsIObserver.h" @@ -18,8 +17,7 @@ namespace net { class ParentProcessDocumentChannel : public DocumentChannel, public nsIAsyncVerifyRedirectCallback, - public nsIObserver, - public ADocumentChannelBridge { + public nsIObserver { public: ParentProcessDocumentChannel(nsDocShellLoadState* aLoadState, class LoadInfo* aLoadInfo, @@ -37,23 +35,13 @@ class ParentProcessDocumentChannel : public DocumentChannel, RedirectToRealChannel( nsTArray>&& aStreamFilterEndpoints, - uint32_t aRedirectFlags, uint32_t aLoadFlags) override; - - void DisconnectChildListeners(nsresult aStatus, - nsresult aLoadGroupStatus) override { - DocumentChannel::DisconnectChildListeners(aStatus, aLoadGroupStatus); - RemoveObserver(); - mDocumentLoadListener = nullptr; - } - void Delete() override {} + uint32_t aRedirectFlags, uint32_t aLoadFlags); void DeleteIPDL() override { mPromise.ResolveIfExists(NS_BINDING_ABORTED, __func__); } - base::ProcessId OtherPid() const override { return 0; } private: virtual ~ParentProcessDocumentChannel(); - void DisconnectDocumentLoadListener(); void RemoveObserver(); RefPtr mDocumentLoadListener; @@ -61,6 +49,7 @@ class ParentProcessDocumentChannel : public DocumentChannel, mStreamFilterEndpoints; MozPromiseHolder mPromise; + MozPromiseRequestHolder mPromiseRequest; bool mRequestObserversCalled = false; }; diff --git a/netwerk/ipc/moz.build b/netwerk/ipc/moz.build index 39af5a6245d8..7db10365375c 100644 --- a/netwerk/ipc/moz.build +++ b/netwerk/ipc/moz.build @@ -5,7 +5,6 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. EXPORTS.mozilla.net += [ - 'ADocumentChannelBridge.h', 'ChannelEventQueue.h', 'DocumentChannel.h', 'DocumentChannelChild.h',