Bug 792652 - Remove methods from MessageListener (r=dvander)
☠☠ backed out by d2098e1d1f2d ☠ ☠
authorBill McCloskey <billm@mozilla.com>
Sun, 30 Oct 2016 11:26:40 -0700
changeset 321721 dbbe3a8c00e7ce274da0c78b9338cf76957a8958
parent 321720 1829d53588088806844de65624363b71453a2ad3
child 321722 5a1e3136323a6a2c2607ef157643253f75e3a5db
push id30934
push usercbook@mozilla.com
push dateWed, 09 Nov 2016 15:38:21 +0000
treeherdermozilla-central@336759fad462 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs792652
milestone52.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 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
dom/plugins/ipc/PluginModuleParent.cpp
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
ipc/glue/MessageLink.h
ipc/glue/ProtocolUtils.h
ipc/glue/WindowsMessageLoop.cpp
ipc/ipdl/ipdl/lower.py
--- a/dom/plugins/ipc/PluginMessageUtils.h
+++ b/dom/plugins/ipc/PluginMessageUtils.h
@@ -5,18 +5,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef DOM_PLUGINS_PLUGINMESSAGEUTILS_H
 #define DOM_PLUGINS_PLUGINMESSAGEUTILS_H
 
 #include "ipc/IPCMessageUtils.h"
 #include "base/message_loop.h"
 
+#include "mozilla/ipc/CrossProcessMutex.h"
 #include "mozilla/ipc/MessageChannel.h"
-#include "mozilla/ipc/CrossProcessMutex.h"
+#include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/UniquePtr.h"
 #include "gfxipc/ShadowLayerUtils.h"
 
 #include "npapi.h"
 #include "npruntime.h"
 #include "npfunctions.h"
 #include "nsString.h"
 #include "nsTArray.h"
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -1031,18 +1031,17 @@ PluginModuleChromeParent::GetInvokingPro
  *
  * This function needs to be updated if the subprotocols are modified in
  * PPluginInstance.ipdl.
  */
 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.
             return static_cast<PluginInstanceParent*>(aProtocol);
         case PPluginBackgroundDestroyerMsgStart: {
             PPluginBackgroundDestroyerParent* actor =
                 static_cast<PPluginBackgroundDestroyerParent*>(aProtocol);
             return static_cast<PluginInstanceParent*>(actor->Manager());
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -467,17 +467,17 @@ private:
 
     // Next item in mChan->mTransactionStack.
     AutoEnterTransaction *mNext;
 
     // Pointer the a reply received for this message, if one was received.
     nsAutoPtr<IPC::Message> mReply;
 };
 
-MessageChannel::MessageChannel(MessageListener *aListener)
+MessageChannel::MessageChannel(IToplevelProtocol *aListener)
   : mListener(aListener),
     mChannelState(ChannelClosed),
     mSide(UnknownSide),
     mLink(nullptr),
     mWorkerLoop(nullptr),
     mChannelErrorTask(nullptr),
     mWorkerLoopID(-1),
     mTimeoutMs(kNoTimeout),
@@ -1852,28 +1852,58 @@ MessageChannel::MaybeUndeferIncall()
 
     MOZ_RELEASE_ASSERT(call.nested_level() == IPC::Message::NOT_NESTED);
     RefPtr<MessageTask> task = new MessageTask(this, Move(call));
     mPending.insertBack(task);
     task->Post();
 }
 
 void
+MessageChannel::EnteredCxxStack()
+{
+    mListener->OnEnteredCxxStack();
+}
+
+void
 MessageChannel::ExitedCxxStack()
 {
     mListener->OnExitedCxxStack();
     if (mSawInterruptOutMsg) {
         MonitorAutoLock lock(*mMonitor);
         // see long comment in OnMaybeDequeueOne()
         EnqueuePendingMessages();
         mSawInterruptOutMsg = false;
     }
 }
 
 void
