Bug 792652 - Simplify IPDL type hierarchy (r=dvander)
☠☠ backed out by d2098e1d1f2d ☠ ☠
authorBill McCloskey <billm@mozilla.com>
Fri, 28 Oct 2016 21:26:21 -0700
changeset 348412 004cd692ba6d4ae50ad7b082d67745760e9213c6
parent 348411 92e7fee81fa21b99f7774443e4b48f42101bdd66
child 348413 1829d53588088806844de65624363b71453a2ad3
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs792652
milestone52.0a1
Bug 792652 - Simplify IPDL type hierarchy (r=dvander) Currently all our protocols inherit from IProtocolManager<IProtocol>. I have no idea why. This patch switches everything over to IProtocol, without any templates. I had to move ReadActor to the .cpp file to avoid redefinition errors.
dom/media/ipc/VideoDecoderManagerParent.h
dom/media/systemservices/MediaParent.h
gfx/layers/IPDLActor.h
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
ipc/ipdl/ipdl/lower.py
js/ipc/WrapperOwner.h
--- a/dom/media/ipc/VideoDecoderManagerParent.h
+++ b/dom/media/ipc/VideoDecoderManagerParent.h
@@ -28,17 +28,17 @@ public:
 
 protected:
   PVideoDecoderParent* AllocPVideoDecoderParent() override;
   bool DeallocPVideoDecoderParent(PVideoDecoderParent* actor) override;
 
   bool RecvReadback(const SurfaceDescriptorGPUVideo& aSD, SurfaceDescriptor* aResult) override;
   bool RecvDeallocateSurfaceDescriptorGPUVideo(const SurfaceDescriptorGPUVideo& aSD) override;
 
-  void ActorDestroy(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) override {}
+  void ActorDestroy(mozilla::ipc::IProtocol::ActorDestroyReason) override {}
 
   void DeallocPVideoDecoderManagerParent() override;
 
  private:
   VideoDecoderManagerParent();
   ~VideoDecoderManagerParent();
 
   void Open(Endpoint<PVideoDecoderManagerParent>&& aEndpoint);
--- a/dom/media/systemservices/MediaParent.h
+++ b/dom/media/systemservices/MediaParent.h
@@ -17,17 +17,17 @@ namespace media {
 
 // media::Parent implements the chrome-process side of ipc for media::Child APIs
 // A same-process version may also be created to service non-e10s calls.
 
 class OriginKeyStore;
 
 class NonE10s
 {
-  typedef mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason
+  typedef mozilla::ipc::IProtocol::ActorDestroyReason
       ActorDestroyReason;
 public:
   virtual ~NonE10s() {}
 protected:
   virtual bool RecvGetOriginKey(const uint32_t& aRequestId,
                                 const nsCString& aOrigin,
                                 const bool& aPrivateBrowsing,
                                 const bool& aPersist) = 0;
@@ -40,17 +40,17 @@ protected:
                                 nsCString aKey);
 };
 
 // Super = PMediaParent or NonE10s
 
 template<class Super>
 class Parent : public Super
 {
-  typedef mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason
+  typedef mozilla::ipc::IProtocol::ActorDestroyReason
       ActorDestroyReason;
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(Parent<Super>)
 
   virtual bool RecvGetOriginKey(const uint32_t& aRequestId,
                                 const nsCString& aOrigin,
                                 const bool& aPrivateBrowsing,
                                 const bool& aPersist) override;
--- a/gfx/layers/IPDLActor.h
+++ b/gfx/layers/IPDLActor.h
@@ -33,17 +33,17 @@ public:
 
   virtual bool RecvDestroy() override
   {
     DestroyIfNeeded();
     Unused << Protocol::Send__delete__(this);
     return true;
   }
 
-  typedef ipc::IProtocolManager<ipc::IProtocol>::ActorDestroyReason Why;
+  typedef ipc::IProtocol::ActorDestroyReason Why;
 
   virtual void ActorDestroy(Why) override {
     DestroyIfNeeded();
   }
 
 protected:
   void DestroyIfNeeded() {
     if (!mDestroyed) {
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -360,10 +360,43 @@ TableToArray(const nsTHashtable<nsPtrHas
   uint32_t i = 0;
   void** elements = aArray.AppendElements(aTable.Count());
   for (auto iter = aTable.ConstIter(); !iter.Done(); iter.Next()) {
     elements[i] = iter.Get()->GetKey();
     ++i;
   }
 }
 
+Maybe<IProtocol*>
+IProtocol::ReadActor(const IPC::Message* aMessage, PickleIterator* aIter, bool aNullable,
+                     const char* aActorDescription, int32_t aProtocolTypeId)
+{
+    int32_t id;
+    if (!IPC::ReadParam(aMessage, aIter, &id)) {
+        ActorIdReadError(aActorDescription);
+        return Nothing();
+    }
+
+    if (id == 1 || (id == 0 && !aNullable)) {
+        BadActorIdError(aActorDescription);
+        return Nothing();
+    }
+
+    if (id == 0) {
+        return Some(static_cast<IProtocol*>(nullptr));
+    }
+
+    IProtocol* listener = this->Lookup(id);
+    if (!listener) {
+        ActorLookupError(aActorDescription);
+        return Nothing();
+    }
+
+    if (listener->GetProtocolTypeId() != aProtocolTypeId) {
+        MismatchedActorTypeError(aActorDescription);
+        return Nothing();
+    }
+
+    return Some(listener);
+}
+
 } // namespace ipc
 } // namespace mozilla
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -125,61 +125,53 @@ struct Trigger
     {
       MOZ_ASSERT(0 <= msg && msg < INT32_MAX);
     }
 
     uint32_t mAction : 1;
     uint32_t mMessage : 31;
 };
 
