Bug 792652 - Remove methods from MessageListener (r=dvander)

With this change, MessageChannel stores mListener as an IToplevelProtocol
rather than a MessageListener (which isn't really a useful concept on
its own). The MessageListener methods are split out to IProtocol and
IToplevelProtocol. MessageListener gets deleted. Some of the inline
functions in MessageChannel had to be moved to MessageChannel.cpp since
IToplevelProtocol isn't defined in MessageChannel.h.

dom/plugins/ipc/PluginMessageUtils.h

@@ -10,8 +10,9 @@
 #include "ipc/IPCMessageUtils.h"
 #include "base/message_loop.h"
 
-#include "mozilla/ipc/MessageChannel.h"
 #include "mozilla/ipc/CrossProcessMutex.h"
+#include "mozilla/ipc/MessageChannel.h"
+#include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/UniquePtr.h"
 #include "gfxipc/ShadowLayerUtils.h"
 

dom/plugins/ipc/PluginModuleParent.cpp

@@ -1036,8 +1036,7 @@ PluginInstanceParent*
 PluginModuleChromeParent::GetManagingInstance(mozilla::ipc::IProtocol* aProtocol)
 {
     MOZ_ASSERT(aProtocol);
-    mozilla::ipc::MessageListener* listener =
-        static_cast<mozilla::ipc::MessageListener*>(aProtocol);
+    mozilla::ipc::IProtocol* listener = aProtocol;
     switch (listener->GetProtocolTypeId()) {
         case PPluginInstanceMsgStart:
             // In this case, aProtocol is the instance itself. Just cast it.

ipc/glue/MessageChannel.cpp

@@ -472,7 +472,7 @@ private:
     nsAutoPtr<IPC::Message> mReply;
 };
 
-MessageChannel::MessageChannel(MessageListener *aListener)
+MessageChannel::MessageChannel(IToplevelProtocol *aListener)
   : mListener(aListener),
     mChannelState(ChannelClosed),
     mSide(UnknownSide),
@@ -1856,6 +1856,12 @@ MessageChannel::MaybeUndeferIncall()
     task->Post();
 }
 
+void
+MessageChannel::EnteredCxxStack()
+{
+    mListener->OnEnteredCxxStack();
+}
+
 void
 MessageChannel::ExitedCxxStack()
 {
@@ -1868,6 +1874,30 @@ MessageChannel::ExitedCxxStack()
     }
 }
 
