Bug 1451363 - part 6 - move GetIPCChannel into ProtocolState; r=mccr8
authorNathan Froyd <froydnj@mozilla.com>
Mon, 23 Apr 2018 14:13:36 -0400
changeset 415152 ce3f196660370950dc20fa5b9ffcd7c18d2c12dc
parent 415151 52f24d629711a20bf2f9717a41ae406bc6489f94
child 415153 05476b05d16802c6032b7d3f6984e89150fceba8
push id33889
push useraciure@mozilla.com
push dateTue, 24 Apr 2018 01:14:50 +0000
treeherdermozilla-central@b35a1f66c452 [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 6 - move GetIPCChannel into ProtocolState; r=mccr8 We can move this information into ProtocolState and save having two virtual functions for every protocol. Moving some bits out of the codegen'd IPC code is a nice bonus, though we keep the strange setup where toplevel protocols have two mChannel member variables.
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
ipc/ipdl/ipdl/lower.py
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -442,16 +442,28 @@ IProtocol::ManagedState::IsTrackingShare
 }
 
 bool
 IProtocol::ManagedState::DestroySharedMemory(Shmem& aShmem)
 {
   return mProtocol->Manager()->DestroySharedMemory(aShmem);
 }
 
+const MessageChannel*
+IProtocol::ManagedState::GetIPCChannel() const
+{
+  return mChannel;
+}
+
+MessageChannel*
+IProtocol::ManagedState::GetIPCChannel()
+{
+  return mChannel;
+}
+
 ProcessId
 IProtocol::OtherPid() const
 {
   return Manager()->OtherPid();
 }
 
 void
 IProtocol::FatalError(const char* const aErrorMsg) const
@@ -529,29 +541,29 @@ void
 IProtocol::SetManagerAndRegister(IProtocol* aManager)
 {
   // Set the manager prior to registering so registering properly inherits
   // the manager's event target.
   SetManager(aManager);
 
   aManager->Register(this);
 
-  SetIPCChannel(aManager->GetIPCChannel());
+  mState->SetIPCChannel(aManager->GetIPCChannel());
 }
 
 void
 IProtocol::SetManagerAndRegister(IProtocol* aManager, int32_t aId)
 {
   // Set the manager prior to registering so registering properly inherits
   // the manager's event target.
   SetManager(aManager);
 
   aManager->RegisterID(this, aId);
 
-  SetIPCChannel(aManager->GetIPCChannel());
+  mState->SetIPCChannel(aManager->GetIPCChannel());
 }
 
 void
 IProtocol::SetEventTargetForActor(IProtocol* aActor, nsIEventTarget* aEventTarget)
 {
   // Make sure we have a manager for the internal method to access.
   aActor->SetManager(this);
   mState->SetEventTargetForActor(aActor, aEventTarget);
@@ -605,18 +617,18 @@ IProtocol::ManagedState::ReplaceEventTar
 }
 
 already_AddRefed<nsIEventTarget>
 IProtocol::ManagedState::GetActorEventTarget(IProtocol* aActor)
 {
   return mProtocol->Manager()->GetActorEventTarget(aActor);
 }
 
-IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId, Side aSide)
- : IProtocol(aSide, MakeUnique<ToplevelState>(this, aSide)),
+IToplevelProtocol::IToplevelProtocol(const char* aName, ProtocolId aProtoId, Side aSide)
+ : IProtocol(aSide, MakeUnique<ToplevelState>(aName, this, aSide)),
    mMonitor("mozilla.ipc.IToplevelProtocol.mMonitor"),
    mProtocolId(aProtoId),
    mOtherPid(mozilla::ipc::kInvalidProcessId),
    mOtherPidState(ProcessIdState::eUnstarted)
 {
 }
 
 IToplevelProtocol::~IToplevelProtocol()
@@ -766,21 +778,25 @@ void
 IToplevelProtocol::ToplevelState::Unregister(int32_t aId)
 {
   mActorMap.Remove(aId);
 
   MutexAutoLock lock(mEventTargetMutex);
   mEventTargetMap.RemoveIfPresent(aId);
 }
 
-IToplevelProtocol::ToplevelState::ToplevelState(IToplevelProtocol* aProtocol, Side aSide)
-  : mProtocol(aProtocol)
+IToplevelProtocol::ToplevelState::ToplevelState(const char* aName,
+                                                IToplevelProtocol* aProtocol,
+                                                Side aSide)
+  : ProtocolState()
+  , mProtocol(aProtocol)
   , mLastRouteId(aSide == ParentSide ? kFreedActorId : kNullActorId)
   , mLastShmemId(aSide == ParentSide ? kFreedActorId : kNullActorId)
   , mEventTargetMutex("ProtocolEventTargetMutex")