-template<class ListenerT>
-class IProtocolManager
+class IProtocol : public MessageListener
 {
 public:
     enum ActorDestroyReason {
         FailedConstructor,
         Deletion,
         AncestorDeletion,
         NormalShutdown,
         AbnormalShutdown
     };
 
     typedef base::ProcessId ProcessId;
 
-    virtual int32_t Register(ListenerT*) = 0;
-    virtual int32_t RegisterID(ListenerT*, int32_t) = 0;
-    virtual ListenerT* Lookup(int32_t) = 0;
+    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;
-    virtual void RemoveManagee(int32_t, ListenerT*) = 0;
+    virtual void RemoveManagee(int32_t, IProtocol*) = 0;
 
     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;
 
     // XXX odd ducks, acknowledged
     virtual ProcessId OtherPid() const = 0;
     virtual MessageChannel* GetIPCChannel() = 0;
 
     virtual void FatalError(const char* const aProtocolName, const char* const aErrorMsg) const = 0;
 
-    Maybe<ListenerT*> ReadActor(const IPC::Message* aMessage, PickleIterator* aIter, bool aNullable,
+    Maybe<IProtocol*> ReadActor(const IPC::Message* aMessage, PickleIterator* aIter, bool aNullable,
                                 const char* aActorDescription, int32_t aProtocolTypeId);
 };
 
 typedef IPCMessageStart ProtocolId;
 
-/**
- * All RPC protocols should implement this interface.
- */
-class IProtocol : public MessageListener
-{
-};
-
 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.
@@ -314,50 +306,16 @@ Open(const PrivateIPDLInterface&,
      MessageChannel*, base::ProcessId, Transport::Mode,
      ProtocolId, ProtocolId);
 
 bool
 UnpackChannelOpened(const PrivateIPDLInterface&,
                     const IPC::Message&,
                     TransportDescriptor*, base::ProcessId*, ProtocolId*);
 
-template<typename ListenerT>
-Maybe<ListenerT*>
-IProtocolManager<ListenerT>::ReadActor(const IPC::Message* aMessage, PickleIterator* aIter, bool aNullable,
-                                       const char* aActorDescription, int32_t aProtocolTypeId)
-{
-    int32_t id;
-    if (!IPC::ReadParam(aMessage, aIter, &id)) {
-        ActorIdReadError(aActorDescription);
-        return Nothing();
-    }
-
-    if (id == 1 || (id == 0 && !aNullable)) {
-        BadActorIdError(aActorDescription);
-        return Nothing();
-    }
-
-    if (id == 0) {
-        return Some(static_cast<ListenerT*>(nullptr));
-    }
-
-    ListenerT* listener = this->Lookup(id);
-    if (!listener) {
-        ActorLookupError(aActorDescription);
-        return Nothing();
-    }
-
-    if (static_cast<MessageListener*>(listener)->GetProtocolTypeId() != aProtocolTypeId) {
-        MismatchedActorTypeError(aActorDescription);
-        return Nothing();
-    }
-
-    return Some(listener);
-}
-
 #if defined(XP_WIN)
 // This is a restricted version of Windows' DuplicateHandle() function
 // that works inside the sandbox and can send handles but not retrieve
 // them.  Unlike DuplicateHandle(), it takes a process ID rather than
 // a process handle.  It returns true on success, false otherwise.
 bool
 DuplicateHandle(HANDLE aSourceHandle,
                 DWORD aTargetProcessId,
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -1088,23 +1088,18 @@ class Protocol(ipdl.ast.Protocol):
         return Type('Channel', ptr=not self.decl.type.isToplevel())
 
     def channelHeaderFile(self):
         return '/'.join(_semsToChannelParts(self.sendSems())) +'.h'
 
     def fqListenerName(self):
       return 'mozilla::ipc::MessageListener'
 
-    def fqBaseClass(self):
-        return 'mozilla::ipc::IProtocol'
-
     def managerInterfaceType(self, ptr=0):
-        return Type('mozilla::ipc::IProtocolManager',
-                    ptr=ptr,
-                    T=Type(self.fqBaseClass()))
+        return Type('mozilla::ipc::IProtocol', ptr=ptr)
 
     def openedProtocolInterfaceType(self, ptr=0):
         return Type('mozilla::ipc::IToplevelProtocol',
                     ptr=ptr)
 
     def _ipdlmgrtype(self):
         assert 1 == len(self.decl.type.managers)
         for mgr in self.decl.type.managers:  return mgr
@@ -2620,17 +2615,17 @@ class _GenerateProtocolActorCode(ipdl.as
     def lower(self, tu, clsname, cxxHeaderFile, cxxFile):
         self.clsname = clsname
         self.hdrfile = cxxHeaderFile
         self.cppfile = cxxFile
         tu.accept(self)
 
     def standardTypedefs(self):
         return [
-            Typedef(Type(self.protocol.fqBaseClass()), 'ProtocolBase'),
+            Typedef(Type('mozilla::ipc::IProtocol'), 'ProtocolBase'),
             Typedef(Type('IPC::Message'), 'Message'),
             Typedef(Type(self.protocol.channelName()), 'Channel'),
             Typedef(Type(self.protocol.fqListenerName()), 'ChannelListener'),
             Typedef(Type('base::ProcessHandle'), 'ProcessHandle'),
             Typedef(Type('mozilla::ipc::MessageChannel'), 'MessageChannel'),
             Typedef(Type('mozilla::ipc::SharedMemory'), 'SharedMemory'),
             Typedef(Type('mozilla::ipc::Trigger'), 'Trigger'),
         ]
@@ -2821,18 +2816,17 @@ class _GenerateProtocolActorCode(ipdl.as
         if ptype.isToplevel() and self.side is 'parent':
             self.hdrfile.addthings([
                     _makeForwardDeclForQClass('nsIFile', []),
                     Whitespace.NL
                     ])
 
         self.cls = Class(
             self.clsname,
-            inherits=[ Inherit(Type(p.fqBaseClass()), viz='public'),
-                       Inherit(p.managerInterfaceType(), viz='protected') ] +
+            inherits=[ Inherit(p.managerInterfaceType(), viz='public') ] +
             optinherits,
             abstract=True)
 
         bridgeActorsCreated = ProcessGraph.bridgeEndpointsOf(ptype, self.side)
         opensActorsCreated = ProcessGraph.opensEndpointsOf(ptype, self.side)
         channelOpenedActors = OrderedDict.fromkeys(bridgeActorsCreated + opensActorsCreated, None)
 
         friends = _FindFriends().findFriends(ptype)
--- a/js/ipc/WrapperOwner.h
+++ b/js/ipc/WrapperOwner.h
@@ -15,18 +15,17 @@
 #include "js/Proxy.h"
 
 namespace mozilla {
 namespace jsipc {
 
 class WrapperOwner : public virtual JavaScriptShared
 {
   public:
-    typedef mozilla::ipc::IProtocolManager<
-                       mozilla::ipc::IProtocol>::ActorDestroyReason
+    typedef mozilla::ipc::IProtocol::ActorDestroyReason
            ActorDestroyReason;
 
     WrapperOwner();
     bool init();
 
     // Standard internal methods.
     // (The traps should be in the same order like js/Proxy.h)
     bool getOwnPropertyDescriptor(JSContext* cx, JS::HandleObject proxy, JS::HandleId id,