Bug 1299594 - part 2 - remove opened actor tracking from IToplevelProtocol; r=billm
authorNathan Froyd <froydnj@gmail.com>
Fri, 02 Sep 2016 16:13:50 -0400
changeset 312409 67e8b862bdb08431c362d3f5e1cbb6a0cfbe3a13
parent 312408 ee21ab5cb4796059a8136f70b8dab2371e4d6213
child 312410 47f3a6275d66fe0a7d88cba05e375011be35b6c8
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1299594
milestone51.0a1
Bug 1299594 - part 2 - remove opened actor tracking from IToplevelProtocol; r=billm The only thing we needed opened actor tracking for was the ability to clone all the actors. But now that we no longer have support for cloning actors, we no longer need to track the actors that we've cloned, which makes a number of things significantly simpler.
gfx/ipc/VsyncBridgeChild.cpp
gfx/ipc/VsyncBridgeParent.cpp
gfx/layers/ipc/CompositorBridgeChild.cpp
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/ipc/ImageBridgeChild.cpp
gfx/layers/ipc/ImageBridgeParent.cpp
gfx/vr/ipc/VRManagerChild.cpp
gfx/vr/ipc/VRManagerParent.cpp
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
ipc/ipdl/ipdl/lower.py
--- a/gfx/ipc/VsyncBridgeChild.cpp
+++ b/gfx/ipc/VsyncBridgeChild.cpp
@@ -32,17 +32,17 @@ VsyncBridgeChild::Create(RefPtr<VsyncIOT
   aThread->GetThread()->Dispatch(task.forget(), nsIThread::DISPATCH_NORMAL);
 
   return child;
 }
 
 void
 VsyncBridgeChild::Open(Endpoint<PVsyncBridgeChild>&& aEndpoint)
 {
-  if (!aEndpoint.Bind(this, nullptr)) {
+  if (!aEndpoint.Bind(this)) {
     // The GPU Process Manager might be gone if we receive ActorDestroy very
     // late in shutdown.
     if (GPUProcessManager* gpm = GPUProcessManager::Get())
       gpm->NotifyRemoteActorDestroyed(mProcessToken);
     return;
   }
 
   mLoop = MessageLoop::current();
--- a/gfx/ipc/VsyncBridgeParent.cpp
+++ b/gfx/ipc/VsyncBridgeParent.cpp
@@ -30,17 +30,17 @@ VsyncBridgeParent::VsyncBridgeParent()
 VsyncBridgeParent::~VsyncBridgeParent()
 {
   MOZ_COUNT_DTOR(VsyncBridgeParent);
 }
 
 void
 VsyncBridgeParent::Open(Endpoint<PVsyncBridgeParent>&& aEndpoint)
 {
-  if (!aEndpoint.Bind(this, nullptr)) {
+  if (!aEndpoint.Bind(this)) {
     // We can't recover from this.
     MOZ_CRASH("Failed to bind VsyncBridgeParent to endpoint");
   }
   AddRef();
   mOpen = true;
 }
 
 bool
--- a/gfx/layers/ipc/CompositorBridgeChild.cpp
+++ b/gfx/layers/ipc/CompositorBridgeChild.cpp
@@ -178,17 +178,17 @@ CompositorBridgeChild::LookupCompositorF
 
 /* static */ bool
 CompositorBridgeChild::InitForContent(Endpoint<PCompositorBridgeChild>&& aEndpoint)
 {
   // There's only one compositor per child process.
   MOZ_ASSERT(!sCompositorBridge);
 
   RefPtr<CompositorBridgeChild> child(new CompositorBridgeChild(nullptr));
-  if (!aEndpoint.Bind(child, nullptr)) {
+  if (!aEndpoint.Bind(child)) {
     NS_RUNTIMEABORT("Couldn't Open() Compositor channel.");
     return false;
   }
 
   child->mCanSend = true;
 
   // We release this ref in DeferredDestroyCompositor.
   sCompositorBridge = child;
@@ -219,17 +219,17 @@ CompositorBridgeChild::InitSameProcess(w
 }
 
 /* static */ RefPtr<CompositorBridgeChild>
 CompositorBridgeChild::CreateRemote(const uint64_t& aProcessToken,
                                     ClientLayerManager* aLayerManager,
                                     Endpoint<PCompositorBridgeChild>&& aEndpoint)
 {
   RefPtr<CompositorBridgeChild> child = new CompositorBridgeChild(aLayerManager);
-  if (!aEndpoint.Bind(child, nullptr)) {
+  if (!aEndpoint.Bind(child)) {
     return nullptr;
   }
 
   child->mCanSend = true;
   child->mProcessToken = aProcessToken;
   return child;
 }
 
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -728,17 +728,17 @@ CompositorBridgeParent::InitSameProcess(
   mSelfRef = this;
 
   Initialize();
 }
 
 bool
 CompositorBridgeParent::Bind(Endpoint<PCompositorBridgeParent>&& aEndpoint)
 {
-  if (!aEndpoint.Bind(this, nullptr)) {
+  if (!aEndpoint.Bind(this)) {
     return false;
   }
   mSelfRef = this;
   return true;
 }
 
 bool
 CompositorBridgeParent::RecvInitialize(const uint64_t& aRootLayerTreeId)
@@ -2122,17 +2122,17 @@ public:
   explicit CrossProcessCompositorBridgeParent()
     : mNotifyAfterRemotePaint(false)
     , mDestroyCalled(false)
   {
     MOZ_ASSERT(NS_IsMainThread());
   }
 
   void Bind(Endpoint<PCompositorBridgeParent>&& aEndpoint) {
-    if (!aEndpoint.Bind(this, nullptr)) {
+    if (!aEndpoint.Bind(this)) {
       return;
     }
     mSelfRef = this;
   }
 
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   // FIXME/bug 774388: work out what shutdown protocol we need.
--- a/gfx/layers/ipc/ImageBridgeChild.cpp
+++ b/gfx/layers/ipc/ImageBridgeChild.cpp
@@ -910,17 +910,17 @@ ImageBridgeChild::InitForContent(Endpoin
     CallSendImageBridgeThreadId, sImageBridgeChildSingleton.get()));
 
   return sImageBridgeChildSingleton;
 }
 
 void
 ImageBridgeChild::Bind(Endpoint<PImageBridgeChild>&& aEndpoint)
 {
-  aEndpoint.Bind(this, nullptr);
+  aEndpoint.Bind(this);
 }
 
 void ImageBridgeChild::ShutDown()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   sIsShutDown = true;
 
--- a/gfx/layers/ipc/ImageBridgeParent.cpp
+++ b/gfx/layers/ipc/ImageBridgeParent.cpp
@@ -213,17 +213,17 @@ ImageBridgeParent::CreateForContent(Endp
     bridge, &ImageBridgeParent::Bind, Move(aEndpoint)));
 
   return true;
 }
 
 void
 ImageBridgeParent::Bind(Endpoint<PImageBridgeParent>&& aEndpoint)
 {
-  if (!aEndpoint.Bind(this, nullptr))
+  if (!aEndpoint.Bind(this))
     return;
   mSelfRef = this;
 }
 
 bool ImageBridgeParent::RecvWillClose()
 {
   // If there is any texture still alive we have to force it to deallocate the
   // device data (GL textures, etc.) now because shortly after SenStop() returns
--- a/gfx/vr/ipc/VRManagerChild.cpp
+++ b/gfx/vr/ipc/VRManagerChild.cpp
@@ -83,17 +83,17 @@ VRManagerChild::IsCreated()
 
 /* static */ bool
 VRManagerChild::InitForContent(Endpoint<PVRManagerChild>&& aEndpoint)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!sVRManagerChildSingleton);
 
   RefPtr<VRManagerChild> child(new VRManagerChild());
-  if (!aEndpoint.Bind(child, nullptr)) {
+  if (!aEndpoint.Bind(child)) {
     NS_RUNTIMEABORT("Couldn't Open() Compositor channel.");
     return false;
   }
   sVRManagerChildSingleton = child;
   return true;
 }
 
 /*static*/ void
@@ -111,17 +111,17 @@ VRManagerChild::InitSameProcess()
 
 /* static */ void
 VRManagerChild::InitWithGPUProcess(Endpoint<PVRManagerChild>&& aEndpoint)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!sVRManagerChildSingleton);
 
   sVRManagerChildSingleton = new VRManagerChild();
-  if (!aEndpoint.Bind(sVRManagerChildSingleton, nullptr)) {
+  if (!aEndpoint.Bind(sVRManagerChildSingleton)) {
     NS_RUNTIMEABORT("Couldn't Open() Compositor channel.");
     return;
   }
 }
 
 /*static*/ void
 VRManagerChild::ShutDown()
 {
--- a/gfx/vr/ipc/VRManagerParent.cpp
+++ b/gfx/vr/ipc/VRManagerParent.cpp
@@ -161,17 +161,17 @@ VRManagerParent::CreateForContent(Endpoi
     vmp, &VRManagerParent::Bind, Move(aEndpoint)));
 
   return true;
 }
 
 void
 VRManagerParent::Bind(Endpoint<PVRManagerParent>&& aEndpoint)
 {
-  if (!aEndpoint.Bind(this, nullptr)) {
+  if (!aEndpoint.Bind(this)) {
     return;
   }
   mSelfRef = this;
 
   RegisterWithManager();
 }
 
 /*static*/ void
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -62,100 +62,30 @@ ProtocolCloneContext::~ProtocolCloneCont
 void ProtocolCloneContext::SetContentParent(ContentParent* aContentParent)
 {
   mContentParent = aContentParent;
 }
 
 static StaticMutex gProtocolMutex;
 
 IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId)
- : mOpener(nullptr)
- , mProtocolId(aProtoId)
+ : mProtocolId(aProtoId)
 {
 }
 
 IToplevelProtocol::~IToplevelProtocol()
 {
   StaticMutexAutoLock al(gProtocolMutex);
 
-  for (IToplevelProtocol* actor = mOpenActors.getFirst();
-       actor;
-       actor = actor->getNext()) {
-    actor->mOpener = nullptr;
-  }
-
-  mOpenActors.clear();
-
-  if (mOpener) {
-      removeFrom(mOpener->mOpenActors);
-  }
-
   if (mTrans) {
     RefPtr<DeleteTask<Transport>> task = new DeleteTask<Transport>(mTrans.release());
     XRE_GetIOMessageLoop()->PostTask(task.forget());
   }
 }
 
-void
-IToplevelProtocol::AddOpenedActorLocked(IToplevelProtocol* aActor)
-{
-  gProtocolMutex.AssertCurrentThreadOwns();
-
-#ifdef DEBUG
-  for (const IToplevelProtocol* actor = mOpenActors.getFirst();
-       actor;
-       actor = actor->getNext()) {
-    NS_ASSERTION(actor != aActor,
-                 "Open the same protocol for more than one time");
-  }
-#endif
-
-  aActor->mOpener = this;
-  mOpenActors.insertBack(aActor);
-}
-
-void
-IToplevelProtocol::AddOpenedActor(IToplevelProtocol* aActor)
-{
-  StaticMutexAutoLock al(gProtocolMutex);
-  AddOpenedActorLocked(aActor);
-}
-
-void
-IToplevelProtocol::GetOpenedActorsLocked(nsTArray<IToplevelProtocol*>& aActors)
-{
-  gProtocolMutex.AssertCurrentThreadOwns();
-
-  for (IToplevelProtocol* actor = mOpenActors.getFirst();
-       actor;
-       actor = actor->getNext()) {
-    aActors.AppendElement(actor);
-  }
-}
-
-void
-IToplevelProtocol::GetOpenedActors(nsTArray<IToplevelProtocol*>& aActors)
-{
-  StaticMutexAutoLock al(gProtocolMutex);
-  GetOpenedActorsLocked(aActors);
-}
-
-size_t
-IToplevelProtocol::GetOpenedActorsUnsafe(IToplevelProtocol** aActors, size_t aActorsMax)
-{
-  size_t count = 0;
-  for (IToplevelProtocol* actor = mOpenActors.getFirst();
-       actor;
-       actor = actor->getNext()) {
-    MOZ_RELEASE_ASSERT(count < aActorsMax);
-    aActors[count++] = actor;
-  }
-  return count;
-}
-
 class ChannelOpened : public IPC::Message
 {
 public:
   ChannelOpened(TransportDescriptor aDescriptor,
                 ProcessId aOtherProcess,
                 ProtocolId aProtocol,
                 PriorityValue aPriority = PRIORITY_NORMAL)
     : IPC::Message(MSG_ROUTING_CONTROL, // these only go to top-level actors
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -211,60 +211,37 @@ template<class PFooSide>
 class Endpoint;
 
 /**
  * All top-level protocols should inherit this class.
  *
  * IToplevelProtocol tracks all top-level protocol actors created from
  * this protocol actor.
  */
-class IToplevelProtocol : private LinkedListElement<IToplevelProtocol>
+class IToplevelProtocol
 {
-    friend class LinkedList<IToplevelProtocol>;
-    friend class LinkedListElement<IToplevelProtocol>;
-
     template<class PFooSide> friend class Endpoint;
 
 protected:
     explicit IToplevelProtocol(ProtocolId aProtoId);
     ~IToplevelProtocol();
 
-    /**
-     * Add an actor to the list of actors that have been opened by this
-     * protocol.
-     */
-    void AddOpenedActor(IToplevelProtocol* aActor);
-
 public:
     void SetTransport(UniquePtr<Transport> aTrans)
     {
         mTrans = Move(aTrans);
     }
 
     Transport* GetTransport() const { return mTrans.get(); }
 
     ProtocolId GetProtocolId() const { return mProtocolId; }
 
-    void GetOpenedActors(nsTArray<IToplevelProtocol*>& aActors);
-
     virtual MessageChannel* GetIPCChannel() = 0;
 
-    // This Unsafe version should only be used when all other threads are
-    // frozen, since it performs no locking. It also takes a stack-allocated
-    // array and its size (number of elements) rather than an nsTArray. The Nuwa
-    // code that calls this function is not allowed to allocate memory.
-    size_t GetOpenedActorsUnsafe(IToplevelProtocol** aActors, size_t aActorsMax);
-
 private:
-    void AddOpenedActorLocked(IToplevelProtocol* aActor);
-    void GetOpenedActorsLocked(nsTArray<IToplevelProtocol*>& aActors);
-
-    LinkedList<IToplevelProtocol> mOpenActors; // All protocol actors opened by this.
-    IToplevelProtocol* mOpener;
-
     ProtocolId mProtocolId;
     UniquePtr<Transport> mTrans;
 };
 
 class IShmemAllocator
 {
 public:
   virtual bool AllocShmem(size_t aSize,
@@ -508,40 +485,32 @@ public:
         }
     }
 
     ProcessId OtherPid() const {
         return mOtherPid;
     }
 
     // This method binds aActor to this endpoint. After this call, the actor can
-    // be used to send and receive messages. The endpoint becomes invalid. The
-    // |aProcessActor| parameter is used to associate protocols with content
-    // processes. In practice, this parameter should always be a ContentParent
-    // or ContentChild, depending on which process you are in. It is used to
-    // find all the channels that need to be "frozen" or "revived" when creating
-    // or cloning the Nuwa process.
-    bool Bind(PFooSide* aActor, IToplevelProtocol* aProcessActor)
+    // be used to send and receive messages. The endpoint becomes invalid.
+    bool Bind(PFooSide* aActor)
     {
         MOZ_RELEASE_ASSERT(mValid);
         MOZ_RELEASE_ASSERT(mMyPid == base::GetCurrentProcId());
 
         UniquePtr<Transport> t = mozilla::ipc::OpenDescriptor(mTransport, mMode);
         if (!t) {
             return false;
         }
         if (!aActor->Open(t.get(), mOtherPid, XRE_GetIOMessageLoop(),
                           mMode == Transport::MODE_SERVER ? ParentSide : ChildSide)) {
             return false;
         }
         mValid = false;
         aActor->SetTransport(Move(t));
-        if (aProcessActor) {
-            aProcessActor->AddOpenedActor(aActor);
-        }
         return true;
     }
 
 
 private:
     friend struct IPC::ParamTraits<Endpoint<PFooSide>>;
 
     Endpoint(const Endpoint&) = delete;
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -4140,28 +4140,23 @@ class _GenerateProtocolActorCode(ipdl.as
                     _allocMethod(actor.ptype, actor.side),
                     args=[ _uniqueptrGet(tvar), pidvar ]))))
             iffailalloc.addifstmt(StmtReturn(_Result.ProcessingError))
 
             settrans = StmtExpr(ExprCall(
                 ExprSelect(pvar, '->', 'IToplevelProtocol::SetTransport'),
                 args=[ExprMove(tvar)]))
 
-            addopened = StmtExpr(ExprCall(
-                ExprVar('IToplevelProtocol::AddOpenedActor'),
-                args=[pvar]))
-
             case.addstmts([
                 StmtDecl(Decl(_uniqueptr(Type('Transport')), tvar.name)),
                 StmtDecl(Decl(Type(_actorName(actor.ptype.name(), actor.side),
                                    ptr=1), pvar.name)),
                 iffailopen,
                 iffailalloc,
                 settrans,
-                addopened,
                 StmtBreak()
             ])
             label = _messageStartName(actor.ptype)
             if actor.side == 'child':
                 label += 'Child'
             return CaseLabel(label), case
 
         pswitch = StmtSwitch(pvar)