From 260415ed7ce8caddd23e9f8510cd769bc8a87980 Mon Sep 17 00:00:00 2001 From: Perry Jiang Date: Fri, 23 Oct 2020 11:51:24 +0000 Subject: [PATCH] Bug 1653470 - refcount PServiceWorkerContainer r=dom-workers-and-storage-reviewers,asuth Depends on D83883 Differential Revision: https://phabricator.services.mozilla.com/D83884 --- .../PServiceWorkerContainer.ipdl | 2 +- .../RemoteServiceWorkerContainerImpl.cpp | 9 +++++--- .../RemoteServiceWorkerContainerImpl.h | 2 +- dom/serviceworkers/ServiceWorkerActors.cpp | 21 ------------------- dom/serviceworkers/ServiceWorkerActors.h | 9 -------- .../ServiceWorkerContainerChild.cpp | 8 +++---- .../ServiceWorkerContainerChild.h | 8 +++++-- .../ServiceWorkerContainerParent.h | 5 ++++- .../ServiceWorkerContainerProxy.h | 2 +- dom/serviceworkers/moz.build | 2 ++ ipc/glue/BackgroundChildImpl.cpp | 10 +++------ ipc/glue/BackgroundChildImpl.h | 7 ++----- ipc/glue/BackgroundParentImpl.cpp | 11 +++------- ipc/glue/BackgroundParentImpl.h | 6 ++---- 14 files changed, 35 insertions(+), 67 deletions(-) diff --git a/dom/serviceworkers/PServiceWorkerContainer.ipdl b/dom/serviceworkers/PServiceWorkerContainer.ipdl index f57846ffb6cb..8cae967abc4f 100644 --- a/dom/serviceworkers/PServiceWorkerContainer.ipdl +++ b/dom/serviceworkers/PServiceWorkerContainer.ipdl @@ -10,7 +10,7 @@ include IPCServiceWorkerRegistrationDescriptor; namespace mozilla { namespace dom { -protocol PServiceWorkerContainer +refcounted protocol PServiceWorkerContainer { manager PBackground; diff --git a/dom/serviceworkers/RemoteServiceWorkerContainerImpl.cpp b/dom/serviceworkers/RemoteServiceWorkerContainerImpl.cpp index 0b156173744e..21a6f012c710 100644 --- a/dom/serviceworkers/RemoteServiceWorkerContainerImpl.cpp +++ b/dom/serviceworkers/RemoteServiceWorkerContainerImpl.cpp @@ -6,6 +6,8 @@ #include "RemoteServiceWorkerContainerImpl.h" +#include + #include "mozilla/ipc/BackgroundChild.h" #include "mozilla/ipc/PBackgroundChild.h" #include "ServiceWorkerContainerChild.h" @@ -195,7 +197,7 @@ void RemoteServiceWorkerContainerImpl::GetReady( } RemoteServiceWorkerContainerImpl::RemoteServiceWorkerContainerImpl() - : mActor(nullptr), mOuter(nullptr), mShutdown(false) { + : mOuter(nullptr), mShutdown(false) { PBackgroundChild* parentActor = BackgroundChild::GetOrCreateForCurrentThread(); if (NS_WARN_IF(!parentActor)) { @@ -203,7 +205,8 @@ RemoteServiceWorkerContainerImpl::RemoteServiceWorkerContainerImpl() return; } - ServiceWorkerContainerChild* actor = ServiceWorkerContainerChild::Create(); + RefPtr actor = + ServiceWorkerContainerChild::Create(); if (NS_WARN_IF(!actor)) { Shutdown(); return; @@ -217,7 +220,7 @@ RemoteServiceWorkerContainerImpl::RemoteServiceWorkerContainerImpl() } MOZ_DIAGNOSTIC_ASSERT(sentActor == actor); - mActor = actor; + mActor = std::move(actor); mActor->SetOwner(this); } diff --git a/dom/serviceworkers/RemoteServiceWorkerContainerImpl.h b/dom/serviceworkers/RemoteServiceWorkerContainerImpl.h index eb95183684f7..fffe7002d1f5 100644 --- a/dom/serviceworkers/RemoteServiceWorkerContainerImpl.h +++ b/dom/serviceworkers/RemoteServiceWorkerContainerImpl.h @@ -16,7 +16,7 @@ class ServiceWorkerContainerChild; class RemoteServiceWorkerContainerImpl final : public ServiceWorkerContainer::Inner { - ServiceWorkerContainerChild* mActor; + RefPtr mActor; ServiceWorkerContainer* mOuter; bool mShutdown; diff --git a/dom/serviceworkers/ServiceWorkerActors.cpp b/dom/serviceworkers/ServiceWorkerActors.cpp index 8cd539028c99..21b192381f86 100644 --- a/dom/serviceworkers/ServiceWorkerActors.cpp +++ b/dom/serviceworkers/ServiceWorkerActors.cpp @@ -22,27 +22,6 @@ void InitServiceWorkerParent(PServiceWorkerParent* aActor, actor->Init(aDescriptor); } -PServiceWorkerContainerChild* AllocServiceWorkerContainerChild() { - MOZ_CRASH("should not be called"); -} - -bool DeallocServiceWorkerContainerChild(PServiceWorkerContainerChild* aActor) { - auto actor = static_cast(aActor); - delete actor; - return true; -} - -PServiceWorkerContainerParent* AllocServiceWorkerContainerParent() { - return new ServiceWorkerContainerParent(); -} - -bool DeallocServiceWorkerContainerParent( - PServiceWorkerContainerParent* aActor) { - auto actor = static_cast(aActor); - delete actor; - return true; -} - void InitServiceWorkerContainerParent(PServiceWorkerContainerParent* aActor) { auto actor = static_cast(aActor); actor->Init(); diff --git a/dom/serviceworkers/ServiceWorkerActors.h b/dom/serviceworkers/ServiceWorkerActors.h index 86b2c6f63263..433cfbf292c5 100644 --- a/dom/serviceworkers/ServiceWorkerActors.h +++ b/dom/serviceworkers/ServiceWorkerActors.h @@ -21,17 +21,8 @@ void InitServiceWorkerParent(PServiceWorkerParent* aActor, // PServiceWorkerContainer -class PServiceWorkerContainerChild; class PServiceWorkerContainerParent; -PServiceWorkerContainerChild* AllocServiceWorkerContainerChild(); - -bool DeallocServiceWorkerContainerChild(PServiceWorkerContainerChild* aActor); - -PServiceWorkerContainerParent* AllocServiceWorkerContainerParent(); - -bool DeallocServiceWorkerContainerParent(PServiceWorkerContainerParent* aActor); - void InitServiceWorkerContainerParent(PServiceWorkerContainerParent* aActor); // PServiceWorkerRegistration diff --git a/dom/serviceworkers/ServiceWorkerContainerChild.cpp b/dom/serviceworkers/ServiceWorkerContainerChild.cpp index c554da28a511..77fc499ab577 100644 --- a/dom/serviceworkers/ServiceWorkerContainerChild.cpp +++ b/dom/serviceworkers/ServiceWorkerContainerChild.cpp @@ -22,8 +22,9 @@ void ServiceWorkerContainerChild::ActorDestroy(ActorDestroyReason aReason) { } // static -ServiceWorkerContainerChild* ServiceWorkerContainerChild::Create() { - ServiceWorkerContainerChild* actor = new ServiceWorkerContainerChild(); +already_AddRefed +ServiceWorkerContainerChild::Create() { + RefPtr actor = new ServiceWorkerContainerChild; if (!NS_IsMainThread()) { WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate(); @@ -36,12 +37,11 @@ ServiceWorkerContainerChild* ServiceWorkerContainerChild::Create() { workerPrivate, "ServiceWorkerContainerChild", [helper] { helper->Actor()->MaybeStartTeardown(); }); if (NS_WARN_IF(!actor->mIPCWorkerRef)) { - delete actor; return nullptr; } } - return actor; + return actor.forget(); } ServiceWorkerContainerChild::ServiceWorkerContainerChild() diff --git a/dom/serviceworkers/ServiceWorkerContainerChild.h b/dom/serviceworkers/ServiceWorkerContainerChild.h index 7788ec1caa56..d6e37fbf514f 100644 --- a/dom/serviceworkers/ServiceWorkerContainerChild.h +++ b/dom/serviceworkers/ServiceWorkerContainerChild.h @@ -12,6 +12,8 @@ namespace mozilla { namespace dom { +class RemoteServiceWorkerContainerImpl; + class IPCWorkerRef; class ServiceWorkerContainerChild final : public PServiceWorkerContainerChild { @@ -21,13 +23,15 @@ class ServiceWorkerContainerChild final : public PServiceWorkerContainerChild { ServiceWorkerContainerChild(); + ~ServiceWorkerContainerChild() = default; + // PServiceWorkerContainerChild void ActorDestroy(ActorDestroyReason aReason) override; public: - static ServiceWorkerContainerChild* Create(); + NS_INLINE_DECL_REFCOUNTING(ServiceWorkerContainerChild, override); - ~ServiceWorkerContainerChild() = default; + static already_AddRefed Create(); void SetOwner(RemoteServiceWorkerContainerImpl* aOwner); diff --git a/dom/serviceworkers/ServiceWorkerContainerParent.h b/dom/serviceworkers/ServiceWorkerContainerParent.h index bb0e556dab12..ccfdced8f649 100644 --- a/dom/serviceworkers/ServiceWorkerContainerParent.h +++ b/dom/serviceworkers/ServiceWorkerContainerParent.h @@ -19,6 +19,8 @@ class ServiceWorkerContainerParent final : public PServiceWorkerContainerParent { RefPtr mProxy; + ~ServiceWorkerContainerParent(); + // PServiceWorkerContainerParent void ActorDestroy(ActorDestroyReason aReason) override; @@ -42,8 +44,9 @@ class ServiceWorkerContainerParent final GetReadyResolver&& aResolver) override; public: + NS_INLINE_DECL_REFCOUNTING(ServiceWorkerContainerParent, override); + ServiceWorkerContainerParent(); - ~ServiceWorkerContainerParent(); void Init(); }; diff --git a/dom/serviceworkers/ServiceWorkerContainerProxy.h b/dom/serviceworkers/ServiceWorkerContainerProxy.h index 202a73a374ff..c04b81cf2b01 100644 --- a/dom/serviceworkers/ServiceWorkerContainerProxy.h +++ b/dom/serviceworkers/ServiceWorkerContainerProxy.h @@ -14,7 +14,7 @@ class ServiceWorkerContainerParent; class ServiceWorkerContainerProxy final { // Background thread only - ServiceWorkerContainerParent* mActor; + RefPtr mActor; ~ServiceWorkerContainerProxy(); diff --git a/dom/serviceworkers/moz.build b/dom/serviceworkers/moz.build index 08aa240ffb4b..224c4ccf2120 100644 --- a/dom/serviceworkers/moz.build +++ b/dom/serviceworkers/moz.build @@ -18,6 +18,8 @@ EXPORTS.mozilla.dom += [ 'ServiceWorkerChild.h', 'ServiceWorkerCloneData.h', 'ServiceWorkerContainer.h', + 'ServiceWorkerContainerChild.h', + 'ServiceWorkerContainerParent.h', 'ServiceWorkerDescriptor.h', 'ServiceWorkerEvents.h', 'ServiceWorkerInfo.h', diff --git a/ipc/glue/BackgroundChildImpl.cpp b/ipc/glue/BackgroundChildImpl.cpp index ef6b7e8e4337..63a17ac70b1c 100644 --- a/ipc/glue/BackgroundChildImpl.cpp +++ b/ipc/glue/BackgroundChildImpl.cpp @@ -43,6 +43,7 @@ #include "mozilla/dom/LocalStorage.h" #include "mozilla/dom/MessagePortChild.h" #include "mozilla/dom/ServiceWorkerActors.h" +#include "mozilla/dom/ServiceWorkerContainerChild.h" #include "mozilla/dom/ServiceWorkerManagerChild.h" #include "mozilla/dom/BrowserChild.h" #include "mozilla/ipc/IPCStreamAlloc.h" @@ -647,14 +648,9 @@ BackgroundChildImpl::AllocPServiceWorkerChild( return {}; } -PServiceWorkerContainerChild* +already_AddRefed BackgroundChildImpl::AllocPServiceWorkerContainerChild() { - return dom::AllocServiceWorkerContainerChild(); -} - -bool BackgroundChildImpl::DeallocPServiceWorkerContainerChild( - PServiceWorkerContainerChild* aActor) { - return dom::DeallocServiceWorkerContainerChild(aActor); + return mozilla::dom::ServiceWorkerContainerChild::Create(); } PServiceWorkerRegistrationChild* diff --git a/ipc/glue/BackgroundChildImpl.h b/ipc/glue/BackgroundChildImpl.h index acee31289dc4..f19884922df0 100644 --- a/ipc/glue/BackgroundChildImpl.h +++ b/ipc/glue/BackgroundChildImpl.h @@ -246,11 +246,8 @@ class BackgroundChildImpl : public PBackgroundChild, already_AddRefed AllocPServiceWorkerChild( const IPCServiceWorkerDescriptor&); - virtual PServiceWorkerContainerChild* AllocPServiceWorkerContainerChild() - override; - - virtual bool DeallocPServiceWorkerContainerChild( - PServiceWorkerContainerChild*) override; + already_AddRefed + AllocPServiceWorkerContainerChild(); virtual PServiceWorkerRegistrationChild* AllocPServiceWorkerRegistrationChild( const IPCServiceWorkerRegistrationDescriptor&) override; diff --git a/ipc/glue/BackgroundParentImpl.cpp b/ipc/glue/BackgroundParentImpl.cpp index eaad4e9c3f90..0cac82a9f56f 100644 --- a/ipc/glue/BackgroundParentImpl.cpp +++ b/ipc/glue/BackgroundParentImpl.cpp @@ -38,6 +38,7 @@ #include "mozilla/dom/RemoteWorkerServiceParent.h" #include "mozilla/dom/ReportingHeader.h" #include "mozilla/dom/ServiceWorkerActors.h" +#include "mozilla/dom/ServiceWorkerContainerParent.h" #include "mozilla/dom/ServiceWorkerManagerParent.h" #include "mozilla/dom/ServiceWorkerParent.h" #include "mozilla/dom/ServiceWorkerRegistrar.h" @@ -1255,7 +1256,6 @@ already_AddRefed BackgroundParentImpl::AllocPServiceWorkerParent( const IPCServiceWorkerDescriptor&) { return MakeAndAddRef(); - ; } IPCResult BackgroundParentImpl::RecvPServiceWorkerConstructor( @@ -1265,14 +1265,9 @@ IPCResult BackgroundParentImpl::RecvPServiceWorkerConstructor( return IPC_OK(); } -PServiceWorkerContainerParent* +already_AddRefed BackgroundParentImpl::AllocPServiceWorkerContainerParent() { - return dom::AllocServiceWorkerContainerParent(); -} - -bool BackgroundParentImpl::DeallocPServiceWorkerContainerParent( - PServiceWorkerContainerParent* aActor) { - return dom::DeallocServiceWorkerContainerParent(aActor); + return MakeAndAddRef(); } mozilla::ipc::IPCResult diff --git a/ipc/glue/BackgroundParentImpl.h b/ipc/glue/BackgroundParentImpl.h index f9f2c597a621..f0f626507f98 100644 --- a/ipc/glue/BackgroundParentImpl.h +++ b/ipc/glue/BackgroundParentImpl.h @@ -368,10 +368,8 @@ class BackgroundParentImpl : public PBackgroundParent, PServiceWorkerParent* aActor, const IPCServiceWorkerDescriptor& aDescriptor) override; - PServiceWorkerContainerParent* AllocPServiceWorkerContainerParent() override; - - bool DeallocPServiceWorkerContainerParent( - PServiceWorkerContainerParent*) override; + already_AddRefed + AllocPServiceWorkerContainerParent() final; mozilla::ipc::IPCResult RecvPServiceWorkerContainerConstructor( PServiceWorkerContainerParent* aActor) override;