Bug 1451363 - part 2b - move protocol event target access into ProtocolState; r=mccr8
authorNathan Froyd <froydnj@mozilla.com>
Mon, 23 Apr 2018 14:13:37 -0400
changeset 468681 3978808c33d927f83929f76688e9c39e185a8a4b
parent 468680 78353bf75968371ecbdcb447b5452f28dd757e85
child 468682 ea228cf8cee53b56a8e0d493d2ee985256e97bc9
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1451363
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1451363 - part 2b - move protocol event target access into ProtocolState; r=mccr8 The reasoning here is the same as for the protocol register/lookup functions: these functions are all basic functionality that should not be overriden by subclasses.
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -529,65 +529,78 @@ IProtocol::SetManager(IProtocol* aManage
   mManager = aManager;
 }
 
 void
 IProtocol::SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget)
 {
   // Make sure we have a manager for the internal method to access.
   aActor->SetManager(this);
-  SetEventTargetForActorInternal(aActor, aEventTarget);
+  mState->SetEventTargetForActor(aActor, aEventTarget);
 }
 
 void
 IProtocol::ReplaceEventTargetForActor(IProtocol* aActor,
                                       nsIEventTarget* aEventTarget)
 {
   // Ensure the actor has been registered.
   MOZ_ASSERT(aActor->Manager());
-  ReplaceEventTargetForActorInternal(aActor, aEventTarget);
-}
-
-void
-IProtocol::SetEventTargetForActorInternal(IProtocol* aActor,
-                                          nsIEventTarget* aEventTarget)
-{
-  Manager()->SetEventTargetForActorInternal(aActor, aEventTarget);
-}
-
-void
-IProtocol::ReplaceEventTargetForActorInternal(IProtocol* aActor,
-                                              nsIEventTarget* aEventTarget)
-{
-  Manager()->ReplaceEventTargetForActorInternal(aActor, aEventTarget);
+  mState->ReplaceEventTargetForActor(aActor, aEventTarget);
 }
 
 nsIEventTarget*
 IProtocol::GetActorEventTarget()
 {
+  return mState->GetActorEventTarget();
+}
+
+already_AddRefed<nsIEventTarget>
+IProtocol::GetActorEventTarget(IProtocol* aActor)
+{
+  return mState->GetActorEventTarget(aActor);
+}
+
+nsIEventTarget*
+IProtocol::ManagedState::GetActorEventTarget()
+{
   // We should only call this function when this actor has been registered and
   // is not unregistered yet.
-  MOZ_RELEASE_ASSERT(mId != kNullActorId && mId != kFreedActorId);
-  RefPtr<nsIEventTarget> target = Manager()->GetActorEventTargetInternal(this);
+  MOZ_RELEASE_ASSERT(mProtocol->mId != kNullActorId && mProtocol->mId != kFreedActorId);
+  RefPtr<nsIEventTarget> target = GetActorEventTarget(mProtocol);
   return target;
 }
 
+void
+IProtocol::ManagedState::SetEventTargetForActor(IProtocol* aActor,
+                                                        nsIEventTarget* aEventTarget)
+{
+  // Go directly through the state so we don't try to redundantly (and
+  // wrongly) call SetManager() on aActor.
+  mProtocol->Manager()->mState->SetEventTargetForActor(aActor, aEventTarget);
+}
+
+void
+IProtocol::ManagedState::ReplaceEventTargetForActor(IProtocol* aActor,
+                                                            nsIEventTarget* aEventTarget)
+{
+  mProtocol->Manager()->ReplaceEventTargetForActor(aActor, aEventTarget);
+}
+
 already_AddRefed<nsIEventTarget>
-IProtocol::GetActorEventTargetInternal(IProtocol* aActor)
+IProtocol::ManagedState::GetActorEventTarget(IProtocol* aActor)
 {
-  return Manager()->GetActorEventTargetInternal(aActor);
+  return mProtocol->Manager()->GetActorEventTarget(aActor);
 }
 
 IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId, Side aSide)
  : IProtocol(aSide, MakeUnique<ToplevelState>(this, aSide)),
    mMonitor("mozilla.ipc.IToplevelProtocol.mMonitor"),
    mProtocolId(aProtoId),
    mOtherPid(mozilla::ipc::kInvalidProcessId),
