Revert "Bug 1317844 - Allow an event target to be specified for each IPC actor (r=dvander)"
authorBill McCloskey <billm@mozilla.com>
Mon, 21 Nov 2016 16:49:44 -0800
changeset 351742 d78b098b868b56b63b117ac2f5b7eda05649278b
parent 351741 10452c86cfbd237afff7881c4981c144d7308802
child 351743 688a4bbf801804c595cd47f316501f437390c9cd
push id10621
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 16:02:43 +0000
treeherdermozilla-aurora@dca7b42e6c67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs1317844
milestone53.0a1
Revert "Bug 1317844 - Allow an event target to be specified for each IPC actor (r=dvander)" This reverts commit 30396b9e84304ad7ee7d5a253ec886325a0f0cbe.
ipc/glue/MessageChannel.cpp
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
ipc/ipdl/ipdl/lower.py
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -1623,24 +1623,17 @@ void
 MessageChannel::MessageTask::Post()
 {
     MOZ_RELEASE_ASSERT(!mScheduled);
     MOZ_RELEASE_ASSERT(isInList());
 
     mScheduled = true;
 
     RefPtr<MessageTask> self = this;
-    nsCOMPtr<nsIEventTarget> eventTarget =
-        mChannel->mListener->GetMessageEventTarget(mMessage);
-
-    if (eventTarget) {
-        eventTarget->Dispatch(self.forget(), NS_DISPATCH_NORMAL);
-    } else {
-        mChannel->mWorkerLoop->PostTask(self.forget());
-    }
+    mChannel->mWorkerLoop->PostTask(self.forget());
 }
 
 void
 MessageChannel::MessageTask::Clear()
 {
     mChannel->AssertWorkerThread();
 
     mChannel = nullptr;
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -412,19 +412,16 @@ IProtocol*
 IProtocol::Lookup(int32_t aId)
 {
   return Manager()->Lookup(aId);
 }
 
 void
 IProtocol::Unregister(int32_t aId)
 {
-  if (mId == aId) {
-    mId = kFreedActorId;
-  }
   Manager()->Unregister(aId);
 }
 
 Shmem::SharedMemory*
 IProtocol::CreateSharedMemory(size_t aSize,
                               SharedMemory::SharedMemoryType aType,
                               bool aUnsafe,
                               int32_t* aId)
@@ -516,45 +513,22 @@ IProtocol::DeallocShmem(Shmem& aMem)
     }
     return false;
   }
 #endif // DEBUG
   aMem.forget(Shmem::IHadBetterBeIPDLCodeCallingThis_OtherwiseIAmADoodyhead());
   return ok;
 }
 
-void
-IProtocol::SetManager(IProtocol* aManager)
-{
-  MOZ_RELEASE_ASSERT(!mManager || mManager == aManager);
-  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);
-}
-
-void
-IProtocol::SetEventTargetForActorInternal(IProtocol* aActor,
-                                          nsIEventTarget* aEventTarget)
-{
-  Manager()->SetEventTargetForActorInternal(aActor, aEventTarget);
-}
-
 IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId, Side aSide)
  : IProtocol(aSide),
    mProtocolId(aProtoId),
    mOtherPid(mozilla::ipc::kInvalidProcessId),