+  , mChannel(aName, aProtocol)
 {
 }
 
 Shmem::SharedMemory*
 IToplevelProtocol::ToplevelState::CreateSharedMemory(size_t aSize,
                                                      Shmem::SharedMemory::SharedMemoryType aType,
                                                      bool aUnsafe,
                                                      Shmem::id_t* aId)
@@ -990,10 +1006,22 @@ IToplevelProtocol::ToplevelState::Replac
   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);
 }
 
+const MessageChannel*
+IToplevelProtocol::ToplevelState::GetIPCChannel() const
+{
+  return &mChannel;
+}
+
+MessageChannel*
+IToplevelProtocol::ToplevelState::GetIPCChannel()
+{
+  return &mChannel;
+}
+
 } // namespace ipc
 } // namespace mozilla
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -15,16 +15,17 @@
 
 #include "prenv.h"
 
 #include "IPCMessageStart.h"
 #include "mozilla/AlreadyAddRefed.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ipc/ByteBuf.h"
 #include "mozilla/ipc/FileDescriptor.h"
+#include "mozilla/ipc/MessageChannel.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/MozPromise.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/NotNull.h"
@@ -154,16 +155,17 @@ public:
     // This class (and its two subclasses) ensure that we don't have a bunch
     // of redundant entries in protocol vtables: we have a single vtable per
     // subclass, and then each protocol has its own instance of one of the
     // subclasses.  This setup makes things a bit slower, but the space
     // savings are worth it.
     class ProtocolState
     {
     public:
+        ProtocolState() : mChannel(nullptr) {}
         virtual ~ProtocolState() = default;
 
         // Shared memory functions.
         virtual Shmem::SharedMemory* CreateSharedMemory(
             size_t, SharedMemory::SharedMemoryType, bool, int32_t*) = 0;
         virtual Shmem::SharedMemory* LookupSharedMemory(int32_t) = 0;
         virtual bool IsTrackingSharedMemory(Shmem::SharedMemory*) = 0;
         virtual bool DestroySharedMemory(Shmem&) = 0;
@@ -177,25 +179,39 @@ public:
         // 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;
+
+        virtual const MessageChannel* GetIPCChannel() const = 0;
+        virtual MessageChannel* GetIPCChannel() = 0;
+
+        // XXX we have this weird setup where ProtocolState has an mChannel
+        // member, but it (probably?) only gets set for protocols that have
+        // a manager.  That is, for toplevel protocols, this member is dead
+        // weight and should be removed, since toplevel protocols maintain
+        // their own channel.
+        void SetIPCChannel(MessageChannel* aChannel) { mChannel = aChannel; }
+
+    protected:
+        MessageChannel* mChannel;
     };
 
     // Managed protocols just forward all of their operations to the topmost
     // managing protocol.
     class ManagedState final : public ProtocolState
     {
     public:
         explicit ManagedState(IProtocol* aProtocol)
-            : mProtocol(aProtocol)
+            : ProtocolState()
+            , mProtocol(aProtocol)
         {}
 
         Shmem::SharedMemory* CreateSharedMemory(
             size_t, SharedMemory::SharedMemoryType, bool, int32_t*) override;
         Shmem::SharedMemory* LookupSharedMemory(int32_t) override;
         bool IsTrackingSharedMemory(Shmem::SharedMemory*) override;
         bool DestroySharedMemory(Shmem&) override;
 
@@ -204,16 +220,19 @@ public:
         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;
 
+        const MessageChannel* GetIPCChannel() const override;
+        MessageChannel* GetIPCChannel() override;
+
     private:
         IProtocol* const mProtocol;
     };
 
     typedef base::ProcessId ProcessId;
     typedef IPC::Message Message;
     typedef IPC::MessageInfo MessageInfo;
 
@@ -253,16 +272,25 @@ public:
     {
         return mState->IsTrackingSharedMemory(aSegment);
     }
     bool DestroySharedMemory(Shmem& aShmem)
     {
         return mState->DestroySharedMemory(aShmem);
     }
 