-   mOtherPidState(ProcessIdState::eUnstarted),
-   mEventTargetMutex("ProtocolEventTargetMutex")
+   mOtherPidState(ProcessIdState::eUnstarted)
 {
 }
 
 IToplevelProtocol::~IToplevelProtocol()
 {
   if (mTrans) {
     RefPtr<DeleteTask<Transport>> task = new DeleteTask<Transport>(mTrans.release());
     XRE_GetIOMessageLoop()->PostTask(task.forget());
@@ -737,16 +750,17 @@ IToplevelProtocol::ToplevelState::Unregi
   MutexAutoLock lock(mEventTargetMutex);
   mEventTargetMap.RemoveIfPresent(aId);
 }
 
 IToplevelProtocol::ToplevelState::ToplevelState(IToplevelProtocol* aProtocol, Side aSide)
   : mProtocol(aProtocol)
   , mLastRouteId(aSide == ParentSide ? kFreedActorId : kNullActorId)
   , mLastShmemId(aSide == ParentSide ? kFreedActorId : kNullActorId)
+  , mEventTargetMutex("ProtocolEventTargetMutex")
 {
 }
 
 Shmem::SharedMemory*
 IToplevelProtocol::ToplevelState::CreateSharedMemory(size_t aSize,
                                                      Shmem::SharedMemory::SharedMemoryType aType,
                                                      bool aUnsafe,
                                                      Shmem::id_t* aId)
@@ -857,17 +871,17 @@ IToplevelProtocol::ToplevelState::ShmemD
   if (rawmem) {
     mShmemMap.Remove(id);
     Shmem::Dealloc(Shmem::PrivateIPDLCaller(), rawmem);
   }
   return true;
 }
 
 already_AddRefed<nsIEventTarget>
-IToplevelProtocol::GetMessageEventTarget(const Message& aMsg)
+IToplevelProtocol::ToplevelState::GetMessageEventTarget(const Message& aMsg)
 {
   int32_t route = aMsg.routing_id();
 
   Maybe<MutexAutoLock> lock;
   lock.emplace(mEventTargetMutex);
 
   nsCOMPtr<nsIEventTarget> target = mEventTargetMap.Lookup(route);
 
@@ -878,59 +892,53 @@ IToplevelProtocol::GetMessageEventTarget
       return nullptr;
     }
 
     // Normally a new actor inherits its event target from its manager. If the
     // manager has no event target, we give the subclass a chance to make a new
     // one.
     if (!target) {
       MutexAutoUnlock unlock(mEventTargetMutex);
-      target = GetConstructedEventTarget(aMsg);
+      target = mProtocol->GetConstructedEventTarget(aMsg);
     }
 
     mEventTargetMap.AddWithID(target, handle.mId);
   } else if (!target) {
     // We don't need the lock after this point.
     lock.reset();
 
-    target = GetSpecificMessageEventTarget(aMsg);
+    target = mProtocol->GetSpecificMessageEventTarget(aMsg);
   }
 
   return target.forget();
 }
 
 already_AddRefed<nsIEventTarget>
-IToplevelProtocol::GetActorEventTargetInternal(IProtocol* aActor)
+IToplevelProtocol::ToplevelState::GetActorEventTarget(IProtocol* aActor)
 {
   MOZ_RELEASE_ASSERT(aActor->Id() != kNullActorId && aActor->Id() != kFreedActorId);
 
   MutexAutoLock lock(mEventTargetMutex);
   nsCOMPtr<nsIEventTarget> target = mEventTargetMap.Lookup(aActor->Id());
   return target.forget();
 }
 
-already_AddRefed<nsIEventTarget>
-IToplevelProtocol::GetActorEventTarget(IProtocol* aActor)
-{
-  return GetActorEventTargetInternal(aActor);
-}
-
 nsIEventTarget*
-IToplevelProtocol::GetActorEventTarget()
+IToplevelProtocol::ToplevelState::GetActorEventTarget()
 {
   // The EventTarget of a ToplevelProtocol shall never be set.
   return nullptr;
 }
 
 void
-IToplevelProtocol::SetEventTargetForActorInternal(IProtocol* aActor,
-                                                  nsIEventTarget* aEventTarget)
+IToplevelProtocol::ToplevelState::SetEventTargetForActor(IProtocol* aActor,
+                                                 nsIEventTarget* aEventTarget)
 {
   // The EventTarget of a ToplevelProtocol shall never be set.
-  MOZ_RELEASE_ASSERT(aActor != this);
+  MOZ_RELEASE_ASSERT(aActor != mProtocol);
 
   // We should only call this function on actors that haven't been used for IPC
   // code yet. Otherwise we'll be posting stuff to the wrong event target before
   // we're called.
   MOZ_RELEASE_ASSERT(aActor->Id() == kNullActorId || aActor->Id() == kFreedActorId);
 
   // Register the actor early. When it's registered again, it will keep the same
   // ID.
@@ -947,22 +955,22 @@ IToplevelProtocol::SetEventTargetForActo
   if (replace) {
     mEventTargetMap.ReplaceWithID(aEventTarget, id);
   } else {
     mEventTargetMap.AddWithID(aEventTarget, id);
   }
 }
 
 void
