From 34024af634dd86dea7a4f839685c00341304bd55 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Wed, 22 Apr 2020 16:25:18 +0000 Subject: [PATCH] Bug 1580565 - Part 6: Add a unique ID to each BrowsingContextGroup, r=kmag This allows us to explicitly specify BrowsingContextGroups when synchronizing them. A major advantage of this is that it means we can handle an attempt to create a BrowsingContext with a parent which the content process is unaware of, which is possible due to changes to the EnsureSubscribed logic in earlier patches in this stack. This is OK, because in the case where the content process cannot see its parent, the parent must be imminently discarding. Differential Revision: https://phabricator.services.mozilla.com/D71668 --- docshell/base/BrowsingContext.cpp | 42 +++--- docshell/base/BrowsingContext.h | 14 +- docshell/base/BrowsingContextGroup.cpp | 67 ++++++---- docshell/base/BrowsingContextGroup.h | 15 ++- dom/ipc/ContentChild.cpp | 66 +++++----- dom/ipc/ContentChild.h | 17 +-- dom/ipc/ContentParent.cpp | 120 +++++++----------- dom/ipc/ContentParent.h | 12 +- dom/ipc/PContent.ipdl | 37 ++---- .../perfmonitoring/PerformanceUtils.cpp | 7 +- xpcom/tests/gtest/TestThreadMetrics.cpp | 3 +- 11 files changed, 184 insertions(+), 216 deletions(-) diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 44b17fb2a15b..159373374acc 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -297,14 +297,14 @@ void BrowsingContext::EnsureAttached() { Register(this); // Attach the browsing context to the tree. - Attach(); + Attach(/* aFromIPC */ false, /* aOriginProcess */ nullptr); } } /* static */ -already_AddRefed BrowsingContext::CreateFromIPC( - BrowsingContext::IPCInitializer&& aInit, BrowsingContextGroup* aGroup, - ContentParent* aOriginProcess) { +void BrowsingContext::CreateFromIPC(BrowsingContext::IPCInitializer&& aInit, + BrowsingContextGroup* aGroup, + ContentParent* aOriginProcess) { MOZ_DIAGNOSTIC_ASSERT(aOriginProcess || XRE_IsContentProcess()); MOZ_DIAGNOSTIC_ASSERT(aGroup); @@ -345,9 +345,7 @@ already_AddRefed BrowsingContext::CreateFromIPC( Register(context); - // Caller handles attaching us to the tree. - - return context.forget(); + context->Attach(/* aFromIPC */ true, aOriginProcess); } BrowsingContext::BrowsingContext(WindowContext* aParentWindow, @@ -460,7 +458,7 @@ void BrowsingContext::Embed() { } } -void BrowsingContext::Attach(bool aFromIPC) { +void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) { MOZ_DIAGNOSTIC_ASSERT(!mEverAttached); mEverAttached = true; @@ -489,17 +487,19 @@ void BrowsingContext::Attach(bool aFromIPC) { PopupBlocker::RegisterOpenPopupSpam(); } - if (!aFromIPC) { + if (XRE_IsContentProcess() && !aFromIPC) { // Send attach to our parent if we need to. - if (XRE_IsContentProcess()) { - ContentChild::GetSingleton()->SendAttachBrowsingContext( - GetIPCInitializer()); - } else if (IsContent()) { - MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess()); - mGroup->EachParent([&](ContentParent* aParent) { - Unused << aParent->SendAttachBrowsingContext(GetIPCInitializer()); - }); - } + ContentChild::GetSingleton()->SendCreateBrowsingContext( + mGroup->Id(), GetIPCInitializer()); + } else if (XRE_IsParentProcess()) { + mGroup->EachOtherParent(aOriginProcess, [&](ContentParent* aParent) { + MOZ_DIAGNOSTIC_ASSERT(IsContent(), + "chrome BCG cannot be synced to content process"); + if (!Canonical()->IsEmbeddedInProcess(aParent->ChildID())) { + Unused << aParent->SendCreateBrowsingContext(mGroup->Id(), + GetIPCInitializer()); + } + }); } } @@ -539,12 +539,12 @@ void BrowsingContext::Detach(bool aFromIPC) { // destroyed. if (!Canonical()->IsEmbeddedInProcess(aParent->ChildID()) && !Canonical()->IsOwnedByProcess(aParent->ChildID())) { - aParent->SendDetachBrowsingContext(Id(), callback, callback); + aParent->SendDiscardBrowsingContext(this, callback, callback); } }); } else if (!aFromIPC) { - ContentChild::GetSingleton()->SendDetachBrowsingContext(Id(), callback, - callback); + ContentChild::GetSingleton()->SendDiscardBrowsingContext(this, callback, + callback); } } diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h index eb13c2edad41..1a7d54a47071 100644 --- a/docshell/base/BrowsingContext.h +++ b/docshell/base/BrowsingContext.h @@ -238,12 +238,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { return mDocShell ? mDocShell->GetWindow() : nullptr; } - // Attach the current BrowsingContext to its parent, in both the child and the - // parent process. BrowsingContext objects are created attached by default, so - // this method need only be called when restoring cached BrowsingContext - // objects. - void Attach(bool aFromIPC = false); - // Detach the current BrowsingContext from its parent, in both the // child and the parent process. void Detach(bool aFromIPC = false); @@ -540,9 +534,9 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { IPCInitializer GetIPCInitializer(); // Create a BrowsingContext object from over IPC. - static already_AddRefed CreateFromIPC( - IPCInitializer&& aInitializer, BrowsingContextGroup* aGroup, - ContentParent* aOriginProcess); + static void CreateFromIPC(IPCInitializer&& aInitializer, + BrowsingContextGroup* aGroup, + ContentParent* aOriginProcess); // Performs access control to check that 'this' can access 'aTarget'. bool CanAccess(BrowsingContext* aTarget, bool aConsiderOpener = true); @@ -569,6 +563,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { FieldTuple&& aFields); private: + void Attach(bool aFromIPC, ContentParent* aOriginProcess); + // Find the special browsing context if aName is '_self', '_parent', // '_top', but not '_blank'. The latter is handled in FindWithName BrowsingContext* FindWithSpecialName(const nsAString& aName, diff --git a/docshell/base/BrowsingContextGroup.cpp b/docshell/base/BrowsingContextGroup.cpp index 709d9396c51b..61e0257d679e 100644 --- a/docshell/base/BrowsingContextGroup.cpp +++ b/docshell/base/BrowsingContextGroup.cpp @@ -16,13 +16,31 @@ namespace mozilla { namespace dom { -BrowsingContextGroup::BrowsingContextGroup() { - if (XRE_IsContentProcess()) { - ContentChild::GetSingleton()->HoldBrowsingContextGroup(this); - } else { - ContentParent::HoldBrowsingContextGroup(this); +static StaticRefPtr sChromeGroup; + +static StaticAutoPtr< + nsDataHashtable>> + sBrowsingContextGroups; + +already_AddRefed BrowsingContextGroup::GetOrCreate( + uint64_t aId) { + if (!sBrowsingContextGroups) { + sBrowsingContextGroups = + new nsDataHashtable>(); + ClearOnShutdown(&sBrowsingContextGroups); } + auto entry = sBrowsingContextGroups->LookupForAdd(aId); + RefPtr group = + entry.OrInsert([&] { return do_AddRef(new BrowsingContextGroup(aId)); }); + return group.forget(); +} + +already_AddRefed BrowsingContextGroup::Create() { + return GetOrCreate(nsContentUtils::GenerateBrowsingContextId()); +} + +BrowsingContextGroup::BrowsingContextGroup(uint64_t aId) : mId(aId) { mTimerEventQueue = ThrottledEventQueue::Create( GetMainThreadSerialEventTarget(), "BrowsingContextGroup timer queue"); @@ -36,6 +54,10 @@ bool BrowsingContextGroup::Contains(BrowsingContext* aBrowsingContext) { void BrowsingContextGroup::Register(BrowsingContext* aBrowsingContext) { MOZ_DIAGNOSTIC_ASSERT(aBrowsingContext); + MOZ_DIAGNOSTIC_ASSERT(this == sChromeGroup ? aBrowsingContext->IsChrome() + : aBrowsingContext->IsContent(), + "Only chrome BCs may exist in the chrome group, and " + "only content BCs may exist in other groups"); mContexts.PutEntry(aBrowsingContext); } @@ -48,11 +70,6 @@ void BrowsingContextGroup::Unregister(BrowsingContext* aBrowsingContext) { // all subscribers. UnsubscribeAllContentParents(); - if (XRE_IsContentProcess()) { - ContentChild::GetSingleton()->ReleaseBrowsingContextGroup(this); - } else { - ContentParent::ReleaseBrowsingContextGroup(this); - } // We may have been deleted here as the ContentChild/Parent may // have held the last references to `this`. // Do not access any members at this point. @@ -115,7 +132,7 @@ void BrowsingContextGroup::EnsureSubscribed(ContentParent* aProcess) { CollectContextInitializers(mToplevels, inits); // Send all of our contexts to the target content process. - Unused << aProcess->SendRegisterBrowsingContextGroup(inits); + Unused << aProcess->SendRegisterBrowsingContextGroup(Id(), inits); // If the focused or active BrowsingContexts belong in this group, tell the // newly subscribed process. @@ -140,6 +157,10 @@ BrowsingContextGroup::~BrowsingContextGroup() { } void BrowsingContextGroup::UnsubscribeAllContentParents() { + if (sBrowsingContextGroups) { + sBrowsingContextGroups->Remove(Id()); + } + for (auto iter = mSubscribers.Iter(); !iter.Done(); iter.Next()) { nsRefPtrHashKey* entry = iter.Get(); entry->GetKey()->OnBrowsingContextGroupUnsubscribe(this); @@ -196,13 +217,11 @@ void BrowsingContextGroup::FlushPostMessageEvents() { } } -static StaticRefPtr sChromeGroup; - /* static */ BrowsingContextGroup* BrowsingContextGroup::GetChromeGroup() { MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess()); if (!sChromeGroup && XRE_IsParentProcess()) { - sChromeGroup = new BrowsingContextGroup(); + sChromeGroup = BrowsingContextGroup::Create(); ClearOnShutdown(&sChromeGroup); } @@ -251,18 +270,20 @@ already_AddRefed BrowsingContextGroup::Select( if (aOpener) { return do_AddRef(aOpener->Group()); } - return MakeAndAddRef(); + return Create(); } -already_AddRefed BrowsingContextGroup::Select( - uint64_t aParentId, uint64_t aOpenerId) { - RefPtr parent = WindowContext::GetById(aParentId); - MOZ_RELEASE_ASSERT(parent || aParentId == 0); +void BrowsingContextGroup::GetAllGroups( + nsTArray>& aGroups) { + aGroups.Clear(); + if (!sBrowsingContextGroups) { + return; + } - RefPtr opener = BrowsingContext::Get(aOpenerId); - MOZ_RELEASE_ASSERT(opener || aOpenerId == 0); - - return Select(parent, opener); + aGroups.SetCapacity(sBrowsingContextGroups->Count()); + for (auto& group : *sBrowsingContextGroups) { + aGroups.AppendElement(group.GetData()); + } } NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(BrowsingContextGroup, mContexts, diff --git a/docshell/base/BrowsingContextGroup.h b/docshell/base/BrowsingContextGroup.h index aafb23cea4a7..efd4c0a9d1cb 100644 --- a/docshell/base/BrowsingContextGroup.h +++ b/docshell/base/BrowsingContextGroup.h @@ -55,18 +55,18 @@ class BrowsingContextGroup final : public nsWrapperCache { aToplevels.AppendElements(mToplevels); } + uint64_t Id() { return mId; } + nsISupports* GetParentObject() const; JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override; - BrowsingContextGroup(); - + // Get or create a BrowsingContextGroup with the given ID. + static already_AddRefed GetOrCreate(uint64_t aId); + static already_AddRefed Create(); static already_AddRefed Select( WindowContext* aParent, BrowsingContext* aOpener); - static already_AddRefed Select(uint64_t aParentId, - uint64_t aOpenerId); - // For each 'ContentParent', except for 'aExcludedParent', // associated with this group call 'aCallback'. template @@ -114,13 +114,18 @@ class BrowsingContextGroup final : public nsWrapperCache { return mWorkerEventQueue; } + static void GetAllGroups(nsTArray>& aGroups); + private: friend class CanonicalBrowsingContext; + explicit BrowsingContextGroup(uint64_t aId); ~BrowsingContextGroup(); void UnsubscribeAllContentParents(); + uint64_t mId; + // A BrowsingContextGroup contains a series of BrowsingContext objects. They // are addressed using a hashtable to avoid linear lookup when adding or // removing elements from the set. diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 229c899d47a1..4fdbf2878ab8 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -2201,8 +2201,6 @@ void ContentChild::ActorDestroy(ActorDestroyReason why) { mIdleObservers.Clear(); - mBrowsingContextGroupHolder.Clear(); - nsCOMPtr svc(do_GetService(NS_CONSOLESERVICE_CONTRACTID)); if (svc) { svc->UnregisterListener(mConsoleListener); @@ -3614,42 +3612,46 @@ PContentChild::Result ContentChild::OnMessageReceived(const Message& aMsg, return result; } -mozilla::ipc::IPCResult ContentChild::RecvAttachBrowsingContext( - BrowsingContext::IPCInitializer&& aInit) { - RefPtr child = BrowsingContext::Get(aInit.mId); - MOZ_RELEASE_ASSERT(!child || child->IsCached()); - - if (!child) { - // Determine the BrowsingContextGroup from our parent or opener fields. - RefPtr group = - BrowsingContextGroup::Select(aInit.mParentId, aInit.GetOpenerId()); - child = BrowsingContext::CreateFromIPC(std::move(aInit), group, nullptr); +mozilla::ipc::IPCResult ContentChild::RecvCreateBrowsingContext( + uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit) { + // We can't already have a BrowsingContext with this ID. + if (RefPtr existing = BrowsingContext::Get(aInit.mId)) { + return IPC_FAIL(this, "Browsing context already exists"); } - child->Attach(/* aFromIPC */ true); + RefPtr parent = WindowContext::GetById(aInit.mParentId); + if (!parent && aInit.mParentId != 0) { + // Handle this case by ignoring the request, as parent must be in the + // process of being discarded. + // In the future it would be nice to avoid sending this message to the child + // at all. + NS_WARNING("Attempt to attach BrowsingContext to discarded parent"); + return IPC_OK(); + } + RefPtr group = + BrowsingContextGroup::GetOrCreate(aGroupId); + BrowsingContext::CreateFromIPC(std::move(aInit), group, nullptr); return IPC_OK(); } -mozilla::ipc::IPCResult ContentChild::RecvDetachBrowsingContext( - uint64_t aContextId, DetachBrowsingContextResolver&& aResolve) { - // NOTE: Immediately resolve the promise, as we've received the message. This - // will allow the parent process to discard references to this BC. - aResolve(true); - - // If we can't find a BrowsingContext with the given ID, it's already been - // collected and we can ignore the request. - RefPtr context = BrowsingContext::Get(aContextId); - if (context) { - context->Detach(/* aFromIPC */ true); +mozilla::ipc::IPCResult ContentChild::RecvDiscardBrowsingContext( + const MaybeDiscarded& aContext, + DiscardBrowsingContextResolver&& aResolve) { + if (!aContext.IsNullOrDiscarded()) { + aContext.get()->Detach(/* aFromIPC */ true); } + // Immediately resolve the promise, as we've received the message. This will + // allow the parent process to discard references to this BC. + aResolve(true); return IPC_OK(); } mozilla::ipc::IPCResult ContentChild::RecvRegisterBrowsingContextGroup( - nsTArray&& aInits) { - RefPtr group = new BrowsingContextGroup(); + uint64_t aGroupId, nsTArray&& aInits) { + RefPtr group = + BrowsingContextGroup::GetOrCreate(aGroupId); // Each of the initializers in aInits is sorted in pre-order, so our parent // should always be available before the element itself. @@ -3665,9 +3667,7 @@ mozilla::ipc::IPCResult ContentChild::RecvRegisterBrowsingContextGroup( MOZ_ASSERT_IF(parent, parent->Group() == group); #endif - RefPtr ctxt = - BrowsingContext::CreateFromIPC(std::move(init), group, nullptr); - ctxt->Attach(/* aFromIPC */ true); + BrowsingContext::CreateFromIPC(std::move(init), group, nullptr); break; } case SyncedContextInitializer::TWindowContextInitializer: { @@ -3999,14 +3999,6 @@ mozilla::ipc::IPCResult ContentChild::RecvDiscardWindowContext( return IPC_OK(); } -void ContentChild::HoldBrowsingContextGroup(BrowsingContextGroup* aBCG) { - mBrowsingContextGroupHolder.AppendElement(aBCG); -} - -void ContentChild::ReleaseBrowsingContextGroup(BrowsingContextGroup* aBCG) { - mBrowsingContextGroupHolder.RemoveElement(aBCG); -} - mozilla::ipc::IPCResult ContentChild::RecvScriptError( const nsString& aMessage, const nsString& aSourceName, const nsString& aSourceLine, const uint32_t& aLineNumber, diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 1b443559034a..a12dbab0e479 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -691,10 +691,6 @@ class ContentChild final PFileDescriptorSetChild* SendPFileDescriptorSetConstructor( const FileDescriptor& aFD) override; - const nsTArray>& BrowsingContextGroups() const { - return mBrowsingContextGroupHolder; - } - private: static void ForceKillTimerCallback(nsITimer* aTimer, void* aClosure); void StartForceKillTimer(); @@ -710,14 +706,15 @@ class ContentChild final virtual void OnChannelReceivedMessage(const Message& aMsg) override; - mozilla::ipc::IPCResult RecvAttachBrowsingContext( - BrowsingContext::IPCInitializer&& aInit); + mozilla::ipc::IPCResult RecvCreateBrowsingContext( + uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit); - mozilla::ipc::IPCResult RecvDetachBrowsingContext( - uint64_t aContextId, DetachBrowsingContextResolver&& aResolve); + mozilla::ipc::IPCResult RecvDiscardBrowsingContext( + const MaybeDiscarded& aContext, + DiscardBrowsingContextResolver&& aResolve); mozilla::ipc::IPCResult RecvRegisterBrowsingContextGroup( - nsTArray&& aInits); + uint64_t aGroupId, nsTArray&& aInits); mozilla::ipc::IPCResult RecvWindowClose( const MaybeDiscarded& aContext, bool aTrustedCaller); @@ -871,8 +868,6 @@ class ContentChild final uint32_t mNetworkLinkType = 0; - nsTArray> mBrowsingContextGroupHolder; - // See `BrowsingContext::mEpochs` for an explanation of this field. uint64_t mBrowsingContextFieldEpoch = 0; diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 97c93d9df413..69c32f14e664 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -586,8 +586,6 @@ StaticAutoPtr> ContentParent::sContentParents; UniquePtr ContentParent::sSandboxBrokerPolicyFactory; #endif -StaticAutoPtr>> - ContentParent::sBrowsingContextGroupHolder; #if defined(XP_MACOSX) && defined(MOZ_SANDBOX) StaticAutoPtr> ContentParent::sMacSandboxParams; #endif @@ -646,9 +644,6 @@ void ContentParent::StartUp() { // child process sCanLaunchSubprocesses = true; - sBrowsingContextGroupHolder = new nsTArray>(); - ClearOnShutdown(&sBrowsingContextGroupHolder); - if (!XRE_IsParentProcess()) { return; } @@ -5900,56 +5895,54 @@ mozilla::ipc::IPCResult ContentParent::RecvSessionStorageData( return IPC_OK(); } -mozilla::ipc::IPCResult ContentParent::RecvAttachBrowsingContext( - BrowsingContext::IPCInitializer&& aInit) { +mozilla::ipc::IPCResult ContentParent::RecvCreateBrowsingContext( + uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit) { RefPtr parent; if (aInit.mParentId != 0) { parent = WindowGlobalParent::GetByInnerWindowId(aInit.mParentId); - MOZ_RELEASE_ASSERT(parent, "Parent doesn't exist in parent process"); + if (!parent) { + return IPC_FAIL(this, "Parent doesn't exist in parent process"); + } } if (parent && parent->GetContentParent() != this) { // We're trying attach a child BrowsingContext to a parent - // BrowsingContext in another process. This is illegal since the - // only thing that could create that child BrowsingContext is a - // parent docshell in the same process as that BrowsingContext. - MOZ_DIAGNOSTIC_ASSERT(false, - "Trying to attach to out of process parent context"); + // WindowContext in another process. This is illegal since the + // only thing that could create that child BrowsingContext is the parent + // window's process. + return IPC_FAIL(this, + "Must create BrowsingContext from the parent's process"); + } - MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning, - ("ParentIPC: Trying to attach to out of process parent context " - "0x%08" PRIx64, - aInit.mParentId)); - return IPC_OK(); + RefPtr opener; + if (aInit.GetOpenerId() != 0) { + opener = BrowsingContext::Get(aInit.GetOpenerId()); + if (!opener) { + return IPC_FAIL(this, "Opener doesn't exist in parent process"); + } } RefPtr child = BrowsingContext::Get(aInit.mId); - if (child && !child->IsCached()) { - // This is highly suspicious. BrowsingContexts should only be - // attached at most once, but finding one indicates that someone - // is doing something they shouldn't. - MOZ_DIAGNOSTIC_ASSERT(false, - "Trying to attach already attached browsing context"); - - MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning, - ("ParentIPC: Trying to attach already attached 0x%08" PRIx64 - " to 0x%08" PRIx64, - aInit.mId, aInit.mParentId)); - return IPC_OK(); + if (child) { + // This is highly suspicious. BrowsingContexts should only be created once, + // so finding one indicates that someone is doing something they shouldn't. + return IPC_FAIL(this, "A BrowsingContext with this ID already exists"); } - if (!child) { - RefPtr group = - BrowsingContextGroup::Select(aInit.mParentId, aInit.GetOpenerId()); - child = BrowsingContext::CreateFromIPC(std::move(aInit), group, this); + // Ensure that the passed-in BrowsingContextGroup is valid. + RefPtr group = + BrowsingContextGroup::GetOrCreate(aGroupId); + if (parent && parent->Group() != group) { + return IPC_FAIL(this, "Parent is not in the given group"); + } + if (opener && opener->Group() != group) { + return IPC_FAIL(this, "Opener is not in the given group"); + } + if (!parent && !opener && !group->Toplevels().IsEmpty()) { + return IPC_FAIL(this, "Unrelated context must be created in a new group"); } - child->Attach(/* aFromIPC */ true); - - child->Group()->EachOtherParent(this, [&](ContentParent* aParent) { - Unused << aParent->SendAttachBrowsingContext(child->GetIPCInitializer()); - }); - + BrowsingContext::CreateFromIPC(std::move(aInit), group, this); return IPC_OK(); } @@ -5979,28 +5972,21 @@ bool ContentParent::CheckBrowsingContextEmbedder(CanonicalBrowsingContext* aBC, return true; } -mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext( - uint64_t aContextId, DetachBrowsingContextResolver&& aResolve) { - // NOTE: Immediately resolve the promise, as we've received the message. This - // will allow the content process to discard references to this BC. +mozilla::ipc::IPCResult ContentParent::RecvDiscardBrowsingContext( + const MaybeDiscarded& aContext, + DiscardBrowsingContextResolver&& aResolve) { + if (!aContext.IsNullOrDiscarded()) { + RefPtr context = aContext.get_canonical(); + if (!CheckBrowsingContextEmbedder(context, "discard")) { + return IPC_FAIL(this, "Illegal Discard attempt"); + } + + context->Detach(/* aFromIPC */ true); + } + + // Resolve the promise, as we've received and handled the message. This will + // allow the content process to fully-discard references to this BC. aResolve(true); - - // NOTE: It's OK if we don't have this context anymore. It was just already - // detached, return. - RefPtr context = - CanonicalBrowsingContext::Get(aContextId); - if (!context || context->IsDiscarded()) { - MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug, - ("ParentIPC: Trying to detach already detached")); - return IPC_OK(); - } - - if (!CheckBrowsingContextEmbedder(context, "detach")) { - return IPC_FAIL(this, "Illegal Detach() attempt"); - } - - context->Detach(/* aFromIPC */ true); - return IPC_OK(); } @@ -6313,18 +6299,6 @@ mozilla::ipc::IPCResult ContentParent::RecvWindowPostMessage( return IPC_OK(); } -/* static */ -void ContentParent::HoldBrowsingContextGroup(BrowsingContextGroup* aBCG) { - sBrowsingContextGroupHolder->AppendElement(aBCG); -} - -/* static */ -void ContentParent::ReleaseBrowsingContextGroup(BrowsingContextGroup* aBCG) { - if (sBrowsingContextGroupHolder) { - sBrowsingContextGroupHolder->RemoveElement(aBCG); - } -} - void ContentParent::OnBrowsingContextGroupSubscribe( BrowsingContextGroup* aGroup) { MOZ_DIAGNOSTIC_ASSERT(aGroup); diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index a46037360526..d6f999371f7a 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -640,11 +640,12 @@ class ContentParent final static bool IsInputEventQueueSupported(); - mozilla::ipc::IPCResult RecvAttachBrowsingContext( - BrowsingContext::IPCInitializer&& aInit); + mozilla::ipc::IPCResult RecvCreateBrowsingContext( + uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit); - mozilla::ipc::IPCResult RecvDetachBrowsingContext( - uint64_t aContextId, DetachBrowsingContextResolver&& aResolve); + mozilla::ipc::IPCResult RecvDiscardBrowsingContext( + const MaybeDiscarded& aContext, + DiscardBrowsingContextResolver&& aResolve); mozilla::ipc::IPCResult RecvWindowClose( const MaybeDiscarded& aContext, bool aTrustedCaller); @@ -1290,9 +1291,6 @@ class ContentParent final const bool& aMinimizeMemoryUsage, const Maybe& aDMDFile) override; - static void HoldBrowsingContextGroup(BrowsingContextGroup* aBCG); - static void ReleaseBrowsingContextGroup(BrowsingContextGroup* aBCG); - void OnBrowsingContextGroupSubscribe(BrowsingContextGroup* aGroup); void OnBrowsingContextGroupUnsubscribe(BrowsingContextGroup* aGroup); diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 5d576d049e5f..02e89d3a2d8b 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -832,7 +832,7 @@ child: // Begin subscribing to a new BrowsingContextGroup, sending down the current // value for every individual BrowsingContext. - async RegisterBrowsingContextGroup(SyncedContextInitializer[] aInits); + async RegisterBrowsingContextGroup(uint64_t aGroupId, SyncedContextInitializer[] aInits); #if defined(MOZ_SANDBOX) && defined(MOZ_DEBUG) && defined(ENABLE_TESTS) // Initialize top-level actor for testing content process sandbox. @@ -1569,32 +1569,23 @@ both: uint32_t flags); /** - * Sync the BrowsingContext with id 'aContextId' and name 'aName' to the - * parent, and attach it to the BrowsingContext 'aParentContext'. If - * 'aParentContext' is 'nullptr' the BrowsingContext is a root in the - * BrowsingContext tree. AttachBrowsingContext must only be called at most - * once for any child BrowsingContext, and only for BrowsingContexts where - * the parent and the child context contains their nsDocShell. + * Creates a new BrowsingContext, initialized with the values provided in + * `BrowsingContextInitializer`. + * + * This message may only be sent to the parent in limited situations. If the + * new BrowsingContext has a parent window, it must be owned by the + * embedding process, otherwise it must be owned by the opener, if set. */ - async AttachBrowsingContext(BrowsingContextInitializer aInit); + async CreateBrowsingContext(uint64_t aGroupId, BrowsingContextInitializer aInit); /** - * Remove the synced BrowsingContext with id 'aContextId' from the parent. - * DetachBrowsingContext is only needed to be called once for any - * BrowsingContext, since detaching a node in the BrowsingContext detaches - * the entire sub-tree rooted at that node. Calling DetachBrowsingContext - * with an already detached BrowsingContext effectively does nothing. Note - * that it is not an error to call DetachBrowsingContext on a - * BrowsingContext belonging to an already detached subtree. - * - * As the passed-in context is allowed to already be detached, it is passed - * by id, rather than using BrowsingContext's serializer. - * - * Callers should keep the BrowsingContext alive until this async request is - * resolved or rejected, in order to ensure that in-flight messages still - * have valid targets. + * Discards the passed-in BrowsingContext. If the BrowsingContext has + * already been discarded, this message does nothing. + * The response promise is fulfilled when the process has flagged the + * BrowsingContext as discarded. */ - async DetachBrowsingContext(uint64_t aContextId) returns (bool unused); + async DiscardBrowsingContext(MaybeDiscardedBrowsingContext aContext) + returns (bool unused); async WindowClose(MaybeDiscardedBrowsingContext aContext, bool aTrustedCaller); diff --git a/toolkit/components/perfmonitoring/PerformanceUtils.cpp b/toolkit/components/perfmonitoring/PerformanceUtils.cpp index 97e2fd3265d5..391f94466d31 100644 --- a/toolkit/components/perfmonitoring/PerformanceUtils.cpp +++ b/toolkit/components/perfmonitoring/PerformanceUtils.cpp @@ -41,12 +41,7 @@ nsTArray> CollectPerformanceInfo() { } nsTArray> groups; - if (XRE_IsContentProcess()) { - groups.AppendElements( - ContentChild::GetSingleton()->BrowsingContextGroups()); - } else { - groups.AppendElements(ContentParent::BrowsingContextGroups()); - } + BrowsingContextGroup::GetAllGroups(groups); nsTArray docGroups; for (auto& browsingContextGroup : groups) { diff --git a/xpcom/tests/gtest/TestThreadMetrics.cpp b/xpcom/tests/gtest/TestThreadMetrics.cpp index 1be6567ef9d4..0b1b12a56317 100644 --- a/xpcom/tests/gtest/TestThreadMetrics.cpp +++ b/xpcom/tests/gtest/TestThreadMetrics.cpp @@ -118,7 +118,8 @@ class ThreadMetrics : public ::testing::Test { protected: virtual void SetUp() { // building the DocGroup structure - RefPtr group = new dom::BrowsingContextGroup(); + RefPtr group = + dom::BrowsingContextGroup::Create(); mDocGroup = group->AddDocument(NS_LITERAL_CSTRING("key"), nullptr); mDocGroup2 = group->AddDocument(NS_LITERAL_CSTRING("key2"), nullptr); mCounter = mDocGroup->GetPerformanceCounter();