Bug 1653470 - refcount PServiceWorkerRegistration r=dom-workers-and-s…

…torage-reviewers,asuth Depends on D83884 Differential Revision: https://phabricator.services.mozilla.com/D83885
dothq · Oct 23, 2020 · 0e70329 · 0e70329
1 parent 260415e
commit 0e70329
Show file tree

Hide file tree

Showing 15 changed files with 32 additions and 74 deletions.
diff --git a/dom/serviceworkers/PServiceWorkerRegistration.ipdl b/dom/serviceworkers/PServiceWorkerRegistration.ipdl
@@ -9,7 +9,7 @@ include IPCServiceWorkerRegistrationDescriptor;
 namespace mozilla {
 namespace dom {
 
-protocol PServiceWorkerRegistration
+refcounted protocol PServiceWorkerRegistration
 {
   manager PBackground;
 

diff --git a/dom/serviceworkers/RemoteServiceWorkerRegistrationImpl.cpp b/dom/serviceworkers/RemoteServiceWorkerRegistrationImpl.cpp
@@ -108,16 +108,15 @@ void RemoteServiceWorkerRegistrationImpl::Unregister(
 
 RemoteServiceWorkerRegistrationImpl::RemoteServiceWorkerRegistrationImpl(
     const ServiceWorkerRegistrationDescriptor& aDescriptor)
-    : mActor(nullptr), mOuter(nullptr), mShutdown(false) {
+    : mOuter(nullptr), mShutdown(false) {
   PBackgroundChild* parentActor =
       BackgroundChild::GetOrCreateForCurrentThread();
   if (NS_WARN_IF(!parentActor)) {
     Shutdown();
     return;
   }
 
-  ServiceWorkerRegistrationChild* actor =
-      ServiceWorkerRegistrationChild::Create();
+  auto actor = ServiceWorkerRegistrationChild::Create();
   if (NS_WARN_IF(!actor)) {
     Shutdown();
     return;
@@ -132,7 +131,7 @@ RemoteServiceWorkerRegistrationImpl::RemoteServiceWorkerRegistrationImpl(
   }
   MOZ_DIAGNOSTIC_ASSERT(sentActor == actor);
 
-  mActor = actor;
+  mActor = std::move(actor);
   mActor->SetOwner(this);
 }
 

diff --git a/dom/serviceworkers/RemoteServiceWorkerRegistrationImpl.h b/dom/serviceworkers/RemoteServiceWorkerRegistrationImpl.h
@@ -16,7 +16,7 @@ class ServiceWorkerRegistrationChild;
 
 class RemoteServiceWorkerRegistrationImpl final
     : public ServiceWorkerRegistration::Inner {
-  ServiceWorkerRegistrationChild* mActor;
+  RefPtr<ServiceWorkerRegistrationChild> mActor;
   ServiceWorkerRegistration* mOuter;
   bool mShutdown;
 

diff --git a/dom/serviceworkers/ServiceWorkerActors.cpp b/dom/serviceworkers/ServiceWorkerActors.cpp
@@ -27,28 +27,6 @@ void InitServiceWorkerContainerParent(PServiceWorkerContainerParent* aActor) {
   actor->Init();
 }
 
-PServiceWorkerRegistrationChild* AllocServiceWorkerRegistrationChild() {
-  MOZ_CRASH("should not be called");
-}
-
-bool DeallocServiceWorkerRegistrationChild(
-    PServiceWorkerRegistrationChild* aActor) {
-  auto actor = static_cast<ServiceWorkerRegistrationChild*>(aActor);
-  delete actor;
-  return true;
-}
-
-PServiceWorkerRegistrationParent* AllocServiceWorkerRegistrationParent() {
-  return new ServiceWorkerRegistrationParent();
-}
-
-bool DeallocServiceWorkerRegistrationParent(
-    PServiceWorkerRegistrationParent* aActor) {
-  auto actor = static_cast<ServiceWorkerRegistrationParent*>(aActor);
-  delete actor;
-  return true;
-}
-
 void InitServiceWorkerRegistrationParent(
     PServiceWorkerRegistrationParent* aActor,
     const IPCServiceWorkerRegistrationDescriptor& aDescriptor) {

diff --git a/dom/serviceworkers/ServiceWorkerActors.h b/dom/serviceworkers/ServiceWorkerActors.h
@@ -13,7 +13,6 @@ namespace dom {
 // PServiceWorker
 
 class IPCServiceWorkerDescriptor;
-class PServiceWorkerChild;
 class PServiceWorkerParent;
 
 void InitServiceWorkerParent(PServiceWorkerParent* aActor,
@@ -28,19 +27,8 @@ void InitServiceWorkerContainerParent(PServiceWorkerContainerParent* aActor);
 // PServiceWorkerRegistration
 
 class IPCServiceWorkerRegistrationDescriptor;
-class PServiceWorkerRegistrationChild;
 class PServiceWorkerRegistrationParent;
 
-PServiceWorkerRegistrationChild* AllocServiceWorkerRegistrationChild();
-
-bool DeallocServiceWorkerRegistrationChild(
-    PServiceWorkerRegistrationChild* aActor);
-
-PServiceWorkerRegistrationParent* AllocServiceWorkerRegistrationParent();
-
-bool DeallocServiceWorkerRegistrationParent(
-    PServiceWorkerRegistrationParent* aActor);
-
 void InitServiceWorkerRegistrationParent(
     PServiceWorkerRegistrationParent* aActor,
     const IPCServiceWorkerRegistrationDescriptor& aDescriptor);

diff --git a/dom/serviceworkers/ServiceWorkerRegistrationChild.cpp b/dom/serviceworkers/ServiceWorkerRegistrationChild.cpp
@@ -39,8 +39,9 @@ IPCResult ServiceWorkerRegistrationChild::RecvFireUpdateFound() {
 }
 
 // static
-ServiceWorkerRegistrationChild* ServiceWorkerRegistrationChild::Create() {
-  ServiceWorkerRegistrationChild* actor = new ServiceWorkerRegistrationChild();
+RefPtr<ServiceWorkerRegistrationChild>
+ServiceWorkerRegistrationChild::Create() {
+  RefPtr actor = new ServiceWorkerRegistrationChild;
 
   if (!NS_IsMainThread()) {
     WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
@@ -54,7 +55,6 @@ ServiceWorkerRegistrationChild* ServiceWorkerRegistrationChild::Create() {
         [helper] { helper->Actor()->MaybeStartTeardown(); });
 
     if (NS_WARN_IF(!actor->mIPCWorkerRef)) {
-      delete actor;
       return nullptr;
     }
   }

diff --git a/dom/serviceworkers/ServiceWorkerRegistrationChild.h b/dom/serviceworkers/ServiceWorkerRegistrationChild.h
@@ -23,6 +23,8 @@ class ServiceWorkerRegistrationChild final
 
   ServiceWorkerRegistrationChild();
 
+  ~ServiceWorkerRegistrationChild() = default;
+
   // PServiceWorkerRegistrationChild
   void ActorDestroy(ActorDestroyReason aReason) override;
 
@@ -32,9 +34,9 @@ class ServiceWorkerRegistrationChild final
   mozilla::ipc::IPCResult RecvFireUpdateFound() override;
 
  public:
-  static ServiceWorkerRegistrationChild* Create();
+  NS_INLINE_DECL_REFCOUNTING(ServiceWorkerRegistrationChild, override);
 
-  ~ServiceWorkerRegistrationChild() = default;
+  static RefPtr<ServiceWorkerRegistrationChild> Create();
 
   void SetOwner(RemoteServiceWorkerRegistrationImpl* aOwner);
 

diff --git a/dom/serviceworkers/ServiceWorkerRegistrationParent.h b/dom/serviceworkers/ServiceWorkerRegistrationParent.h
@@ -20,6 +20,8 @@ class ServiceWorkerRegistrationParent final
   RefPtr<ServiceWorkerRegistrationProxy> mProxy;
   bool mDeleteSent;
 
+  ~ServiceWorkerRegistrationParent();
+
   // PServiceWorkerRegistrationParent
   void ActorDestroy(ActorDestroyReason aReason) override;
 
@@ -32,8 +34,9 @@ class ServiceWorkerRegistrationParent final
                                      UpdateResolver&& aResolver) override;
 
  public:
+  NS_INLINE_DECL_REFCOUNTING(ServiceWorkerRegistrationParent, override);
+
   ServiceWorkerRegistrationParent();
-  ~ServiceWorkerRegistrationParent();
 
   void Init(const IPCServiceWorkerRegistrationDescriptor& aDescriptor);
 

diff --git a/dom/serviceworkers/ServiceWorkerRegistrationProxy.cpp b/dom/serviceworkers/ServiceWorkerRegistrationProxy.cpp
@@ -166,9 +166,7 @@ bool ServiceWorkerRegistrationProxy::MatchesDescriptor(
 
 ServiceWorkerRegistrationProxy::ServiceWorkerRegistrationProxy(
     const ServiceWorkerRegistrationDescriptor& aDescriptor)
-    : mActor(nullptr),
-      mEventTarget(GetCurrentSerialEventTarget()),
-      mDescriptor(aDescriptor) {}
+    : mEventTarget(GetCurrentSerialEventTarget()), mDescriptor(aDescriptor) {}
 
 void ServiceWorkerRegistrationProxy::Init(
     ServiceWorkerRegistrationParent* aActor) {

diff --git a/dom/serviceworkers/ServiceWorkerRegistrationProxy.h b/dom/serviceworkers/ServiceWorkerRegistrationProxy.h
@@ -22,7 +22,7 @@ class ServiceWorkerRegistrationParent;
 class ServiceWorkerRegistrationProxy final
     : public ServiceWorkerRegistrationListener {
   // Background thread only
-  ServiceWorkerRegistrationParent* mActor;
+  RefPtr<ServiceWorkerRegistrationParent> mActor;
 
   // Written on background thread and read on main thread
   nsCOMPtr<nsISerialEventTarget> mEventTarget;

diff --git a/dom/serviceworkers/moz.build b/dom/serviceworkers/moz.build
@@ -33,8 +33,10 @@ EXPORTS.mozilla.dom += [
     'ServiceWorkerParent.h',
     'ServiceWorkerRegistrar.h',
     'ServiceWorkerRegistration.h',
+    'ServiceWorkerRegistrationChild.h',
     'ServiceWorkerRegistrationDescriptor.h',
     'ServiceWorkerRegistrationInfo.h',
+    'ServiceWorkerRegistrationParent.h',
     'ServiceWorkerShutdownState.h',
     'ServiceWorkerUtils.h',
 ]

diff --git a/ipc/glue/BackgroundChildImpl.cpp b/ipc/glue/BackgroundChildImpl.cpp
@@ -653,15 +653,11 @@ BackgroundChildImpl::AllocPServiceWorkerContainerChild() {
   return mozilla::dom::ServiceWorkerContainerChild::Create();
 }
 
-PServiceWorkerRegistrationChild*
+already_AddRefed<PServiceWorkerRegistrationChild>
 BackgroundChildImpl::AllocPServiceWorkerRegistrationChild(
     const IPCServiceWorkerRegistrationDescriptor&) {
-  return dom::AllocServiceWorkerRegistrationChild();
-}
-
-bool BackgroundChildImpl::DeallocPServiceWorkerRegistrationChild(
-    PServiceWorkerRegistrationChild* aActor) {
-  return dom::DeallocServiceWorkerRegistrationChild(aActor);
+  MOZ_CRASH("Shouldn't be called.");
+  return {};
 }
 
 dom::PEndpointForReportChild* BackgroundChildImpl::AllocPEndpointForReportChild(

diff --git a/ipc/glue/BackgroundChildImpl.h b/ipc/glue/BackgroundChildImpl.h
@@ -249,11 +249,9 @@ class BackgroundChildImpl : public PBackgroundChild,
   already_AddRefed<PServiceWorkerContainerChild>
   AllocPServiceWorkerContainerChild();
 
-  virtual PServiceWorkerRegistrationChild* AllocPServiceWorkerRegistrationChild(
-      const IPCServiceWorkerRegistrationDescriptor&) override;
-
-  virtual bool DeallocPServiceWorkerRegistrationChild(
-      PServiceWorkerRegistrationChild*) override;
+  already_AddRefed<PServiceWorkerRegistrationChild>
+  AllocPServiceWorkerRegistrationChild(
+      const IPCServiceWorkerRegistrationDescriptor&);
 
   virtual PEndpointForReportChild* AllocPEndpointForReportChild(
       const nsString& aGroupName, const PrincipalInfo& aPrincipalInfo) override;

diff --git a/ipc/glue/BackgroundParentImpl.cpp b/ipc/glue/BackgroundParentImpl.cpp
@@ -42,6 +42,7 @@
 #include "mozilla/dom/ServiceWorkerManagerParent.h"
 #include "mozilla/dom/ServiceWorkerParent.h"
 #include "mozilla/dom/ServiceWorkerRegistrar.h"
+#include "mozilla/dom/ServiceWorkerRegistrationParent.h"
 #include "mozilla/dom/SessionStorageManager.h"
 #include "mozilla/dom/SharedWorkerParent.h"
 #include "mozilla/dom/StorageActivityService.h"
@@ -1277,15 +1278,10 @@ BackgroundParentImpl::RecvPServiceWorkerContainerConstructor(
   return IPC_OK();
 }
 
-PServiceWorkerRegistrationParent*
+already_AddRefed<PServiceWorkerRegistrationParent>
 BackgroundParentImpl::AllocPServiceWorkerRegistrationParent(
     const IPCServiceWorkerRegistrationDescriptor&) {
-  return dom::AllocServiceWorkerRegistrationParent();
-}
-
-bool BackgroundParentImpl::DeallocPServiceWorkerRegistrationParent(
-    PServiceWorkerRegistrationParent* aActor) {
-  return dom::DeallocServiceWorkerRegistrationParent(aActor);
+  return MakeAndAddRef<mozilla::dom::ServiceWorkerRegistrationParent>();
 }
 
 mozilla::ipc::IPCResult

diff --git a/ipc/glue/BackgroundParentImpl.h b/ipc/glue/BackgroundParentImpl.h
@@ -374,11 +374,9 @@ class BackgroundParentImpl : public PBackgroundParent,
   mozilla::ipc::IPCResult RecvPServiceWorkerContainerConstructor(
       PServiceWorkerContainerParent* aActor) override;
 
-  PServiceWorkerRegistrationParent* AllocPServiceWorkerRegistrationParent(
-      const IPCServiceWorkerRegistrationDescriptor&) override;
-
-  bool DeallocPServiceWorkerRegistrationParent(
-      PServiceWorkerRegistrationParent*) override;
+  already_AddRefed<PServiceWorkerRegistrationParent>
+  AllocPServiceWorkerRegistrationParent(
+      const IPCServiceWorkerRegistrationDescriptor&) final;
 
   mozilla::ipc::IPCResult RecvPServiceWorkerRegistrationConstructor(
       PServiceWorkerRegistrationParent* aActor,