+    MessageChannel* GetIPCChannel()
+    {
+        return mState->GetIPCChannel();
+    }
+    const MessageChannel* GetIPCChannel() const
+    {
+        return mState->GetIPCChannel();
+    }
+
     // XXX odd ducks, acknowledged
     virtual ProcessId OtherPid() const;
     Side GetSide() const { return mSide; }
 
     void FatalError(const char* const aErrorMsg) const;
     virtual void HandleFatalError(const char* aErrorMsg) const;
 
     Maybe<IProtocol*> ReadActor(const IPC::Message* aMessage, PickleIterator* aIter, bool aNullable,
@@ -271,18 +299,16 @@ public:
     virtual Result OnMessageReceived(const Message& aMessage) = 0;
     virtual Result OnMessageReceived(const Message& aMessage, Message *& aReply) = 0;
     virtual Result OnCallReceived(const Message& aMessage, Message *& aReply) = 0;
 
     virtual int32_t GetProtocolTypeId() = 0;
 
     int32_t Id() const { return mId; }
     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
@@ -300,17 +326,16 @@ public:
     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))
     {}
 
     friend class IToplevelProtocol;
 
     void SetId(int32_t aId) { mId = aId; }
     void ResetManager() { mManager = nullptr; }
     // We have separate functions because the accessibility code manually
@@ -318,26 +343,23 @@ protected:
     void SetManager(IProtocol* aManager);
 
     // Sets the manager for the protocol and registers the protocol with
     // its manager, setting up channels for the protocol as well.  Not
     // for use outside of IPDL.
     void SetManagerAndRegister(IProtocol* aManager);
     void SetManagerAndRegister(IProtocol* aManager, int32_t aId);
 
-    void SetIPCChannel(MessageChannel* aChannel) { mChannel = aChannel; }
-
     static const int32_t kNullActorId = 0;
     static const int32_t kFreedActorId = 1;
 
 private:
     int32_t mId;
     Side mSide;
     IProtocol* mManager;
-    MessageChannel* mChannel;
     UniquePtr<ProtocolState> mState;
 };
 
 typedef IPCMessageStart ProtocolId;
 
 #define IPC_OK() mozilla::ipc::IPCResult::Ok()
 #define IPC_FAIL(actor, why) mozilla::ipc::IPCResult::Fail(WrapNotNull(actor), __func__, (why))
 #define IPC_FAIL_NO_REASON(actor) mozilla::ipc::IPCResult::Fail(WrapNotNull(actor), __func__)
@@ -367,31 +389,32 @@ class Endpoint;
  * IToplevelProtocol tracks all top-level protocol actors created from
  * this protocol actor.
  */
 class IToplevelProtocol : public IProtocol
 {
     template<class PFooSide> friend class Endpoint;
 
 protected:
-    explicit IToplevelProtocol(ProtocolId aProtoId, Side aSide);
+    explicit IToplevelProtocol(const char* aName, ProtocolId aProtoId,
+                               Side aSide);
     ~IToplevelProtocol();
 
 public:
     enum ProcessIdState {
         eUnstarted,
         ePending,
         eReady,
         eError
     };
 
     class ToplevelState final : public ProtocolState
     {
     public:
-        ToplevelState(IToplevelProtocol* aProtocol, Side aSide);
+        ToplevelState(const char* aName, IToplevelProtocol* aProtocol, Side aSide);
 
         Shmem::SharedMemory* CreateSharedMemory(
             size_t, SharedMemory::SharedMemoryType, bool, int32_t*) override;
         Shmem::SharedMemory* LookupSharedMemory(int32_t) override;
         bool IsTrackingSharedMemory(Shmem::SharedMemory*) override;
         bool DestroySharedMemory(Shmem&) override;
 
         void DeallocShmems();
@@ -407,25 +430,30 @@ public:
         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);
 
+        const MessageChannel* GetIPCChannel() const override;
+        MessageChannel* GetIPCChannel() override;
+
     private:
         IToplevelProtocol* const mProtocol;
         IDMap<IProtocol*> mActorMap;
         int32_t mLastRouteId;
         IDMap<Shmem::SharedMemory*> mShmemMap;
         Shmem::id_t mLastShmemId;
 
         Mutex mEventTargetMutex;
         IDMap<nsCOMPtr<nsIEventTarget>> mEventTargetMap;
+
+        MessageChannel mChannel;
     };
 
     using SchedulerGroupSet = nsILabelableRunnable::SchedulerGroupSet;
 
     void SetTransport(UniquePtr<Transport> aTrans)
     {
         mTrans = Move(aTrans);
     }
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -3235,23 +3235,20 @@ class _GenerateProtocolActorCode(ipdl.as
             + [ Whitespace.NL ]
         ))
 
         self.cls.addstmt(Label.PUBLIC)
         # Actor()
         ctor = ConstructorDefn(ConstructorDecl(self.clsname))
         side = ExprVar('mozilla::ipc::' + self.side.title() + 'Side')
         if ptype.isToplevel():