+void
+MessageChannel::EnteredCall()
+{
+    mListener->OnEnteredCall();
+}
+
+void
+MessageChannel::ExitedCall()
+{
+    mListener->OnExitedCall();
+}
+
+void
+MessageChannel::EnteredSyncSend()
+{
+    mListener->OnEnteredSyncSend();
+}
+
+void
+MessageChannel::ExitedSyncSend()
+{
+    mListener->OnExitedSyncSend();
+}
+
 void
 MessageChannel::EnqueuePendingMessages()
 {

ipc/glue/MessageChannel.h

@@ -33,6 +33,7 @@ namespace mozilla {
 namespace ipc {
 
 class MessageChannel;
+class IToplevelProtocol;
 
 class RefCountedMonitor : public Monitor
 {
@@ -60,6 +61,15 @@ enum class SyncSendError {
     ReplyError,
 };
 
+enum ChannelState {
+    ChannelClosed,
+    ChannelOpening,
+    ChannelConnected,
+    ChannelTimeout,
+    ChannelClosing,
+    ChannelError
+};
+
 class AutoEnterTransaction;
 
 class MessageChannel : HasResultCodes
@@ -79,7 +89,7 @@ class MessageChannel : HasResultCodes
     typedef IPC::MessageInfo MessageInfo;
     typedef mozilla::ipc::Transport Transport;
 
-    explicit MessageChannel(MessageListener *aListener);
+    explicit MessageChannel(IToplevelProtocol *aListener);
     ~MessageChannel();
 
     // "Open" from the perspective of the transport layer; the underlying
@@ -328,29 +338,16 @@ class MessageChannel : HasResultCodes
     // This helper class manages mCxxStackDepth on behalf of MessageChannel.
     // When the stack depth is incremented from zero to non-zero, it invokes
     // a callback, and similarly for when the depth goes from non-zero to zero.
-    void EnteredCxxStack() {
-       mListener->OnEnteredCxxStack();
-    }
-
+    void EnteredCxxStack();
     void ExitedCxxStack();
 
-    void EnteredCall() {
-        mListener->OnEnteredCall();
-    }
+    void EnteredCall();
+    void ExitedCall();
 
-    void ExitedCall() {
-        mListener->OnExitedCall();
-    }
+    void EnteredSyncSend();
+    void ExitedSyncSend();
 
-    void EnteredSyncSend() {
-        mListener->OnEnteredSyncSend();
-    }
-
-    void ExitedSyncSend() {
-        mListener->OnExitedSyncSend();
-    }
-
-    MessageListener *Listener() const {
+    IToplevelProtocol *Listener() const {
         return mListener;
     }
 
@@ -495,7 +492,7 @@ class MessageChannel : HasResultCodes
   private:
     // Based on presumption the listener owns and overlives the channel,
     // this is never nullified.
-    MessageListener* mListener;
+    IToplevelProtocol* mListener;
     ChannelState mChannelState;
     RefPtr<RefCountedMonitor> mMonitor;
     Side mSide;

ipc/glue/MessageLink.h

@@ -39,113 +39,6 @@ enum Side {
     UnknownSide
 };
 
-enum ChannelState {
-    ChannelClosed,
-    ChannelOpening,
-    ChannelConnected,
-    ChannelTimeout,
-    ChannelClosing,
-    ChannelError
-};
-
-// What happens if Interrupt calls race?
-enum RacyInterruptPolicy {
-    RIPError,
-    RIPChildWins,
-    RIPParentWins
-};
-
-class MessageListener
-  : protected HasResultCodes,
-    public mozilla::SupportsWeakPtr<MessageListener>
-{
-  public:
-    MOZ_DECLARE_WEAKREFERENCE_TYPENAME(MessageListener)
-    typedef IPC::Message Message;
-    typedef IPC::MessageInfo MessageInfo;
-
-    virtual ~MessageListener() { }
-
-    virtual void OnChannelClose() = 0;
-    virtual void OnChannelError() = 0;
-    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 void OnProcessingError(Result aError, const char* aMsgName) = 0;
-    virtual void OnChannelConnected(int32_t peer_pid) {}
-    virtual bool OnReplyTimeout() {
-        return false;
-    }
-
-    // WARNING: This function is called with the MessageChannel monitor held.
-    virtual void IntentionalCrash() {
-        MOZ_CRASH("Intentional IPDL crash");
-    }
-
-    // The code here is only useful for fuzzing. It should not be used for any
-    // other purpose.
-#ifdef DEBUG
-    // Returns true if we should simulate a timeout.
-    // WARNING: This is a testing-only function that is called with the
-    // MessageChannel monitor held. Don't do anything fancy here or we could
-    // deadlock.
-    virtual bool ArtificialTimeout() {
-        return false;
-    }
-
-    // Returns true if we want to cause the worker thread to sleep with the
-    // monitor unlocked.
-    virtual bool NeedArtificialSleep() {
-        return false;
-    }
-
-    // This function should be implemented to sleep for some amount of time on
-    // the worker thread. Will only be called if NeedArtificialSleep() returns
-    // true.
-    virtual void ArtificialSleep() {}
-#else
-    bool ArtificialTimeout() { return false; }
-    bool NeedArtificialSleep() { return false; }
-    void ArtificialSleep() {}
-#endif
-
-    virtual void OnEnteredCxxStack() {
-        NS_RUNTIMEABORT("default impl shouldn't be invoked");
-    }
-    virtual void OnExitedCxxStack() {
-        NS_RUNTIMEABORT("default impl shouldn't be invoked");
-    }
-    virtual void OnEnteredCall() {
-        NS_RUNTIMEABORT("default impl shouldn't be invoked");
-    }
-    virtual void OnExitedCall() {
-        NS_RUNTIMEABORT("default impl shouldn't be invoked");
-    }
-    virtual RacyInterruptPolicy MediateInterruptRace(const MessageInfo& parent,
-                                                     const MessageInfo& child)
-    {
-        return RIPChildWins;
-    }
-
-    /**
-     * Return true if windows messages can be handled while waiting for a reply
-     * to a sync IPDL message.
-     */
-    virtual bool HandleWindowsMessages(const Message& aMsg) const { return true; }
-
-    virtual void OnEnteredSyncSend() {
-    }
-    virtual void OnExitedSyncSend() {
-    }
-
-    virtual void ProcessRemoteNativeEventsInInterruptCall() {
-    }
-
-    // FIXME/bug 792652: this doesn't really belong here, but a
-    // large refactoring is needed to put it where it belongs.
-    virtual int32_t GetProtocolTypeId() = 0;
-};
-
 class MessageLink
 {
   public:

ipc/glue/ProtocolUtils.h

@@ -130,7 +130,14 @@ struct Trigger
     uint32_t mMessage : 31;
 };
 
-class IProtocol : public MessageListener
+// What happens if Interrupt calls race?
+enum RacyInterruptPolicy {
+    RIPError,
+    RIPChildWins,
+    RIPParentWins
+};
+
+class IProtocol : public HasResultCodes
 {
 public:
     enum ActorDestroyReason {
@@ -142,6 +149,8 @@ public:
     };
 
     typedef base::ProcessId ProcessId;
+    typedef IPC::Message Message;
+    typedef IPC::MessageInfo MessageInfo;
 
     virtual int32_t Register(IProtocol*) = 0;
     virtual int32_t RegisterID(IProtocol*, int32_t) = 0;
@@ -163,6 +172,12 @@ public:
 
     Maybe<IProtocol*> ReadActor(const IPC::Message* aMessage, PickleIterator* aIter, bool aNullable,
                                 const char* aActorDescription, int32_t aProtocolTypeId);
+
+    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;
 };
 
 typedef IPCMessageStart ProtocolId;
@@ -196,6 +211,78 @@ public:
 
     virtual MessageChannel* GetIPCChannel() = 0;
 
+    virtual void OnChannelClose() = 0;
+    virtual void OnChannelError() = 0;
+    virtual void OnProcessingError(Result aError, const char* aMsgName) = 0;
+    virtual void OnChannelConnected(int32_t peer_pid) {}
+    virtual bool OnReplyTimeout() {
+        return false;
+    }
+
+    // WARNING: This function is called with the MessageChannel monitor held.
+    virtual void IntentionalCrash() {
+        MOZ_CRASH("Intentional IPDL crash");
+    }
+
+    // The code here is only useful for fuzzing. It should not be used for any
+    // other purpose.
+#ifdef DEBUG
+    // Returns true if we should simulate a timeout.
+    // WARNING: This is a testing-only function that is called with the
+    // MessageChannel monitor held. Don't do anything fancy here or we could
+    // deadlock.
+    virtual bool ArtificialTimeout() {
+        return false;
+    }
+
+    // Returns true if we want to cause the worker thread to sleep with the
+    // monitor unlocked.
+    virtual bool NeedArtificialSleep() {
+        return false;
+    }
+
+    // This function should be implemented to sleep for some amount of time on
+    // the worker thread. Will only be called if NeedArtificialSleep() returns
+    // true.
+    virtual void ArtificialSleep() {}
+#else
+    bool ArtificialTimeout() { return false; }
+    bool NeedArtificialSleep() { return false; }
+    void ArtificialSleep() {}
+#endif
+
+    virtual void OnEnteredCxxStack() {
+        NS_RUNTIMEABORT("default impl shouldn't be invoked");
+    }
+    virtual void OnExitedCxxStack() {
+        NS_RUNTIMEABORT("default impl shouldn't be invoked");
+    }
+    virtual void OnEnteredCall() {
+        NS_RUNTIMEABORT("default impl shouldn't be invoked");
+    }
+    virtual void OnExitedCall() {
+        NS_RUNTIMEABORT("default impl shouldn't be invoked");
+    }
+    virtual RacyInterruptPolicy MediateInterruptRace(const MessageInfo& parent,
+                                                     const MessageInfo& child)
+    {
+        return RIPChildWins;
+    }
+
+    /**
+     * Return true if windows messages can be handled while waiting for a reply
+     * to a sync IPDL message.
+     */
+    virtual bool HandleWindowsMessages(const Message& aMsg) const { return true; }
+
+    virtual void OnEnteredSyncSend() {
+    }
+    virtual void OnExitedSyncSend() {
+    }
+
+    virtual void ProcessRemoteNativeEventsInInterruptCall() {
+    }
+
 private:
     ProtocolId mProtocolId;
     UniquePtr<Transport> mTrans;

ipc/glue/WindowsMessageLoop.cpp

@@ -17,6 +17,7 @@
 #include "WinUtils.h"
 
 #include "mozilla/ArrayUtils.h"
+#include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/PaintTracker.h"
 #include "mozilla/WindowsVersion.h"
 

ipc/ipdl/ipdl/lower.py

@@ -1090,9 +1090,6 @@ class Protocol(ipdl.ast.Protocol):
     def channelHeaderFile(self):
         return '/'.join(_semsToChannelParts(self.sendSems())) +'.h'
 
-    def fqListenerName(self):
-      return 'mozilla::ipc::MessageListener'
-
     def managerInterfaceType(self, ptr=0):
         return Type('mozilla::ipc::IProtocol', ptr=ptr)
 
@@ -2623,7 +2620,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
             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('mozilla::ipc::IProtocol'), 'ChannelListener'),
             Typedef(Type('base::ProcessHandle'), 'ProcessHandle'),
             Typedef(Type('mozilla::ipc::MessageChannel'), 'MessageChannel'),
             Typedef(Type('mozilla::ipc::SharedMemory'), 'SharedMemory'),