-IToplevelProtocol::ReplaceEventTargetForActorInternal(
+IToplevelProtocol::ToplevelState::ReplaceEventTargetForActor(
   IProtocol* aActor,
   nsIEventTarget* aEventTarget)
 {
   // The EventTarget of a ToplevelProtocol shall never be set.
-  MOZ_RELEASE_ASSERT(aActor != this);
+  MOZ_RELEASE_ASSERT(aActor != mProtocol);
 
   int32_t id = aActor->Id();
   // The ID of the actor should have existed.
   MOZ_RELEASE_ASSERT(id!= kNullActorId && id!= kFreedActorId);
 
   MutexAutoLock lock(mEventTargetMutex);
   mEventTargetMap.ReplaceWithID(aEventTarget, id);
 }
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -168,16 +168,25 @@ public:
         virtual bool IsTrackingSharedMemory(Shmem::SharedMemory*) = 0;
         virtual bool DestroySharedMemory(Shmem&) = 0;
 
         // Protocol management functions.
         virtual int32_t Register(IProtocol*) = 0;
         virtual int32_t RegisterID(IProtocol*, int32_t) = 0;
         virtual IProtocol* Lookup(int32_t) = 0;
         virtual void Unregister(int32_t) = 0;
+
+        // Returns the event target set by SetEventTargetForActor() if available.
+        virtual nsIEventTarget* GetActorEventTarget() = 0;
+
+        virtual void SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget) = 0;
+        virtual void ReplaceEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget) = 0;
+
+        virtual already_AddRefed<nsIEventTarget>
+        GetActorEventTarget(IProtocol* aActor) = 0;
     };
 
     // Managed protocols just forward all of their operations to the topmost
     // managing protocol.
     class ManagedState final : public ProtocolState
     {
     public:
         explicit ManagedState(IProtocol* aProtocol)
@@ -190,16 +199,21 @@ public:
         bool IsTrackingSharedMemory(Shmem::SharedMemory*) override;
         bool DestroySharedMemory(Shmem&) override;
 
         int32_t Register(IProtocol*) override;
         int32_t RegisterID(IProtocol*, int32_t) override;
         IProtocol* Lookup(int32_t) override;
         void Unregister(int32_t) override;
 
+        nsIEventTarget* GetActorEventTarget() override;
+        void SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget) override;
+        void ReplaceEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget) override;
+        already_AddRefed<nsIEventTarget> GetActorEventTarget(IProtocol* aActor) override;
+
     private:
         IProtocol* const mProtocol;
     };
 
     typedef base::ProcessId ProcessId;
     typedef IPC::Message Message;
     typedef IPC::MessageInfo MessageInfo;
 
@@ -279,18 +293,18 @@ public:
     void SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget);
 
     // Replace the event target for the messages of aActor. There must not be
     // any messages of aActor in the task queue, or we might run into some
     // unexpected behavior.
     void ReplaceEventTargetForActor(IProtocol* aActor,
                                     nsIEventTarget* aEventTarget);
 
-    // Returns the event target set by SetEventTargetForActor() if available.
-    virtual nsIEventTarget* GetActorEventTarget();
+    nsIEventTarget* GetActorEventTarget();
+    already_AddRefed<nsIEventTarget> GetActorEventTarget(IProtocol* aActor);
 
 protected:
     IProtocol(Side aSide, UniquePtr<ProtocolState> aState)
         : mId(0)
         , mSide(aSide)
         , mManager(nullptr)
         , mChannel(nullptr)
         , mState(Move(aState))
@@ -298,24 +312,16 @@ protected:
 
     friend class IToplevelProtocol;
 
     void SetId(int32_t aId) { mId = aId; }
     void ResetManager() { mManager = nullptr; }
     void SetManager(IProtocol* aManager);
     void SetIPCChannel(MessageChannel* aChannel) { mChannel = aChannel; }
 
-    virtual void SetEventTargetForActorInternal(IProtocol* aActor, nsIEventTarget* aEventTarget);
-    virtual void ReplaceEventTargetForActorInternal(
-      IProtocol* aActor,
-      nsIEventTarget* aEventTarget);
-
-    virtual already_AddRefed<nsIEventTarget>
-    GetActorEventTargetInternal(IProtocol* aActor);
-
     static const int32_t kNullActorId = 0;
     static const int32_t kFreedActorId = 1;
 
 private:
     int32_t mId;
     Side mSide;
     IProtocol* mManager;
     MessageChannel* mChannel;