-   mLastRouteId(aSide == ParentSide ? kFreedActorId : kNullActorId),
-   mLastShmemId(aSide == ParentSide ? kFreedActorId : kNullActorId),
-   mEventTargetMutex("ProtocolEventTargetMutex")
+   mLastRouteId(aSide == ParentSide ? 1 : 0),
+   mLastShmemId(aSide == ParentSide ? 1 : 0)
 {
 }
 
 IToplevelProtocol::~IToplevelProtocol()
 {
   if (mTrans) {
     RefPtr<DeleteTask<Transport>> task = new DeleteTask<Transport>(mTrans.release());
     XRE_GetIOMessageLoop()->PostTask(task.forget());
@@ -619,57 +593,39 @@ bool
 IToplevelProtocol::IsOnCxxStack() const
 {
   return GetIPCChannel()->IsOnCxxStack();
 }
 
 int32_t
 IToplevelProtocol::Register(IProtocol* aRouted)
 {
-  if (aRouted->Id() != kNullActorId && aRouted->Id() != kFreedActorId) {
-    // If there's already an ID, just return that.
-    return aRouted->Id();
-  }
   int32_t id = GetSide() == ParentSide ? ++mLastRouteId : --mLastRouteId;
   mActorMap.AddWithID(aRouted, id);
-  aRouted->SetId(id);
-
-  // Inherit our event target from our manager.
-  if (IProtocol* manager = aRouted->Manager()) {
-    MutexAutoLock lock(mEventTargetMutex);
-    if (nsCOMPtr<nsIEventTarget> target = mEventTargetMap.Lookup(manager->Id())) {
-      mEventTargetMap.AddWithID(target, id);
-    }
-  }
-
   return id;
 }
 
 int32_t
 IToplevelProtocol::RegisterID(IProtocol* aRouted,
                               int32_t aId)
 {
   mActorMap.AddWithID(aRouted, aId);
-  aRouted->SetId(aId);
   return aId;
 }
 
 IProtocol*
 IToplevelProtocol::Lookup(int32_t aId)
 {
   return mActorMap.Lookup(aId);
 }
 
 void
 IToplevelProtocol::Unregister(int32_t aId)
 {
-  mActorMap.Remove(aId);
-
-  MutexAutoLock lock(mEventTargetMutex);
-  mEventTargetMap.RemoveIfPresent(aId);
+  return mActorMap.Remove(aId);
 }
 
 Shmem::SharedMemory*
 IToplevelProtocol::CreateSharedMemory(size_t aSize,
                                       Shmem::SharedMemory::SharedMemoryType aType,
                                       bool aUnsafe,
                                       Shmem::id_t* aId)
 {
@@ -765,57 +721,10 @@ IToplevelProtocol::ShmemDestroyed(const 
   Shmem::SharedMemory* rawmem = LookupSharedMemory(id);
   if (rawmem) {
     mShmemMap.Remove(id);
     Shmem::Dealloc(Shmem::IHadBetterBeIPDLCodeCallingThis_OtherwiseIAmADoodyhead(), rawmem);
   }
   return true;
 }
 
-already_AddRefed<nsIEventTarget>
-IToplevelProtocol::GetMessageEventTarget(const Message& aMsg)
-{
-  int32_t route = aMsg.routing_id();
-
-  MutexAutoLock lock(mEventTargetMutex);
-  nsCOMPtr<nsIEventTarget> target = mEventTargetMap.Lookup(route);
-
-  if (aMsg.is_constructor()) {
-    ActorHandle handle;
-    PickleIterator iter = PickleIterator(aMsg);
-    if (!IPC::ReadParam(&aMsg, &iter, &handle)) {
-      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);
-    }
-
-    mEventTargetMap.AddWithID(target, handle.mId);
-  }
-
-  return target.forget();
-}
-
-void
-IToplevelProtocol::SetEventTargetForActorInternal(IProtocol* aActor,
-                                                  nsIEventTarget* aEventTarget)
-{
-  // 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.
-  int32_t id = Register(aActor);
-  aActor->SetId(id);
-
-  MutexAutoLock lock(mEventTargetMutex);
-  mEventTargetMap.AddWithID(aEventTarget, id);
-}
-
 } // namespace ipc
 } // namespace mozilla
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -11,17 +11,16 @@
 #include "base/id_map.h"
 #include "base/process.h"
 #include "base/process_util.h"
 #include "chrome/common/ipc_message_utils.h"
 
 #include "prenv.h"
 
 #include "IPCMessageStart.h"
