Bug 544518: Send Messages directly through the Transport on the IO thread rather than through a no-added-value AsyncChannel indirection. r=bent

ipc/glue/AsyncChannel.cpp

@@ -54,6 +54,25 @@ struct RunnableMethodTraits<mozilla::ipc::AsyncChannel>
     static void ReleaseCallee(mozilla::ipc::AsyncChannel* obj) { }
 };
 
+// We rely on invariants about the lifetime of the transport:
+//
+//  - outlives this AsyncChannel
+//  - deleted on the IO thread
+//
+// These invariants allow us to send messages directly through the
+// transport without having to worry about orphaned Send() tasks on
+// the IO thread touching AsyncChannel memory after it's been deleted
+// on the worker thread.  We also don't need to refcount the
+// Transport, because whatever task triggers its deletion only runs on
+// the IO thread, and only runs after this AsyncChannel is done with
+// the Transport.
+template<>
+struct RunnableMethodTraits<mozilla::ipc::AsyncChannel::Transport>
+{
+    static void RetainCallee(mozilla::ipc::AsyncChannel::Transport* obj) { }
+    static void ReleaseCallee(mozilla::ipc::AsyncChannel::Transport* obj) { }
+};
+
 namespace {
 
 // This is an async message
@@ -216,8 +235,7 @@ AsyncChannel::Send(Message* msg)
             return false;
         }
 
-        mIOLoop->PostTask(FROM_HERE,
-                          NewRunnableMethod(this, &AsyncChannel::OnSend, msg));
+        SendThroughTransport(msg);
     }
 
     return true;
@@ -251,12 +269,19 @@ AsyncChannel::OnSpecialMessage(uint16 id, const Message& msg)
 
 void
 AsyncChannel::SendSpecialMessage(Message* msg)
+{
+    AssertWorkerThread();
+    SendThroughTransport(msg);
+}
+
+void
+AsyncChannel::SendThroughTransport(Message* msg)
 {
     AssertWorkerThread();
 
     mIOLoop->PostTask(
         FROM_HERE,
-        NewRunnableMethod(this, &AsyncChannel::OnSend, msg));
+        NewRunnableMethod(mTransport, &Transport::Send, msg));
 }
 
 void
@@ -463,14 +488,6 @@ AsyncChannel::PostErrorNotifyTask()
     mWorkerLoop->PostTask(FROM_HERE, mChannelErrorTask);
 }
 
-void
-AsyncChannel::OnSend(Message* aMsg)
-{
-    AssertIOThread();
-    mTransport->Send(aMsg);
-    // mTransport assumes ownership of aMsg
-}
-
 void
 AsyncChannel::OnCloseChannel()
 {

ipc/glue/AsyncChannel.h

@@ -161,6 +161,8 @@ protected:
 
     // Run on the worker thread
 
+    void SendThroughTransport(Message* msg);
+
     void OnNotifyMaybeChannelError();
     virtual bool ShouldDeferNotifyMaybeError() {
         return false;
@@ -173,7 +175,6 @@ protected:
     // Run on the IO thread
 
     void OnChannelOpened();
-    void OnSend(Message* aMsg);
     void OnCloseChannel();
     void PostErrorNotifyTask();
 

ipc/glue/RPCChannel.cpp

@@ -180,9 +180,7 @@ RPCChannel::Call(Message* msg, Message* reply)
     msg->set_rpc_local_stack_depth(1 + StackDepth());
     mStack.push(*msg);
 
-    mIOLoop->PostTask(
-        FROM_HERE,
-        NewRunnableMethod(this, &RPCChannel::OnSend, msg));
+    SendThroughTransport(msg);
 
     while (1) {
         // if a handler invoked by *Dispatch*() spun a nested event
@@ -498,9 +496,7 @@ RPCChannel::DispatchIncall(const Message& call)
     {
         MutexAutoLock lock(mMutex);
         if (ChannelConnected == mChannelState)
-            mIOLoop->PostTask(
-                FROM_HERE,
-                NewRunnableMethod(this, &RPCChannel::OnSend, reply));
+            SendThroughTransport(reply);
     }
 }
 

ipc/glue/SyncChannel.cpp

@@ -111,9 +111,7 @@ SyncChannel::Send(Message* msg, Message* reply)
 
     mPendingReply = msg->type() + 1;
     int32 msgSeqno = msg->seqno();
-    mIOLoop->PostTask(
-        FROM_HERE,
-        NewRunnableMethod(this, &SyncChannel::OnSend, msg));
+    SendThroughTransport(msg);
 
     while (1) {
         bool maybeTimedOut = !SyncChannel::WaitForNotify();
@@ -178,9 +176,7 @@ SyncChannel::OnDispatchMessage(const Message& msg)
     {
         MutexAutoLock lock(mMutex);
         if (ChannelConnected == mChannelState)
-            mIOLoop->PostTask(
-                FROM_HERE,
-                NewRunnableMethod(this, &SyncChannel::OnSend, reply));
+            SendThroughTransport(reply);
     }
 }
 

ipc/glue/SyncChannel.h

@@ -129,7 +129,6 @@ protected:
     bool ShouldContinueFromTimeout();
 
     // Executed on the IO thread.
-    void OnSendReply(Message* msg);
     void NotifyWorkerThread();
 
     // On both