+            name = ExprLiteral.String(_actorName(p.name, self.side))
             ctor.memberinits = [
                 ExprMemberInit(ExprVar('mozilla::ipc::IToplevelProtocol'),
-                               [_protocolId(ptype), side]),
-                ExprMemberInit(p.channelVar(), [
-                    ExprLiteral.String(_actorName(p.name, self.side)),
-                    ExprCall(ExprVar('ALLOW_THIS_IN_INITIALIZER_LIST'),
-                             [ ExprVar.THIS ]) ]),
+                               [name, _protocolId(ptype), side]),
                 ExprMemberInit(p.stateVar(),
                                [ p.startState() ])
             ]
         else:
             ctor.memberinits = [
                 ExprMemberInit(ExprVar('mozilla::ipc::IProtocol'), [side]),
                 ExprMemberInit(p.stateVar(),
                                [ p.deadState() ])
@@ -3479,18 +3476,21 @@ class _GenerateProtocolActorCode(ipdl.as
         if (ptype.isToplevel() and ptype.isInterrupt()):
 
             processnative = MethodDefn(
                 MethodDecl('ProcessNativeEventsInInterruptCall', ret=Type.VOID))
 
             processnative.addstmts([
                     CppDirective('ifdef', 'OS_WIN'),
                     StmtExpr(ExprCall(
-                            ExprSelect(p.channelVar(), '.',
-                                       'ProcessNativeEventsInInterruptCall'))),
+                        ExprSelect(ExprCall(ExprSelect(ExprCall(ExprVar('DowncastState')),
+                                                       '->',
+                                                       'GetIPCChannel')),
+                                   '->',
+                                   'ProcessNativeEventsInInterruptCall'))),
                     CppDirective('else'),
                     _fatalError('This method is Windows-only'),
                     CppDirective('endif'),
                     ])
 
             self.cls.addstmts([ processnative, Whitespace.NL ])
 
         ## private methods
@@ -3614,19 +3614,16 @@ class _GenerateProtocolActorCode(ipdl.as
         # we're toplevel
         self.cls.addstmts([ deallocsubtree, Whitespace.NL ])
 
         if ptype.isToplevel():
             deallocself = MethodDefn(MethodDecl(deallocselfvar.name, methodspec=MethodSpec.VIRTUAL))
             self.cls.addstmts([ deallocself, Whitespace.NL ])
 
         ## private members
-        if ptype.isToplevel():
-            self.cls.addstmt(StmtDecl(Decl(p.channelType(), 'mChannel')))
-
         self.cls.addstmt(StmtDecl(Decl(Type('State'), p.stateVar().name)))
 
         for managed in ptype.manages:
             self.cls.addstmts([
                 StmtDecl(Decl(
                     p.managedVarType(managed, self.side),
                     p.managedVar(managed, self.side).name)) ])
 
@@ -3643,32 +3640,16 @@ class _GenerateProtocolActorCode(ipdl.as
         sourcevar = ExprVar('aSource')
         ivar = ExprVar('i')
         kidsvar = ExprVar('kids')
         ithkid = ExprIndex(kidsvar, ivar)
 
         methods = []
 
         if p.decl.type.isToplevel():
-            getchannel = MethodDefn(MethodDecl(
-                p.getChannelMethod().name,
-                ret=Type('MessageChannel', ptr=1),
-                methodspec=MethodSpec.OVERRIDE))
-            getchannel.addstmt(StmtReturn(ExprAddrOf(p.channelVar())))
-
-            getchannelconst = MethodDefn(MethodDecl(
-                p.getChannelMethod().name,
-                ret=Type('MessageChannel', ptr=1, const=1),
-                methodspec=MethodSpec.OVERRIDE, const=1))
-            getchannelconst.addstmt(StmtReturn(ExprAddrOf(p.channelVar())))
-
-            methods += [ getchannel,
-                         getchannelconst ]
-
-        if p.decl.type.isToplevel():
             tmpvar = ExprVar('tmp')
 
             # "private" message that passes shmem mappings from one process
             # to the other
             if p.subtreeUsesShmem():
                 self.asyncSwitch.addcase(
                     CaseLabel('SHMEM_CREATED_MESSAGE_TYPE'),
                     self.genShmemCreatedHandler())