-#include "mozilla/AlreadyAddRefed.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ipc/FileDescriptor.h"
 #include "mozilla/ipc/Shmem.h"
 #include "mozilla/ipc/Transport.h"
 #include "mozilla/ipc/MessageLink.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Mutex.h"
@@ -52,18 +51,16 @@ enum {
     GOODBYE_MESSAGE_TYPE       = kuint16max - 3,
     CANCEL_MESSAGE_TYPE        = kuint16max - 2,
 
     // kuint16max - 1 is used by ipc_channel.h.
 };
 
 } // namespace
 
-class nsIEventTarget;
-
 namespace mozilla {
 namespace dom {
 class ContentParent;
 } // namespace dom
 
 namespace net {
 class NeckoParent;
 } // namespace net
@@ -139,18 +136,16 @@ struct Trigger
 
 // What happens if Interrupt calls race?
 enum RacyInterruptPolicy {
     RIPError,
     RIPChildWins,
     RIPParentWins
 };
 
-class IToplevelProtocol;
-
 class IProtocol : public HasResultCodes
 {
 public:
     enum ActorDestroyReason {
         FailedConstructor,
         Deletion,
         AncestorDeletion,
         NormalShutdown,
@@ -196,36 +191,21 @@ public:
     IProtocol* Manager() const { return mManager; }
     virtual const MessageChannel* GetIPCChannel() const { return mChannel; }
     virtual MessageChannel* GetIPCChannel() { return mChannel; }
 
     bool AllocShmem(size_t aSize, Shmem::SharedMemory::SharedMemoryType aType, Shmem* aOutMem);
     bool AllocUnsafeShmem(size_t aSize, Shmem::SharedMemory::SharedMemoryType aType, Shmem* aOutMem);
     bool DeallocShmem(Shmem& aMem);
 
-    // Sets an event target to which all messages for aActor will be
-    // dispatched. This method must be called before right before the SendPFoo
-    // message for aActor is sent. And SendPFoo *must* be called if
-    // SetEventTargetForActor is called. The receiver when calling
-    // SetEventTargetForActor must be the actor that will be the manager for
-    // aActor.
-    void SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget);
-
 protected:
-    friend class IToplevelProtocol;
-
     void SetId(int32_t aId) { mId = aId; }
-    void SetManager(IProtocol* aManager);
+    void SetManager(IProtocol* aManager) { mManager = aManager; }
     void SetIPCChannel(MessageChannel* aChannel) { mChannel = aChannel; }
 
-    virtual void SetEventTargetForActorInternal(IProtocol* aActor, nsIEventTarget* aEventTarget);
-
-    static const int32_t kNullActorId = 0;
-    static const int32_t kFreedActorId = 1;
-
 private:
     int32_t mId;
     Side mSide;
     IProtocol* mManager;
     MessageChannel* mChannel;
 };
 
 typedef IPCMessageStart ProtocolId;
@@ -374,36 +354,24 @@ public:
     virtual void OnEnteredSyncSend() {
     }
     virtual void OnExitedSyncSend() {
     }
 
     virtual void ProcessRemoteNativeEventsInInterruptCall() {
     }
 
-    virtual already_AddRefed<nsIEventTarget>
-    GetMessageEventTarget(const Message& aMsg);
-
-protected:
-    virtual already_AddRefed<nsIEventTarget>
-    GetConstructedEventTarget(const Message& aMsg) { return nullptr; }
-
-    virtual void SetEventTargetForActorInternal(IProtocol* aActor, nsIEventTarget* aEventTarget);
-
-  private:
+private:
     ProtocolId mProtocolId;
     UniquePtr<Transport> mTrans;
     base::ProcessId mOtherPid;
     IDMap<IProtocol*> mActorMap;
     int32_t mLastRouteId;
     IDMap<Shmem::SharedMemory*> mShmemMap;
     Shmem::id_t mLastShmemId;
-
-    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;
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -4091,25 +4091,25 @@ class _GenerateProtocolActorCode(ipdl.as
 
     def ctorPrologue(self, md, errfn=ExprLiteral.NULL, idexpr=None):
         actordecl = md.actorDecl()
         actorvar = actordecl.var()
         actorproto = actordecl.ipdltype.protocol
         actortype = ipdl.type.ActorType(actorproto)
 
         if idexpr is None:
-            registerexpr = ExprCall(self.protocol.registerMethod(),
-                                    args=[ actorvar ])
+            idexpr = ExprCall(self.protocol.registerMethod(),
+                              args=[ actorvar ])
         else:
-            registerexpr = ExprCall(self.protocol.registerIDMethod(),
-                                    args=[ actorvar, idexpr ])
+            idexpr = ExprCall(self.protocol.registerIDMethod(),
+                              args=[ actorvar, idexpr ])
 
         return [
             self.failIfNullActor(actorvar, errfn, msg="Error constructing actor %s" % actortype.name() + self.side.capitalize()),
-            StmtExpr(ExprCall(registerexpr)),
+            StmtExpr(ExprCall(ExprSelect(actorvar, '->', 'SetId'), args=[idexpr])),
             StmtExpr(ExprCall(ExprSelect(actorvar, '->', 'SetManager'), args=[ExprVar.THIS])),
             StmtExpr(ExprCall(ExprSelect(actorvar, '->', 'SetIPCChannel'),
                               args=[self.protocol.callGetChannel()])),
             StmtExpr(_callInsertManagedActor(
                 self.protocol.managedVar(md.decl.type.constructedType(),
                                          self.side),
                 actorvar)),
             StmtExpr(ExprAssn(_actorState(actorvar),
@@ -4206,24 +4206,21 @@ class _GenerateProtocolActorCode(ipdl.as
 
         return method
 
     def destroyActor(self, md, actorexpr, why=_DestroyReason.Deletion):
         if md.decl.type.isCtor():
             destroyedType = md.decl.type.constructedType()
         else:
             destroyedType = self.protocol.decl.type
-        managervar = ExprVar('mgr')
-        return ([ StmtDecl(Decl(Type('IProtocol', ptr=1), managervar.name),
-                           init=self.protocol.managerVar(actorexpr)),
-                  StmtExpr(self.callActorDestroy(actorexpr, why)),
+        return ([ StmtExpr(self.callActorDestroy(actorexpr, why)),
                   StmtExpr(self.callDeallocSubtree(md, actorexpr)),
                   StmtExpr(self.callRemoveActor(
                       actorexpr,
-                      manager=managervar,
+                      manager=self.protocol.managerVar(actorexpr),
                       ipdltype=destroyedType))
                 ])
 
     def dtorPrologue(self, actorexpr):
         return [ self.failIfNullActor(actorexpr), Whitespace.NL ]
 
     def dtorEpilogue(self, md, actorexpr):
         return self.destroyActor(md, actorexpr)
@@ -4356,17 +4353,18 @@ class _GenerateProtocolActorCode(ipdl.as
         failif = StmtIf(ExprNot(actorExpr))
         if msg:
             failif.addifstmt(_printWarningMessage(msg))
         failif.addifstmt(StmtReturn(retOnNull))
         return failif
 
     def unregisterActor(self):
         return [ StmtExpr(ExprCall(self.protocol.unregisterMethod(),
-                                   args=[ _actorId() ])) ]
+                                   args=[ _actorId() ])),
+                 StmtExpr(ExprCall(ExprVar('SetId'), args=[_FREED_ACTOR_ID])) ]
 
     def makeMessage(self, md, errfn, fromActor=None):
         msgvar = self.msgvar
         routingId = self.protocol.routingId(fromActor)
         this = None
         if md.decl.type.isDtor():  this = md.actorDecl().var()
 
         stmts = ([ StmtDecl(Decl(Type('IPC::Message', ptr=1), msgvar.name),