@@ -385,22 +391,33 @@ public:
         bool ShmemCreated(const Message& aMsg);
         bool ShmemDestroyed(const Message& aMsg);
 
         int32_t Register(IProtocol*) override;
         int32_t RegisterID(IProtocol*, int32_t) override;
         IProtocol* Lookup(int32_t) override;
         void Unregister(int32_t) override;
 
+        nsIEventTarget* GetActorEventTarget() override;
+        void SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget) override;
+        void ReplaceEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget) override;
+        already_AddRefed<nsIEventTarget> GetActorEventTarget(IProtocol* aActor) override;
+
+        virtual already_AddRefed<nsIEventTarget>
+        GetMessageEventTarget(const Message& aMsg);
+
     private:
         IToplevelProtocol* const mProtocol;
         IDMap<IProtocol*> mActorMap;
         int32_t mLastRouteId;
         IDMap<Shmem::SharedMemory*> mShmemMap;
         Shmem::id_t mLastShmemId;
+
+        Mutex mEventTargetMutex;
+        IDMap<nsCOMPtr<nsIEventTarget>> mEventTargetMap;
     };
 
     using SchedulerGroupSet = nsILabelableRunnable::SchedulerGroupSet;
 
     void SetTransport(UniquePtr<Transport> aTrans)
     {
         mTrans = Move(aTrans);
     }
@@ -514,69 +531,54 @@ public:
     // when it's difficult to determine an EventTarget ahead of time. See the
     // comment in nsILabelableRunnable.h for more information.
     virtual bool
     GetMessageSchedulerGroups(const Message& aMsg, SchedulerGroupSet& aGroups)
     {
         return false;
     }
 
-    virtual already_AddRefed<nsIEventTarget>
-    GetMessageEventTarget(const Message& aMsg);
-
-    already_AddRefed<nsIEventTarget>
-    GetActorEventTarget(IProtocol* aActor);
-
-    virtual nsIEventTarget*
-    GetActorEventTarget() override;
-
     virtual void OnChannelReceivedMessage(const Message& aMsg) {}
 
     bool IsMainThreadProtocol() const { return mIsMainThreadProtocol; }
     void SetIsMainThreadProtocol() { mIsMainThreadProtocol = NS_IsMainThread(); }
 
+    already_AddRefed<nsIEventTarget>
+    GetMessageEventTarget(const Message& aMsg)
+    {
+        return DowncastState()->GetMessageEventTarget(aMsg);
+    }
+
 protected:
     ToplevelState* DowncastState() const
     {
         return static_cast<ToplevelState*>(mState.get());
     }
 
     // Override this method in top-level protocols to change the event target
     // for a new actor (and its sub-actors).
     virtual already_AddRefed<nsIEventTarget>
     GetConstructedEventTarget(const Message& aMsg) { return nullptr; }
 
     // Override this method in top-level protocols to change the event target
     // for specific messages.
     virtual already_AddRefed<nsIEventTarget>
     GetSpecificMessageEventTarget(const Message& aMsg) { return nullptr; }
 
-    virtual void SetEventTargetForActorInternal(IProtocol* aActor,
-                                                nsIEventTarget* aEventTarget) override;
-    virtual void ReplaceEventTargetForActorInternal(
-      IProtocol* aActor,
-      nsIEventTarget* aEventTarget) override;
-
-    virtual already_AddRefed<nsIEventTarget>
-    GetActorEventTargetInternal(IProtocol* aActor) override;
-
     // This monitor protects mOtherPid and mOtherPidState. All other fields
     // should only be accessed on the worker thread.
     mutable mozilla::Monitor mMonitor;
   private:
     base::ProcessId OtherPidMaybeInvalid() const;
 
     ProtocolId mProtocolId;
     UniquePtr<Transport> mTrans;
     base::ProcessId mOtherPid;
     ProcessIdState mOtherPidState;
     bool mIsMainThreadProtocol;
-
-    Mutex mEventTargetMutex;
-    IDMap<nsCOMPtr<nsIEventTarget>> mEventTargetMap;
 };
 
 class IShmemAllocator
 {
 public:
   virtual bool AllocShmem(size_t aSize,
                           mozilla::ipc::SharedMemory::SharedMemoryType aShmType,
                           mozilla::ipc::Shmem* aShmem) = 0;