+MessageChannel::EnteredCall()
+{
+    mListener->OnEnteredCall();
+}
+
+void
+MessageChannel::ExitedCall()
+{
+    mListener->OnExitedCall();
+}
+
+void
+MessageChannel::EnteredSyncSend()
+{
+    mListener->OnEnteredSyncSend();
+}
+
+void
+MessageChannel::ExitedSyncSend()
+{
+    mListener->OnExitedSyncSend();
+}
+
+void
 MessageChannel::EnqueuePendingMessages()
 {
     AssertWorkerThread();
     mMonitor->AssertCurrentThreadOwns();
 
     MaybeUndeferIncall();
 
     // XXX performance tuning knob: could process all or k pending
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -28,16 +28,17 @@
 #include <deque>
 #include <stack>
 #include <math.h>
 
 namespace mozilla {
 namespace ipc {
 
 class MessageChannel;
+class IToplevelProtocol;
 
 class RefCountedMonitor : public Monitor
 {
   public:
     RefCountedMonitor()
         : Monitor("mozilla.ipc.MessageChannel.mMonitor")
     {}
 
@@ -55,16 +56,25 @@ enum class SyncSendError {
     NotConnectedBeforeSend,
     DisconnectedDuringSend,
     CancelledBeforeSend,
     CancelledAfterSend,
     TimedOut,
     ReplyError,
 };
 
+enum ChannelState {
+    ChannelClosed,
+    ChannelOpening,
+    ChannelConnected,
+    ChannelTimeout,
+    ChannelClosing,
+    ChannelError
+};
+
 class AutoEnterTransaction;
 
 class MessageChannel : HasResultCodes
 {
     friend class ProcessLink;
     friend class ThreadLink;
 
     class CxxStackFrame;
@@ -74,17 +84,17 @@ class MessageChannel : HasResultCodes
 
   public:
     static const int32_t kNoTimeout;
 
     typedef IPC::Message Message;
     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
     // socketpair/pipe should already be created.
     //
     // Returns true if the transport layer was successfully connected,
     // i.e., mChannelState == ChannelConnected.
     bool Open(Transport* aTransport, MessageLoop* aIOLoop=0, Side aSide=UnknownSide);
@@ -323,39 +333,26 @@ class MessageChannel : HasResultCodes
     int32_t NextSeqno() {
         AssertWorkerThread();
         return (mSide == ChildSide) ? --mNextSeqno : ++mNextSeqno;
     }
 
     // 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 ExitedCall() {
-        mListener->OnExitedCall();
-    }
+    void EnteredCall();
+    void ExitedCall();
 
-    void EnteredSyncSend() {
-        mListener->OnEnteredSyncSend();
-    }
+    void EnteredSyncSend();
+    void ExitedSyncSend();
 
-    void ExitedSyncSend() {
-        mListener->OnExitedSyncSend();
-    }
-
-    MessageListener *Listener() const {
+    IToplevelProtocol *Listener() const {
         return mListener;
     }
 
     void DebugAbort(const char* file, int line, const char* cond,
                     const char* why,
                     bool reply=false);
 
     // This method is only safe to call on the worker thread, or in a
@@ -490,17 +487,17 @@ class MessageChannel : HasResultCodes
 
     typedef LinkedList<RefPtr<MessageTask>> MessageQueue;
     typedef std::map<size_t, Message> MessageMap;
     typedef IPC::Message::msgid_t msgid_t;
 
   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;
     MessageLink* mLink;
     MessageLoop* mWorkerLoop;           // thread where work is done
     RefPtr<CancelableRunnable> mChannelErrorTask;  // NotifyMaybeChannelError runnable
 
     // id() of mWorkerLoop.  This persists even after mWorkerLoop is cleared
--- a/ipc/glue/MessageLink.h
+++ b/ipc/glue/MessageLink.h
@@ -34,123 +34,16 @@ struct HasResultCodes
 };
 
 enum Side {
     ParentSide,
     ChildSide,
     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:
     typedef IPC::Message Message;
 
     explicit MessageLink(MessageChannel *aChan);
     virtual ~MessageLink();
 
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -125,28 +125,37 @@ struct Trigger
     {
       MOZ_ASSERT(0 <= msg && msg < INT32_MAX);
     }
 
     uint32_t mAction : 1;
     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 {
         FailedConstructor,
         Deletion,
         AncestorDeletion,
         NormalShutdown,
         AbnormalShutdown
     };
 
     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;
     virtual IProtocol* Lookup(int32_t) = 0;
     virtual void Unregister(int32_t) = 0;
     virtual void RemoveManagee(int32_t, IProtocol*) = 0;
 
     virtual Shmem::SharedMemory* CreateSharedMemory(
@@ -158,16 +167,22 @@ public:
     // 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<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;
 
 template<class PFooSide>
 class Endpoint;
 
 /**
@@ -191,16 +206,88 @@ public:
     }
 
     Transport* GetTransport() const { return mTrans.get(); }
 
     ProtocolId GetProtocolId() const { return mProtocolId; }
 
     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;
 };
 
 class IShmemAllocator
 {
 public:
--- a/ipc/glue/WindowsMessageLoop.cpp
+++ b/ipc/glue/WindowsMessageLoop.cpp
@@ -12,16 +12,17 @@
 
 #include "nsAutoPtr.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
 #include "nsIXULAppInfo.h"
 #include "WinUtils.h"
 
 #include "mozilla/ArrayUtils.h"
+#include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/PaintTracker.h"
 #include "mozilla/WindowsVersion.h"
 
 using namespace mozilla;
 using namespace mozilla::ipc;
 using namespace mozilla::ipc::windows;
 
 /**
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -1085,19 +1085,16 @@ class Protocol(ipdl.ast.Protocol):
         return '->'
 
     def channelType(self):
         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 managerInterfaceType(self, ptr=0):
         return Type('mozilla::ipc::IProtocol', ptr=ptr)
 
     def openedProtocolInterfaceType(self, ptr=0):
         return Type('mozilla::ipc::IToplevelProtocol',
                     ptr=ptr)
 
     def _ipdlmgrtype(self):
@@ -2618,17 +2615,17 @@ class _GenerateProtocolActorCode(ipdl.as
         self.cppfile = cxxFile
         tu.accept(self)
 
     def standardTypedefs(self):
         return [
             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'),
             Typedef(Type('mozilla::ipc::Trigger'), 'Trigger'),
         ]
 
 
     def visitTranslationUnit(self, tu):