From eb0f286337aaa8f54e686d67c51a3d9823a7109b Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Wed, 14 Oct 2020 15:58:36 +0000 Subject: [PATCH] Bug 1657404 - Change PGamepadEventChannel to "refcounted protocol" r=handyman Differential Revision: https://phabricator.services.mozilla.com/D93119 --- dom/gamepad/GamepadManager.cpp | 12 ++++----- dom/gamepad/GamepadManager.h | 6 +---- dom/gamepad/ipc/GamepadEventChannelChild.cpp | 5 ++++ dom/gamepad/ipc/GamepadEventChannelChild.h | 13 ++++++++-- dom/gamepad/ipc/GamepadEventChannelParent.cpp | 10 +++++--- dom/gamepad/ipc/GamepadEventChannelParent.h | 13 +++++++--- dom/gamepad/ipc/PGamepadEventChannel.ipdl | 2 +- ipc/glue/BackgroundChildImpl.cpp | 14 ----------- ipc/glue/BackgroundChildImpl.h | 6 ----- ipc/glue/BackgroundParentImpl.cpp | 25 ++----------------- ipc/glue/BackgroundParentImpl.h | 10 ++------ 11 files changed, 44 insertions(+), 72 deletions(-) diff --git a/dom/gamepad/GamepadManager.cpp b/dom/gamepad/GamepadManager.cpp index 916c381b4a51..1e7b70f978e5 100644 --- a/dom/gamepad/GamepadManager.cpp +++ b/dom/gamepad/GamepadManager.cpp @@ -115,8 +115,9 @@ GamepadManager::Observe(nsISupports* aSubject, const char* aTopic, } void GamepadManager::StopMonitoring() { - for (uint32_t i = 0; i < mChannelChildren.Length(); ++i) { - PGamepadEventChannelChild::Send__delete__(mChannelChildren[i]); + for (auto& channelChild : mChannelChildren) { + PGamepadEventChannelChild::Send__delete__(channelChild); + channelChild = nullptr; } if (gfx::VRManagerChild::IsCreated()) { gfx::VRManagerChild* vm = gfx::VRManagerChild::Get(); @@ -149,15 +150,12 @@ void GamepadManager::AddListener(nsGlobalWindowInner* aWindow) { return; } - GamepadEventChannelChild* child = new GamepadEventChannelChild(); - PGamepadEventChannelChild* initedChild = - actor->SendPGamepadEventChannelConstructor(child); - if (NS_WARN_IF(!initedChild)) { + RefPtr child(GamepadEventChannelChild::Create()); + if (!actor->SendPGamepadEventChannelConstructor(child.get())) { // We are probably shutting down. return; } - MOZ_ASSERT(initedChild == child); mChannelChildren.AppendElement(child); if (gfx::VRManagerChild::IsCreated()) { diff --git a/dom/gamepad/GamepadManager.h b/dom/gamepad/GamepadManager.h index 9e66589f2497..c44ad7878a65 100644 --- a/dom/gamepad/GamepadManager.h +++ b/dom/gamepad/GamepadManager.h @@ -119,11 +119,7 @@ class GamepadManager final : public nsIObserver { // true when shutdown has begun bool mShuttingDown; - // Gamepad IPDL child - // This pointer is only used by this singleton instance and - // will be destroyed during the IPDL shutdown chain, so we - // don't need to refcount it here. - nsTArray mChannelChildren; + nsTArray> mChannelChildren; private: nsresult Init(); diff --git a/dom/gamepad/ipc/GamepadEventChannelChild.cpp b/dom/gamepad/ipc/GamepadEventChannelChild.cpp index 1fee730b1381..9c3bf35ee4ee 100644 --- a/dom/gamepad/ipc/GamepadEventChannelChild.cpp +++ b/dom/gamepad/ipc/GamepadEventChannelChild.cpp @@ -29,6 +29,11 @@ class GamepadUpdateRunnable final : public Runnable { } // namespace +already_AddRefed GamepadEventChannelChild::Create() { + return RefPtr(new GamepadEventChannelChild()) + .forget(); +} + mozilla::ipc::IPCResult GamepadEventChannelChild::RecvGamepadUpdate( const GamepadChangeEvent& aGamepadEvent) { DebugOnly rv = diff --git a/dom/gamepad/ipc/GamepadEventChannelChild.h b/dom/gamepad/ipc/GamepadEventChannelChild.h index 2a6576406a26..78d337e29c95 100644 --- a/dom/gamepad/ipc/GamepadEventChannelChild.h +++ b/dom/gamepad/ipc/GamepadEventChannelChild.h @@ -14,15 +14,24 @@ namespace dom { class GamepadEventChannelChild final : public PGamepadEventChannelChild { public: - GamepadEventChannelChild() = default; - ~GamepadEventChannelChild() = default; + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(GamepadEventChannelChild, override) + + static already_AddRefed Create(); mozilla::ipc::IPCResult RecvGamepadUpdate( const GamepadChangeEvent& aGamepadEvent); mozilla::ipc::IPCResult RecvReplyGamepadPromise(const uint32_t& aPromiseID); void AddPromise(const uint32_t& aID, dom::Promise* aPromise); + GamepadEventChannelChild(const GamepadEventChannelChild&) = delete; + GamepadEventChannelChild(GamepadEventChannelChild&&) = delete; + GamepadEventChannelChild& operator=(const GamepadEventChannelChild&) = delete; + GamepadEventChannelChild& operator=(GamepadEventChannelChild&&) = delete; + private: + GamepadEventChannelChild() = default; + ~GamepadEventChannelChild() = default; + nsRefPtrHashtable mPromiseList; }; diff --git a/dom/gamepad/ipc/GamepadEventChannelParent.cpp b/dom/gamepad/ipc/GamepadEventChannelParent.cpp index 2c932658e964..1defa1682e44 100644 --- a/dom/gamepad/ipc/GamepadEventChannelParent.cpp +++ b/dom/gamepad/ipc/GamepadEventChannelParent.cpp @@ -38,7 +38,13 @@ class SendGamepadUpdateRunnable final : public Runnable { } // namespace -bool GamepadEventChannelParent::Init() { +already_AddRefed +GamepadEventChannelParent::Create() { + return RefPtr(new GamepadEventChannelParent()) + .forget(); +} + +GamepadEventChannelParent::GamepadEventChannelParent() { AssertIsOnBackgroundThread(); mBackgroundEventTarget = GetCurrentEventTarget(); @@ -48,8 +54,6 @@ bool GamepadEventChannelParent::Init() { MOZ_ASSERT(service); service->AddChannelParent(this); - - return true; } void GamepadEventChannelParent::ActorDestroy(ActorDestroyReason aWhy) { diff --git a/dom/gamepad/ipc/GamepadEventChannelParent.h b/dom/gamepad/ipc/GamepadEventChannelParent.h index e5c4a5d14500..02f0bc004c7f 100644 --- a/dom/gamepad/ipc/GamepadEventChannelParent.h +++ b/dom/gamepad/ipc/GamepadEventChannelParent.h @@ -13,10 +13,9 @@ namespace dom { class GamepadEventChannelParent final : public PGamepadEventChannelParent { public: - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(GamepadEventChannelParent) - GamepadEventChannelParent() = default; + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(GamepadEventChannelParent, override) - bool Init(); + static already_AddRefed Create(); void ActorDestroy(ActorDestroyReason aWhy) override; mozilla::ipc::IPCResult RecvVibrateHaptic(const uint32_t& aControllerIdx, @@ -31,8 +30,16 @@ class GamepadEventChannelParent final : public PGamepadEventChannelParent { const uint32_t& aPromiseID); void DispatchUpdateEvent(const GamepadChangeEvent& aEvent); + GamepadEventChannelParent(const GamepadEventChannelParent&) = delete; + GamepadEventChannelParent(GamepadEventChannelParent&&) = delete; + GamepadEventChannelParent& operator=(const GamepadEventChannelParent&) = + delete; + GamepadEventChannelParent& operator=(GamepadEventChannelParent&&) = delete; + private: + GamepadEventChannelParent(); ~GamepadEventChannelParent() = default; + nsCOMPtr mBackgroundEventTarget; }; diff --git a/dom/gamepad/ipc/PGamepadEventChannel.ipdl b/dom/gamepad/ipc/PGamepadEventChannel.ipdl index f1eb47c27b10..71a759ec4235 100644 --- a/dom/gamepad/ipc/PGamepadEventChannel.ipdl +++ b/dom/gamepad/ipc/PGamepadEventChannel.ipdl @@ -7,7 +7,7 @@ include GamepadEventTypes; namespace mozilla { namespace dom { -async protocol PGamepadEventChannel { +refcounted protocol PGamepadEventChannel { manager PBackground; parent: async __delete__(); diff --git a/ipc/glue/BackgroundChildImpl.cpp b/ipc/glue/BackgroundChildImpl.cpp index 63501b784926..ac4ac59624bb 100644 --- a/ipc/glue/BackgroundChildImpl.cpp +++ b/ipc/glue/BackgroundChildImpl.cpp @@ -605,20 +605,6 @@ bool BackgroundChildImpl::DeallocPMIDIManagerChild(PMIDIManagerChild* aActor) { return true; } -// Gamepad API Background IPC -dom::PGamepadEventChannelChild* -BackgroundChildImpl::AllocPGamepadEventChannelChild() { - MOZ_CRASH("PGamepadEventChannelChild actor should be manually constructed!"); - return nullptr; -} - -bool BackgroundChildImpl::DeallocPGamepadEventChannelChild( - PGamepadEventChannelChild* aActor) { - MOZ_ASSERT(aActor); - delete static_cast(aActor); - return true; -} - dom::PGamepadTestChannelChild* BackgroundChildImpl::AllocPGamepadTestChannelChild() { MOZ_CRASH("PGamepadTestChannelChild actor should be manually constructed!"); diff --git a/ipc/glue/BackgroundChildImpl.h b/ipc/glue/BackgroundChildImpl.h index d7234cd6d7f5..120cd9ca9f69 100644 --- a/ipc/glue/BackgroundChildImpl.h +++ b/ipc/glue/BackgroundChildImpl.h @@ -223,12 +223,6 @@ class BackgroundChildImpl : public PBackgroundChild, virtual bool DeallocPQuotaChild(PQuotaChild* aActor) override; - // Gamepad API Background IPC - virtual PGamepadEventChannelChild* AllocPGamepadEventChannelChild() override; - - virtual bool DeallocPGamepadEventChannelChild( - PGamepadEventChannelChild* aActor) override; - virtual PGamepadTestChannelChild* AllocPGamepadTestChannelChild() override; virtual bool DeallocPGamepadTestChannelChild( diff --git a/ipc/glue/BackgroundParentImpl.cpp b/ipc/glue/BackgroundParentImpl.cpp index 325ec5105320..3f8c6a9ee7df 100644 --- a/ipc/glue/BackgroundParentImpl.cpp +++ b/ipc/glue/BackgroundParentImpl.cpp @@ -1135,30 +1135,9 @@ mozilla::ipc::IPCResult BackgroundParentImpl::RecvPFileSystemRequestConstructor( } // Gamepad API Background IPC -dom::PGamepadEventChannelParent* +already_AddRefed BackgroundParentImpl::AllocPGamepadEventChannelParent() { - RefPtr parent = - new dom::GamepadEventChannelParent(); - - return parent.forget().take(); -} - -bool BackgroundParentImpl::DeallocPGamepadEventChannelParent( - dom::PGamepadEventChannelParent* aActor) { - MOZ_ASSERT(aActor); - RefPtr parent = - dont_AddRef(static_cast(aActor)); - return true; -} - -mozilla::ipc::IPCResult -BackgroundParentImpl::RecvPGamepadEventChannelConstructor( - PGamepadEventChannelParent* aActor) { - MOZ_ASSERT(aActor); - if (!static_cast(aActor)->Init()) { - return IPC_FAIL_NO_REASON(this); - } - return IPC_OK(); + return dom::GamepadEventChannelParent::Create(); } dom::PGamepadTestChannelParent* diff --git a/ipc/glue/BackgroundParentImpl.h b/ipc/glue/BackgroundParentImpl.h index 9a46ecebfc97..a768e86627cb 100644 --- a/ipc/glue/BackgroundParentImpl.h +++ b/ipc/glue/BackgroundParentImpl.h @@ -333,14 +333,8 @@ class BackgroundParentImpl : public PBackgroundParent, PFileSystemRequestParent* actor, const FileSystemParams& params) override; // Gamepad API Background IPC - virtual PGamepadEventChannelParent* AllocPGamepadEventChannelParent() - override; - - virtual bool DeallocPGamepadEventChannelParent( - PGamepadEventChannelParent* aActor) override; - - virtual mozilla::ipc::IPCResult RecvPGamepadEventChannelConstructor( - PGamepadEventChannelParent* aActor) override; + virtual already_AddRefed + AllocPGamepadEventChannelParent() override; virtual PGamepadTestChannelParent* AllocPGamepadTestChannelParent() override;