From 431beb8c3b670e0da8507ac619a56bb690f91b06 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Tue, 25 May 2021 15:54:48 +0000 Subject: [PATCH] Bug 1709346 - Part 1: Track BrowsingContextWebProgress for subframes, r=mattwoodrow,kmag,necko-reviewers This allows loads to be tracked as they are ongoing on a per-context basis in the parent process, and for events to be generated for each subframe as it is destroyed. This patch also stops sending the `IsLoadingDocument` flag on the request to the main process and removes RemoteWebProgress, as they are no longer necessary due to being tracked directly. Finally this patch also adds some logging to BrowsingContextWebProgress to make it easier to diagnose this type of issue in the future. Differential Revision: https://phabricator.services.mozilla.com/D115706 --- docshell/base/BrowsingContext.cpp | 6 +- docshell/base/BrowsingContextWebProgress.cpp | 282 +++++++++++++++---- docshell/base/BrowsingContextWebProgress.h | 56 +++- docshell/base/CanonicalBrowsingContext.cpp | 56 +++- docshell/base/CanonicalBrowsingContext.h | 2 + dom/ipc/BrowserChild.cpp | 5 +- dom/ipc/BrowserParent.cpp | 83 +++--- dom/ipc/BrowserParent.h | 6 +- dom/ipc/PBrowser.ipdl | 1 - dom/ipc/RemoteWebProgress.cpp | 58 ---- dom/ipc/RemoteWebProgress.h | 39 --- dom/ipc/WindowGlobalParent.cpp | 10 +- dom/ipc/moz.build | 2 - netwerk/ipc/DocumentLoadListener.cpp | 32 +-- 14 files changed, 378 insertions(+), 260 deletions(-) delete mode 100644 dom/ipc/RemoteWebProgress.cpp delete mode 100644 dom/ipc/RemoteWebProgress.h diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 41d235b8b9e5..aa586050ac22 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -779,8 +779,10 @@ void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) { } }); - if (IsTopContent() && !Canonical()->GetWebProgress()) { - Canonical()->mWebProgress = new BrowsingContextWebProgress(); + // We want to create a BrowsingContextWebProgress for all content + // BrowsingContexts. + if (IsContent() && !Canonical()->mWebProgress) { + Canonical()->mWebProgress = new BrowsingContextWebProgress(Canonical()); } } diff --git a/docshell/base/BrowsingContextWebProgress.cpp b/docshell/base/BrowsingContextWebProgress.cpp index 5d1cbb916913..622c0bde5de1 100644 --- a/docshell/base/BrowsingContextWebProgress.cpp +++ b/docshell/base/BrowsingContextWebProgress.cpp @@ -3,19 +3,44 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "BrowsingContextWebProgress.h" +#include "mozilla/dom/CanonicalBrowsingContext.h" +#include "mozilla/Logging.h" +#include "nsCOMPtr.h" +#include "nsIWebProgressListener.h" +#include "nsString.h" +#include "nsPrintfCString.h" +#include "nsIChannel.h" +#include "xptinfo.h" namespace mozilla { namespace dom { -NS_IMPL_ADDREF(BrowsingContextWebProgress) -NS_IMPL_RELEASE(BrowsingContextWebProgress) +static mozilla::LazyLogModule gBCWebProgressLog("BCWebProgress"); -NS_INTERFACE_MAP_BEGIN(BrowsingContextWebProgress) +static nsCString DescribeBrowsingContext(CanonicalBrowsingContext* aContext); +static nsCString DescribeWebProgress(nsIWebProgress* aWebProgress); +static nsCString DescribeRequest(nsIRequest* aRequest); +static nsCString DescribeWebProgressFlags(uint32_t aFlags, + const nsACString& aPrefix); +static nsCString DescribeError(nsresult aError); + +NS_IMPL_CYCLE_COLLECTION(BrowsingContextWebProgress, mCurrentBrowsingContext) + +NS_IMPL_CYCLE_COLLECTING_ADDREF(BrowsingContextWebProgress) +NS_IMPL_CYCLE_COLLECTING_RELEASE(BrowsingContextWebProgress) + +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(BrowsingContextWebProgress) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIWebProgress) NS_INTERFACE_MAP_ENTRY(nsIWebProgress) NS_INTERFACE_MAP_ENTRY(nsIWebProgressListener) NS_INTERFACE_MAP_END +BrowsingContextWebProgress::BrowsingContextWebProgress( + CanonicalBrowsingContext* aBrowsingContext) + : mCurrentBrowsingContext(aBrowsingContext) {} + +BrowsingContextWebProgress::~BrowsingContextWebProgress() = default; + NS_IMETHODIMP BrowsingContextWebProgress::AddProgressListener( nsIWebProgressListener* aListener, uint32_t aNotifyMask) { nsWeakPtr listener = do_GetWeakReference(aListener); @@ -48,18 +73,18 @@ NS_IMETHODIMP BrowsingContextWebProgress::GetDOMWindow( } NS_IMETHODIMP BrowsingContextWebProgress::GetIsTopLevel(bool* aIsTopLevel) { - *aIsTopLevel = true; + *aIsTopLevel = mCurrentBrowsingContext->IsTop(); return NS_OK; } NS_IMETHODIMP BrowsingContextWebProgress::GetIsLoadingDocument( bool* aIsLoadingDocument) { - *aIsLoadingDocument = false; + *aIsLoadingDocument = mIsLoadingDocument; return NS_OK; } NS_IMETHODIMP BrowsingContextWebProgress::GetLoadType(uint32_t* aLoadType) { - *aLoadType = 0; + *aLoadType = mLoadType; return NS_OK; } @@ -94,28 +119,38 @@ void BrowsingContextWebProgress::UpdateAndNotifyListeners( } mListenerInfoList.Compact(); + + // Notify the parent BrowsingContextWebProgress of the event to continue + // propagating. + auto* parent = mCurrentBrowsingContext->GetParent(); + if (parent && parent->GetWebProgress()) { + aCallback(parent->GetWebProgress()); + } } void BrowsingContextWebProgress::ContextDiscarded() { - if (!mAwaitingStop) { + if (!mIsLoadingDocument) { return; } + // If our BrowsingContext is being discarded while still loading a document, + // fire a synthetic `STATE_STOP` to end the ongoing load. + MOZ_LOG(gBCWebProgressLog, LogLevel::Info, + ("Discarded while loading %s", + DescribeBrowsingContext(mCurrentBrowsingContext).get())); + // This matches what nsDocLoader::doStopDocumentLoad does, except we don't // bother notifying for `STATE_STOP | STATE_IS_DOCUMENT`, // nsBrowserStatusFilter would filter it out before it gets to the parent // process. - const int32_t flags = STATE_STOP | STATE_IS_WINDOW | STATE_IS_NETWORK; - UpdateAndNotifyListeners(((flags >> 16) & nsIWebProgress::NOTIFY_STATE_ALL), - [&](nsIWebProgressListener* listener) { - // TODO(emilio): We might want to stash the - // request from OnStateChange on a member or - // something if having no request causes trouble - // here. Ditto for the webprogress instance. - listener->OnStateChange(this, - /* aRequest = */ nullptr, - flags, NS_ERROR_ABORT); - }); + OnStateChange(this, mLoadingDocumentRequest, + STATE_STOP | STATE_IS_WINDOW | STATE_IS_NETWORK, + NS_ERROR_ABORT); +} + +void BrowsingContextWebProgress::ContextReplaced( + CanonicalBrowsingContext* aNewContext) { + mCurrentBrowsingContext = aNewContext; } //////////////////////////////////////////////////////////////////////////////// @@ -126,42 +161,62 @@ BrowsingContextWebProgress::OnStateChange(nsIWebProgress* aWebProgress, nsIRequest* aRequest, uint32_t aStateFlags, nsresult aStatus) { - const uint32_t startDocumentFlags = - nsIWebProgressListener::STATE_START | - nsIWebProgressListener::STATE_IS_DOCUMENT | - nsIWebProgressListener::STATE_IS_REQUEST | - nsIWebProgressListener::STATE_IS_WINDOW | - nsIWebProgressListener::STATE_IS_NETWORK; - bool isTopLevel = false; - nsresult rv = aWebProgress->GetIsTopLevel(&isTopLevel); - NS_ENSURE_SUCCESS(rv, rv); - bool isTopLevelStartDocumentEvent = - (aStateFlags & startDocumentFlags) == startDocumentFlags && isTopLevel; - // If we receive a matching STATE_START for a top-level document event, - // and we are currently not suspending this event, start suspending all - // further matching STATE_START events after this one. - if (isTopLevelStartDocumentEvent && !mAwaitingStop) { - mAwaitingStop = true; - } else if (mAwaitingStop) { - // If we are currently suspending matching STATE_START events, check if this - // is a corresponding STATE_STOP event. - const uint32_t stopWindowFlags = nsIWebProgressListener::STATE_STOP | - nsIWebProgressListener::STATE_IS_WINDOW; - bool isTopLevelStopWindowEvent = - (aStateFlags & stopWindowFlags) == stopWindowFlags && isTopLevel; - if (isTopLevelStopWindowEvent) { - // If this is a STATE_STOP event corresponding to the initial STATE_START - // event, stop suspending matching STATE_START events. - mAwaitingStop = false; - } else if (isTopLevelStartDocumentEvent) { - // We have received a matching STATE_START event at least twice, but - // haven't received the corresponding STATE_STOP event for the first one. - // Don't let this event through. This is probably a duplicate event from - // the new BrowserParent due to a process switch. - return NS_OK; + MOZ_LOG( + gBCWebProgressLog, LogLevel::Info, + ("OnStateChange(%s, %s, %s, %s) on %s", + DescribeWebProgress(aWebProgress).get(), DescribeRequest(aRequest).get(), + DescribeWebProgressFlags(aStateFlags, "STATE_"_ns).get(), + DescribeError(aStatus).get(), + DescribeBrowsingContext(mCurrentBrowsingContext).get())); + + bool targetIsThis = aWebProgress == this; + + // We may receive a request from an in-process nsDocShell which doesn't have + // `aWebProgress == this` which we should still consider as targeting + // ourselves. + if (nsCOMPtr docShell = do_QueryInterface(aWebProgress); + docShell && docShell->GetBrowsingContext() == mCurrentBrowsingContext) { + targetIsThis = true; + aWebProgress->GetLoadType(&mLoadType); + } + + // Track `mIsLoadingDocument` based on the notifications we've received so far + // if the nsIWebProgress being targeted is this one. + if (targetIsThis) { + constexpr uint32_t startFlags = nsIWebProgressListener::STATE_START | + nsIWebProgressListener::STATE_IS_DOCUMENT | + nsIWebProgressListener::STATE_IS_REQUEST | + nsIWebProgressListener::STATE_IS_WINDOW | + nsIWebProgressListener::STATE_IS_NETWORK; + constexpr uint32_t stopFlags = nsIWebProgressListener::STATE_STOP | + nsIWebProgressListener::STATE_IS_WINDOW; + constexpr uint32_t redirectFlags = + nsIWebProgressListener::STATE_IS_REDIRECTED_DOCUMENT; + if ((aStateFlags & startFlags) == startFlags) { + if (mIsLoadingDocument) { + // We received a duplicate `STATE_START` notification, silence this + // notification until we receive the matching `STATE_STOP` to not fire + // duplicate `STATE_START` notifications into frontend on process + // switches. + return NS_OK; + } + mIsLoadingDocument = true; + + // Record the request we started the load with, so we can emit a synthetic + // `STATE_STOP` notification if the BrowsingContext is discarded before + // the notification arrives. + mLoadingDocumentRequest = aRequest; + } else if ((aStateFlags & stopFlags) == stopFlags) { + // We've received a `STATE_STOP` notification targeting this web progress, + // clear our loading document flag. + mIsLoadingDocument = false; + mLoadingDocumentRequest = nullptr; + } else if (mIsLoadingDocument && + (aStateFlags & redirectFlags) == redirectFlags) { + // If we see a redirected document load, update the loading request which + // we'll emit the synthetic STATE_STOP notification with. + mLoadingDocumentRequest = aRequest; } - // Allow all other progress events that don't match top-level start document - // flags. } UpdateAndNotifyListeners( @@ -179,6 +234,12 @@ BrowsingContextWebProgress::OnProgressChange(nsIWebProgress* aWebProgress, int32_t aMaxSelfProgress, int32_t aCurTotalProgress, int32_t aMaxTotalProgress) { + MOZ_LOG( + gBCWebProgressLog, LogLevel::Info, + ("OnProgressChange(%s, %s, %d, %d, %d, %d) on %s", + DescribeWebProgress(aWebProgress).get(), DescribeRequest(aRequest).get(), + aCurSelfProgress, aMaxSelfProgress, aCurTotalProgress, aMaxTotalProgress, + DescribeBrowsingContext(mCurrentBrowsingContext).get())); UpdateAndNotifyListeners( nsIWebProgress::NOTIFY_PROGRESS, [&](nsIWebProgressListener* listener) { listener->OnProgressChange(aWebProgress, aRequest, aCurSelfProgress, @@ -193,6 +254,13 @@ BrowsingContextWebProgress::OnLocationChange(nsIWebProgress* aWebProgress, nsIRequest* aRequest, nsIURI* aLocation, uint32_t aFlags) { + MOZ_LOG( + gBCWebProgressLog, LogLevel::Info, + ("OnProgressChange(%s, %s, %s, %s) on %s", + DescribeWebProgress(aWebProgress).get(), DescribeRequest(aRequest).get(), + aLocation ? aLocation->GetSpecOrDefault().get() : "", + DescribeWebProgressFlags(aFlags, "LOCATION_CHANGE_"_ns).get(), + DescribeBrowsingContext(mCurrentBrowsingContext).get())); UpdateAndNotifyListeners( nsIWebProgress::NOTIFY_LOCATION, [&](nsIWebProgressListener* listener) { listener->OnLocationChange(aWebProgress, aRequest, aLocation, aFlags); @@ -205,6 +273,12 @@ BrowsingContextWebProgress::OnStatusChange(nsIWebProgress* aWebProgress, nsIRequest* aRequest, nsresult aStatus, const char16_t* aMessage) { + MOZ_LOG( + gBCWebProgressLog, LogLevel::Info, + ("OnStatusChange(%s, %s, %s, \"%s\") on %s", + DescribeWebProgress(aWebProgress).get(), DescribeRequest(aRequest).get(), + DescribeError(aStatus).get(), NS_ConvertUTF16toUTF8(aMessage).get(), + DescribeBrowsingContext(mCurrentBrowsingContext).get())); UpdateAndNotifyListeners( nsIWebProgress::NOTIFY_STATUS, [&](nsIWebProgressListener* listener) { listener->OnStatusChange(aWebProgress, aRequest, aStatus, aMessage); @@ -216,6 +290,11 @@ NS_IMETHODIMP BrowsingContextWebProgress::OnSecurityChange(nsIWebProgress* aWebProgress, nsIRequest* aRequest, uint32_t aState) { + MOZ_LOG( + gBCWebProgressLog, LogLevel::Info, + ("OnSecurityChange(%s, %s, %x) on %s", + DescribeWebProgress(aWebProgress).get(), DescribeRequest(aRequest).get(), + aState, DescribeBrowsingContext(mCurrentBrowsingContext).get())); UpdateAndNotifyListeners( nsIWebProgress::NOTIFY_SECURITY, [&](nsIWebProgressListener* listener) { listener->OnSecurityChange(aWebProgress, aRequest, aState); @@ -227,6 +306,11 @@ NS_IMETHODIMP BrowsingContextWebProgress::OnContentBlockingEvent(nsIWebProgress* aWebProgress, nsIRequest* aRequest, uint32_t aEvent) { + MOZ_LOG( + gBCWebProgressLog, LogLevel::Info, + ("OnContentBlockingEvent(%s, %s, %x) on %s", + DescribeWebProgress(aWebProgress).get(), DescribeRequest(aRequest).get(), + aEvent, DescribeBrowsingContext(mCurrentBrowsingContext).get())); UpdateAndNotifyListeners(nsIWebProgress::NOTIFY_CONTENT_BLOCKING, [&](nsIWebProgressListener* listener) { listener->OnContentBlockingEvent(aWebProgress, @@ -235,5 +319,97 @@ BrowsingContextWebProgress::OnContentBlockingEvent(nsIWebProgress* aWebProgress, return NS_OK; } +//////////////////////////////////////////////////////////////////////////////// +// Helper methods for notification logging + +static nsCString DescribeBrowsingContext(CanonicalBrowsingContext* aContext) { + if (!aContext) { + return ""_ns; + } + + nsCOMPtr currentURI = aContext->GetCurrentURI(); + return nsPrintfCString( + "{top:%d, id:%" PRIx64 ", url:%s}", aContext->IsTop(), aContext->Id(), + currentURI ? currentURI->GetSpecOrDefault().get() : ""); +} + +static nsCString DescribeWebProgress(nsIWebProgress* aWebProgress) { + if (!aWebProgress) { + return ""_ns; + } + + bool isTopLevel = false; + aWebProgress->GetIsTopLevel(&isTopLevel); + bool isLoadingDocument = false; + aWebProgress->GetIsLoadingDocument(&isLoadingDocument); + return nsPrintfCString("{isTopLevel:%d, isLoadingDocument:%d}", isTopLevel, + isLoadingDocument); +} + +static nsCString DescribeRequest(nsIRequest* aRequest) { + if (!aRequest) { + return ""_ns; + } + + nsCOMPtr channel = do_QueryInterface(aRequest); + if (!channel) { + return ""_ns; + } + + nsCOMPtr originalURI; + channel->GetOriginalURI(getter_AddRefs(originalURI)); + nsCOMPtr uri; + channel->GetURI(getter_AddRefs(uri)); + + return nsPrintfCString( + "{URI:%s, originalURI:%s}", + uri ? uri->GetSpecOrDefault().get() : "", + originalURI ? originalURI->GetSpecOrDefault().get() : ""); +} + +static nsCString DescribeWebProgressFlags(uint32_t aFlags, + const nsACString& aPrefix) { + nsCString flags; + uint32_t remaining = aFlags; + + // Hackily fetch the names of each constant from the XPT information used for + // reflecting it into JS. This doesn't need to be reliable and just exists as + // a logging aid. + // + // If a change to xpt in the future breaks this code, just delete it and + // replace it with a normal hex log. + if (const auto* ifaceInfo = + nsXPTInterfaceInfo::ByIID(NS_GET_IID(nsIWebProgressListener))) { + for (uint16_t i = 0; i < ifaceInfo->ConstantCount(); ++i) { + const auto& constInfo = ifaceInfo->Constant(i); + nsDependentCString name(constInfo.Name()); + if (!StringBeginsWith(name, aPrefix)) { + continue; + } + + if (remaining & constInfo.mValue) { + remaining &= ~constInfo.mValue; + if (!flags.IsEmpty()) { + flags.AppendLiteral("|"); + } + flags.Append(name); + } + } + } + if (remaining != 0 || flags.IsEmpty()) { + if (!flags.IsEmpty()) { + flags.AppendLiteral("|"); + } + flags.AppendInt(remaining, 16); + } + return flags; +} + +static nsCString DescribeError(nsresult aError) { + nsCString name; + GetErrorName(aError, name); + return name; +} + } // namespace dom } // namespace mozilla diff --git a/docshell/base/BrowsingContextWebProgress.h b/docshell/base/BrowsingContextWebProgress.h index fca2ad960807..210562d0e72a 100644 --- a/docshell/base/BrowsingContextWebProgress.h +++ b/docshell/base/BrowsingContextWebProgress.h @@ -9,18 +9,35 @@ #include "nsIWebProgressListener.h" #include "nsTObserverArray.h" #include "nsWeakReference.h" +#include "nsCycleCollectionParticipant.h" namespace mozilla { namespace dom { +class CanonicalBrowsingContext; + +/// Object acting as the nsIWebProgress instance for a BrowsingContext over its +/// lifetime. +/// +/// An active toplevel CanonicalBrowsingContext will always have a +/// BrowsingContextWebProgress, which will be moved between contexts as +/// BrowsingContextGroup-changing loads are performed. +/// +/// Subframes will only have a `BrowsingContextWebProgress` if they are loaded +/// in a content process, and will use the nsDocShell instead if they are loaded +/// in the parent process, as parent process documents cannot have or be +/// out-of-process iframes. class BrowsingContextWebProgress final : public nsIWebProgress, public nsIWebProgressListener { public: - NS_DECL_ISUPPORTS + NS_DECL_CYCLE_COLLECTING_ISUPPORTS + NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(BrowsingContextWebProgress, + nsIWebProgress) NS_DECL_NSIWEBPROGRESS NS_DECL_NSIWEBPROGRESSLISTENER - BrowsingContextWebProgress() = default; + explicit BrowsingContextWebProgress( + CanonicalBrowsingContext* aBrowsingContext); struct ListenerInfo { ListenerInfo(nsIWeakReference* aListener, unsigned long aNotifyMask) @@ -41,9 +58,12 @@ class BrowsingContextWebProgress final : public nsIWebProgress, }; void ContextDiscarded(); + void ContextReplaced(CanonicalBrowsingContext* aNewContext); + + void SetLoadType(uint32_t aLoadType) { mLoadType = aLoadType; } private: - virtual ~BrowsingContextWebProgress() = default; + virtual ~BrowsingContextWebProgress(); void UpdateAndNotifyListeners( uint32_t aFlag, @@ -52,16 +72,26 @@ class BrowsingContextWebProgress final : public nsIWebProgress, using ListenerArray = nsAutoTObserverArray; ListenerArray mListenerInfoList; - // This indicates whether we are currently suspending onStateChange top level - // STATE_START events for a document. We start suspending whenever we receive - // the first STATE_START event with the matching flags (see - // ::RecvOnStateChange for details), until we get a corresponding STATE_STOP - // event. In the meantime, if there other onStateChange events, this flag does - // not affect them. We do this to avoid duplicate onStateChange STATE_START - // events that happen during process switch. With this flag, we allow - // onStateChange STATE_START event from the old BrowserParent, but not the - // same event from the new BrowserParent during a process switch. - bool mAwaitingStop = false; + // The current BrowsingContext which owns this BrowsingContextWebProgress. + // This context may change during navigations and may not be fully attached at + // all times. + RefPtr mCurrentBrowsingContext; + + // The current request being actively loaded by the BrowsingContext. Only set + // while mIsLoadingDocument is true, and is used to fire STATE_STOP + // notifications if the BrowsingContext is discarded while the load is + // ongoing. + nsCOMPtr mLoadingDocumentRequest; + + // The most recent load type observed for this BrowsingContextWebProgress. + uint32_t mLoadType = 0; + + // Are we currently in the process of loading a document? This is true between + // the `STATE_START` notification from content and the `STATE_STOP` + // notification being received. Duplicate `STATE_START` events may be + // discarded while loading a document to avoid noise caused by process + // switches. + bool mIsLoadingDocument = false; }; } // namespace dom diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index a19bc9485230..f841e770c40d 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -157,16 +157,61 @@ nsISecureBrowserUI* CanonicalBrowsingContext::GetSecureBrowserUI() { return mSecureBrowserUI; } +namespace { +// The DocShellProgressBridge is attached to a root content docshell loaded in +// the parent process. Notifications are paired up with the docshell which they +// came from, so that they can be fired to the correct +// BrowsingContextWebProgress and bubble through this tree separately. +// +// Notifications are filtered by a nsBrowserStatusFilter before being received +// by the DocShellProgressBridge. +class DocShellProgressBridge : public nsIWebProgressListener { + public: + NS_DECL_ISUPPORTS + // NOTE: This relies in the expansion of `NS_FORWARD_SAFE` and all listener + // methods accepting an `aWebProgress` argument. If this changes in the + // future, this may need to be written manually. + NS_FORWARD_SAFE_NSIWEBPROGRESSLISTENER(GetTargetContext(aWebProgress)) + + explicit DocShellProgressBridge(uint64_t aTopContextId) + : mTopContextId(aTopContextId) {} + + private: + virtual ~DocShellProgressBridge() = default; + + nsIWebProgressListener* GetTargetContext(nsIWebProgress* aWebProgress) { + RefPtr context; + if (nsCOMPtr docShell = do_QueryInterface(aWebProgress)) { + context = docShell->GetBrowsingContext()->Canonical(); + } else { + context = CanonicalBrowsingContext::Get(mTopContextId); + } + return context && !context->IsDiscarded() ? context->GetWebProgress() + : nullptr; + } + + uint64_t mTopContextId = 0; +}; + +NS_IMPL_ISUPPORTS(DocShellProgressBridge, nsIWebProgressListener) +} // namespace + void CanonicalBrowsingContext::MaybeAddAsProgressListener( nsIWebProgress* aWebProgress) { - if (!GetWebProgress()) { + // Only add as a listener if the created docshell is a toplevel content + // docshell. We'll get notifications for all of our subframes through a single + // listener. + if (!IsTopContent()) { return; } - if (!mStatusFilter) { + + if (!mDocShellProgressBridge) { + mDocShellProgressBridge = new DocShellProgressBridge(Id()); mStatusFilter = new nsBrowserStatusFilter(); - mStatusFilter->AddProgressListener(GetWebProgress(), + mStatusFilter->AddProgressListener(mDocShellProgressBridge, nsIWebProgress::NOTIFY_ALL); } + aWebProgress->AddProgressListener(mStatusFilter, nsIWebProgress::NOTIFY_ALL); } @@ -177,9 +222,10 @@ void CanonicalBrowsingContext::ReplacedBy( MOZ_ASSERT(!aNewContext->mSessionHistory); MOZ_ASSERT(IsTop() && aNewContext->IsTop()); if (mStatusFilter) { - mStatusFilter->RemoveProgressListener(mWebProgress); + mStatusFilter->RemoveProgressListener(mDocShellProgressBridge); mStatusFilter = nullptr; } + mWebProgress->ContextReplaced(aNewContext); aNewContext->mWebProgress = std::move(mWebProgress); // Use the Transaction for the fields which need to be updated whether or not @@ -2234,7 +2280,7 @@ bool CanonicalBrowsingContext::AllowedInBFCache( NS_IMPL_CYCLE_COLLECTION_INHERITED(CanonicalBrowsingContext, BrowsingContext, mSessionHistory, mContainerFeaturePolicy, - mCurrentBrowserParent) + mCurrentBrowserParent, mWebProgress) NS_IMPL_ADDREF_INHERITED(CanonicalBrowsingContext, BrowsingContext) NS_IMPL_RELEASE_INHERITED(CanonicalBrowsingContext, BrowsingContext) diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index 58d74b9f85aa..238dffa8eeb3 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -449,6 +449,8 @@ class CanonicalBrowsingContext final : public BrowsingContext { RefPtr mSecureBrowserUI; RefPtr mWebProgress; + + nsCOMPtr mDocShellProgressBridge; RefPtr mStatusFilter; RefPtr mContainerFeaturePolicy; diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 07f578c16517..18f11925c1e7 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -3766,10 +3766,7 @@ nsresult BrowserChild::PrepareProgressListenerData( } aWebProgressData.browsingContext() = docShell->GetBrowsingContext(); - nsresult rv = - aWebProgress->GetIsLoadingDocument(&aWebProgressData.isLoadingDocument()); - NS_ENSURE_SUCCESS(rv, rv); - rv = aWebProgress->GetLoadType(&aWebProgressData.loadType()); + nsresult rv = aWebProgress->GetLoadType(&aWebProgressData.loadType()); NS_ENSURE_SUCCESS(rv, rv); return PrepareRequestData(aRequest, aRequestData); diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index e18940a56311..52aed5e91b2b 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -29,7 +29,6 @@ #include "mozilla/dom/PointerEventHandler.h" #include "mozilla/dom/BrowserBridgeParent.h" #include "mozilla/dom/RemoteDragStartData.h" -#include "mozilla/dom/RemoteWebProgress.h" #include "mozilla/dom/RemoteWebProgressRequest.h" #include "mozilla/dom/SessionHistoryEntry.h" #include "mozilla/dom/SessionStoreUtils.h" @@ -2683,16 +2682,19 @@ mozilla::ipc::IPCResult BrowserParent::RecvOnStateChange( const WebProgressData& aWebProgressData, const RequestData& aRequestData, const uint32_t aStateFlags, const nsresult aStatus, const Maybe& aStateChangeData) { - nsCOMPtr webProgress; - nsCOMPtr request; - RefPtr browsingContext; - - if (!ReconstructWebProgressAndRequest( - aWebProgressData, aRequestData, getter_AddRefs(webProgress), - getter_AddRefs(request), getter_AddRefs(browsingContext))) { + RefPtr browsingContext = + BrowsingContextForWebProgress(aWebProgressData); + if (!browsingContext) { return IPC_OK(); } + nsCOMPtr request; + if (aRequestData.requestURI()) { + request = MakeAndAddRef( + aRequestData.requestURI(), aRequestData.originalRequestURI(), + aRequestData.matchedList()); + } + if (aStateChangeData.isSome()) { if (!browsingContext->IsTopContent()) { return IPC_FAIL( @@ -2712,9 +2714,8 @@ mozilla::ipc::IPCResult BrowserParent::RecvOnStateChange( } } - if (nsCOMPtr listener = - GetBrowsingContext()->Top()->GetWebProgress()) { - listener->OnStateChange(webProgress, request, aStateFlags, aStatus); + if (auto* listener = browsingContext->GetWebProgress()) { + listener->OnStateChange(listener, request, aStateFlags, aStatus); } return IPC_OK(); @@ -2742,16 +2743,19 @@ mozilla::ipc::IPCResult BrowserParent::RecvOnLocationChange( nsIURI* aLocation, const uint32_t aFlags, const bool aCanGoBack, const bool aCanGoForward, const Maybe& aLocationChangeData) { - nsCOMPtr webProgress; - nsCOMPtr request; - RefPtr browsingContext; - - if (!ReconstructWebProgressAndRequest( - aWebProgressData, aRequestData, getter_AddRefs(webProgress), - getter_AddRefs(request), getter_AddRefs(browsingContext))) { + RefPtr browsingContext = + BrowsingContextForWebProgress(aWebProgressData); + if (!browsingContext) { return IPC_OK(); } + nsCOMPtr request; + if (aRequestData.requestURI()) { + request = MakeAndAddRef( + aRequestData.requestURI(), aRequestData.originalRequestURI(), + aRequestData.matchedList()); + } + browsingContext->SetCurrentRemoteURI(aLocation); nsCOMPtr browser = GetBrowser(); @@ -2784,9 +2788,8 @@ mozilla::ipc::IPCResult BrowserParent::RecvOnLocationChange( } } - if (nsCOMPtr listener = - browsingContext->Top()->GetWebProgress()) { - listener->OnLocationChange(webProgress, request, aLocation, aFlags); + if (auto* listener = browsingContext->GetWebProgress()) { + listener->OnLocationChange(listener, request, aLocation, aFlags); } // Since we've now changed Documents, notify the BrowsingContext that we've @@ -2889,20 +2892,13 @@ already_AddRefed BrowserParent::GetBrowser() { return browser.forget(); } -bool BrowserParent::ReconstructWebProgressAndRequest( - const WebProgressData& aWebProgressData, const RequestData& aRequestData, - nsIWebProgress** aOutWebProgress, nsIRequest** aOutRequest, - CanonicalBrowsingContext** aOutBrowsingContext) { - MOZ_DIAGNOSTIC_ASSERT(aOutWebProgress, - "aOutWebProgress should never be null"); - MOZ_DIAGNOSTIC_ASSERT(aOutRequest, "aOutRequest should never be null"); - MOZ_DIAGNOSTIC_ASSERT(aOutBrowsingContext, - "aOutBrowsingContext should never be null"); - +already_AddRefed +BrowserParent::BrowsingContextForWebProgress( + const WebProgressData& aWebProgressData) { // Look up the BrowsingContext which this notification was fired for. if (aWebProgressData.browsingContext().IsNullOrDiscarded()) { NS_WARNING("WebProgress Ignored: BrowsingContext is null or discarded"); - return false; + return nullptr; } RefPtr browsingContext = aWebProgressData.browsingContext().get_canonical(); @@ -2915,7 +2911,7 @@ bool BrowserParent::ReconstructWebProgressAndRequest( WindowGlobalParent* embedder = browsingContext->GetParentWindowContext(); if (!embedder || embedder->GetBrowserParent() != this) { NS_WARNING("WebProgress Ignored: wrong embedder process"); - return false; + return nullptr; } } @@ -2926,26 +2922,15 @@ bool BrowserParent::ReconstructWebProgressAndRequest( browsingContext->GetCurrentWindowGlobal(); current && current->GetBrowserParent() != this) { NS_WARNING("WebProgress Ignored: no longer current window global"); - return false; + return nullptr; } - // Construct a temporary RemoteWebProgress and RemoteWebProgressRequest which - // contains relevant state used by our in-parent callbacks. - nsCOMPtr webProgress = MakeAndAddRef( - aWebProgressData.loadType(), aWebProgressData.isLoadingDocument(), - browsingContext->IsTopContent()); - - nsCOMPtr request; - if (aRequestData.requestURI()) { - request = MakeAndAddRef( - aRequestData.requestURI(), aRequestData.originalRequestURI(), - aRequestData.matchedList()); + if (RefPtr progress = + browsingContext->GetWebProgress()) { + progress->SetLoadType(aWebProgressData.loadType()); } - webProgress.forget(aOutWebProgress); - request.forget(aOutRequest); - browsingContext.forget(aOutBrowsingContext); - return true; + return browsingContext.forget(); } mozilla::ipc::IPCResult BrowserParent::RecvSessionStoreUpdate( diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h index 13dfce6bf5e7..abf9f87acba7 100644 --- a/dom/ipc/BrowserParent.h +++ b/dom/ipc/BrowserParent.h @@ -307,10 +307,8 @@ class BrowserParent final : public PBrowserParent, already_AddRefed GetBrowser(); - bool ReconstructWebProgressAndRequest( - const WebProgressData& aWebProgressData, const RequestData& aRequestData, - nsIWebProgress** aOutWebProgress, nsIRequest** aOutRequest, - CanonicalBrowsingContext** aOutBrowsingContext); + already_AddRefed BrowsingContextForWebProgress( + const WebProgressData& aWebProgressData); mozilla::ipc::IPCResult RecvSessionStoreUpdate( const Maybe& aDocShellCaps, const Maybe& aPrivatedMode, diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index b2e911d6d94a..1f3f38177b1e 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -116,7 +116,6 @@ namespace dom { struct WebProgressData { MaybeDiscardedBrowsingContext browsingContext; - bool isLoadingDocument; uint32_t loadType; }; diff --git a/dom/ipc/RemoteWebProgress.cpp b/dom/ipc/RemoteWebProgress.cpp deleted file mode 100644 index 0bba9a3aa975..000000000000 --- a/dom/ipc/RemoteWebProgress.cpp +++ /dev/null @@ -1,58 +0,0 @@ -/* 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/. */ - -#include "RemoteWebProgress.h" - -namespace mozilla::dom { - -NS_IMPL_ADDREF(RemoteWebProgress) -NS_IMPL_RELEASE(RemoteWebProgress) - -NS_INTERFACE_MAP_BEGIN(RemoteWebProgress) - NS_INTERFACE_MAP_ENTRY(nsISupports) - NS_INTERFACE_MAP_ENTRY(nsIWebProgress) -NS_INTERFACE_MAP_END - -NS_IMETHODIMP RemoteWebProgress::AddProgressListener( - nsIWebProgressListener* aListener, uint32_t aNotifyMask) { - return NS_ERROR_NOT_IMPLEMENTED; -} - -NS_IMETHODIMP RemoteWebProgress::RemoveProgressListener( - nsIWebProgressListener* aListener) { - return NS_ERROR_NOT_IMPLEMENTED; -} - -NS_IMETHODIMP RemoteWebProgress::GetDOMWindow(mozIDOMWindowProxy** aDOMWindow) { - return NS_ERROR_NOT_AVAILABLE; -} - -NS_IMETHODIMP RemoteWebProgress::GetIsTopLevel(bool* aIsTopLevel) { - NS_ENSURE_ARG_POINTER(aIsTopLevel); - *aIsTopLevel = mIsTopLevel; - return NS_OK; -} - -NS_IMETHODIMP RemoteWebProgress::GetIsLoadingDocument( - bool* aIsLoadingDocument) { - NS_ENSURE_ARG_POINTER(aIsLoadingDocument); - *aIsLoadingDocument = mIsLoadingDocument; - return NS_OK; -} - -NS_IMETHODIMP RemoteWebProgress::GetLoadType(uint32_t* aLoadType) { - NS_ENSURE_ARG_POINTER(aLoadType); - *aLoadType = mLoadType; - return NS_OK; -} - -NS_IMETHODIMP RemoteWebProgress::GetTarget(nsIEventTarget** aTarget) { - return NS_ERROR_NOT_IMPLEMENTED; -} - -NS_IMETHODIMP RemoteWebProgress::SetTarget(nsIEventTarget* aTarget) { - return NS_ERROR_NOT_IMPLEMENTED; -} - -} // namespace mozilla::dom diff --git a/dom/ipc/RemoteWebProgress.h b/dom/ipc/RemoteWebProgress.h deleted file mode 100644 index 118b41438d71..000000000000 --- a/dom/ipc/RemoteWebProgress.h +++ /dev/null @@ -1,39 +0,0 @@ -/* 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_dom_RemoteWebProgress_h -#define mozilla_dom_RemoteWebProgress_h - -#include "nsIWebProgress.h" -#include "nsCOMPtr.h" - -namespace mozilla { -namespace dom { - -class RemoteWebProgress final : public nsIWebProgress { - public: - NS_DECL_ISUPPORTS - NS_DECL_NSIWEBPROGRESS - - RemoteWebProgress() - : mLoadType(0), mIsLoadingDocument(false), mIsTopLevel(false) {} - - RemoteWebProgress(uint32_t aLoadType, bool aIsLoadingDocument, - bool aIsTopLevel) - : mLoadType(aLoadType), - mIsLoadingDocument(aIsLoadingDocument), - mIsTopLevel(aIsTopLevel) {} - - private: - virtual ~RemoteWebProgress() = default; - - uint32_t mLoadType; - bool mIsLoadingDocument; - bool mIsTopLevel; -}; - -} // namespace dom -} // namespace mozilla - -#endif // mozilla_dom_RemoteWebProgress_h diff --git a/dom/ipc/WindowGlobalParent.cpp b/dom/ipc/WindowGlobalParent.cpp index 91d3f6398646..dd3805979916 100644 --- a/dom/ipc/WindowGlobalParent.cpp +++ b/dom/ipc/WindowGlobalParent.cpp @@ -22,7 +22,6 @@ #include "mozilla/dom/BrowserHost.h" #include "mozilla/dom/BrowserParent.h" #include "mozilla/dom/MediaController.h" -#include "mozilla/dom/RemoteWebProgress.h" #include "mozilla/dom/WindowGlobalChild.h" #include "mozilla/dom/ChromeUtils.h" #include "mozilla/dom/ipc/IdType.h" @@ -525,14 +524,9 @@ void WindowGlobalParent::NotifyContentBlockingEvent( // Notify the OnContentBlockingEvent if necessary. if (event) { - if (!GetBrowsingContext()->GetWebProgress()) { - return; + if (auto* webProgress = GetBrowsingContext()->GetWebProgress()) { + webProgress->OnContentBlockingEvent(webProgress, aRequest, event.value()); } - - nsCOMPtr webProgress = - new RemoteWebProgress(0, false, BrowsingContext()->IsTopContent()); - GetBrowsingContext()->Top()->GetWebProgress()->OnContentBlockingEvent( - webProgress, aRequest, event.value()); } } diff --git a/dom/ipc/moz.build b/dom/ipc/moz.build index 83ba06240706..0afa6d309170 100644 --- a/dom/ipc/moz.build +++ b/dom/ipc/moz.build @@ -62,7 +62,6 @@ EXPORTS.mozilla.dom += [ "RefMessageBodyService.h", "RemoteBrowser.h", "RemoteType.h", - "RemoteWebProgress.h", "RemoteWebProgressRequest.h", "SharedMessageBody.h", "TabContext.h", @@ -114,7 +113,6 @@ UNIFIED_SOURCES += [ "ReferrerInfoUtils.cpp", "RefMessageBodyService.cpp", "RemoteBrowser.cpp", - "RemoteWebProgress.cpp", "RemoteWebProgressRequest.cpp", "SharedMap.cpp", "SharedMessageBody.cpp", diff --git a/netwerk/ipc/DocumentLoadListener.cpp b/netwerk/ipc/DocumentLoadListener.cpp index b91f33d5ca4c..27cc9939dc5f 100644 --- a/netwerk/ipc/DocumentLoadListener.cpp +++ b/netwerk/ipc/DocumentLoadListener.cpp @@ -53,7 +53,6 @@ #include "mozilla/dom/BrowserParent.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/nsHTTPSOnlyUtils.h" -#include "mozilla/dom/RemoteWebProgress.h" #include "mozilla/dom/RemoteWebProgressRequest.h" #include "mozilla/net/UrlClassifierFeatureFactory.h" #include "mozilla/ExtensionPolicyService.h" @@ -179,13 +178,6 @@ static auto CreateObjectLoadInfo(nsDocShellLoadState* aLoadState, return loadInfo.forget(); } -static auto WebProgressForBrowsingContext( - CanonicalBrowsingContext* aBrowsingContext) - -> already_AddRefed { - return RefPtr(aBrowsingContext->GetWebProgress()) - .forget(); -} - /** * An extension to nsDocumentOpenInfo that we run in the parent process, so * that we can make the decision to retarget to content handlers or the external @@ -866,17 +858,15 @@ auto DocumentLoadListener::OpenInParent(nsDocShellLoadState* aLoadState, void DocumentLoadListener::FireStateChange(uint32_t aStateFlags, nsresult aStatus) { nsCOMPtr request = GetChannel(); - nsCOMPtr webProgress = - new RemoteWebProgress(GetLoadType(), true, true); - RefPtr loadingWebProgress = - WebProgressForBrowsingContext(GetLoadingBrowsingContext()); + RefPtr webProgress = + GetLoadingBrowsingContext()->GetWebProgress(); - if (loadingWebProgress) { + if (webProgress) { NS_DispatchToMainThread( NS_NewRunnableFunction("DocumentLoadListener::FireStateChange", [=]() { - loadingWebProgress->OnStateChange(webProgress, request, aStateFlags, - aStatus); + webProgress->OnStateChange(webProgress, request, aStateFlags, + aStatus); })); } } @@ -2625,18 +2615,16 @@ NS_IMETHODIMP DocumentLoadListener::OnStatus(nsIRequest* aRequest, nsresult aStatus, const char16_t* aStatusArg) { nsCOMPtr channel = mChannel; - nsCOMPtr webProgress = - new RemoteWebProgress(mLoadStateLoadType, true, true); - RefPtr topWebProgress = - WebProgressForBrowsingContext(GetTopBrowsingContext()); + RefPtr webProgress = + GetLoadingBrowsingContext()->GetWebProgress(); const nsString message(aStatusArg); - if (topWebProgress) { + if (webProgress) { NS_DispatchToMainThread( NS_NewRunnableFunction("DocumentLoadListener::OnStatus", [=]() { - topWebProgress->OnStatusChange(webProgress, channel, aStatus, - message.get()); + webProgress->OnStatusChange(webProgress, channel, aStatus, + message.get()); })); } return NS_OK;