Bug 977672: Cleanup runnables and tasks in UnixSocket.cpp, r=kyle
authorThomas Zimmermann <tdz@users.sourceforge.net>
Fri, 28 Feb 2014 10:16:53 +0100
changeset 171629 196d98e2fc8ade808ee045e93a652135f400833b
parent 171628 4f7dc3feaf3178dd241d6b90543fc83196639f12
child 171630 fc62596d4befc45d5720f487bf227ac76ee9c2f2
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerskyle
bugs977672
milestone30.0a1
Bug 977672: Cleanup runnables and tasks in UnixSocket.cpp, r=kyle This patch cleans up runnables and tasks in UnixSocket.cpp. Every runnable's name now ends in Runnable. There are a base classes for runnables and tasks that hold reference to UnixSocketImpl and from which all runnables and tasks inherit.
ipc/unixsocket/UnixSocket.cpp
--- a/ipc/unixsocket/UnixSocket.cpp
+++ b/ipc/unixsocket/UnixSocket.cpp
@@ -205,242 +205,278 @@ public:
 
     return NS_OK;
   }
 
 private:
   T* mInstance;
 };
 
-class OnSocketEventTask : public nsRunnable
+class UnixSocketImplRunnable : public nsRunnable
+{
+public:
+  UnixSocketImpl* GetImpl() const
+  {
+    return mImpl;
+  }
+protected:
+  UnixSocketImplRunnable(UnixSocketImpl* aImpl)
+  : mImpl(aImpl)
+  {
+    MOZ_ASSERT(aImpl);
+  }
+  virtual ~UnixSocketImplRunnable()
+  { }
+private:
+  UnixSocketImpl* mImpl;
+};
+
+class OnSocketEventRunnable : public UnixSocketImplRunnable
 {
 public:
   enum SocketEvent {
     CONNECT_SUCCESS,
     CONNECT_ERROR,
     DISCONNECT
   };
 
-  OnSocketEventTask(UnixSocketImpl* aImpl, SocketEvent e) :
-    mImpl(aImpl),
-    mEvent(e)
+  OnSocketEventRunnable(UnixSocketImpl* aImpl, SocketEvent e)
+  : UnixSocketImplRunnable(aImpl)
+  , mEvent(e)
   {
-    MOZ_ASSERT(aImpl);
     MOZ_ASSERT(!NS_IsMainThread());
   }
 
-  NS_IMETHOD Run()
+  NS_IMETHOD Run() MOZ_OVERRIDE
   {
     MOZ_ASSERT(NS_IsMainThread());
-    if (mImpl->IsShutdownOnMainThread()) {
+
+    UnixSocketImpl* impl = GetImpl();
+
+    if (impl->IsShutdownOnMainThread()) {
       NS_WARNING("CloseSocket has already been called!");
       // Since we've already explicitly closed and the close happened before
       // this, this isn't really an error. Since we've warned, return OK.
       return NS_OK;
     }
     if (mEvent == CONNECT_SUCCESS) {
-      mImpl->mConsumer->NotifySuccess();
+      impl->mConsumer->NotifySuccess();
     } else if (mEvent == CONNECT_ERROR) {
-      mImpl->mConsumer->NotifyError();
+      impl->mConsumer->NotifyError();
     } else if (mEvent == DISCONNECT) {
-      mImpl->mConsumer->NotifyDisconnect();
+      impl->mConsumer->NotifyDisconnect();
     }
     return NS_OK;
   }
 private:
-  UnixSocketImpl* mImpl;
   SocketEvent mEvent;
 };
 
-class SocketReceiveTask : public nsRunnable
+class SocketReceiveRunnable : public UnixSocketImplRunnable
 {
 public:
-  SocketReceiveTask(UnixSocketImpl* aImpl, UnixSocketRawData* aData) :
-    mImpl(aImpl),
-    mRawData(aData)
+  SocketReceiveRunnable(UnixSocketImpl* aImpl, UnixSocketRawData* aData)
+  : UnixSocketImplRunnable(aImpl)
+  , mRawData(aData)
   {
-    MOZ_ASSERT(aImpl);
     MOZ_ASSERT(aData);
   }
 
-  NS_IMETHOD Run()
+  NS_IMETHOD Run() MOZ_OVERRIDE
   {
     MOZ_ASSERT(NS_IsMainThread());
-    if (mImpl->IsShutdownOnMainThread()) {
+
+    UnixSocketImpl* impl = GetImpl();
+
+    if (impl->IsShutdownOnMainThread()) {
       NS_WARNING("mConsumer is null, aborting receive!");
       // Since we've already explicitly closed and the close happened before
       // this, this isn't really an error. Since we've warned, return OK.
       return NS_OK;
     }
 
-    MOZ_ASSERT(mImpl->mConsumer);
-    mImpl->mConsumer->ReceiveSocketData(mRawData);
+    MOZ_ASSERT(impl->mConsumer);
+    impl->mConsumer->ReceiveSocketData(mRawData);
     return NS_OK;
   }
 private:
-  UnixSocketImpl* mImpl;
   nsAutoPtr<UnixSocketRawData> mRawData;
 };
 
-class SocketSendTask : public Task
+class RequestClosingSocketRunnable : public UnixSocketImplRunnable
 {
 public:
-  SocketSendTask(UnixSocketConsumer* aConsumer, UnixSocketImpl* aImpl,
-                 UnixSocketRawData* aData)
-    : mConsumer(aConsumer),
-      mImpl(aImpl),
-      mData(aData)
-  {
-    MOZ_ASSERT(aConsumer);
-    MOZ_ASSERT(aImpl);
-    MOZ_ASSERT(aData);
-  }
-
-  void
-  Run()
-  {
-    MOZ_ASSERT(!NS_IsMainThread());
-    MOZ_ASSERT(!mImpl->IsShutdownOnIOThread());
+  RequestClosingSocketRunnable(UnixSocketImpl* aImpl)
+  : UnixSocketImplRunnable(aImpl)
+  { }
 
-    mImpl->QueueWriteData(mData);
-  }
-
-private:
-  nsRefPtr<UnixSocketConsumer> mConsumer;
-  UnixSocketImpl* mImpl;
-  UnixSocketRawData* mData;
-};
-
-class RequestClosingSocketTask : public nsRunnable
-{
-public:
-  RequestClosingSocketTask(UnixSocketImpl* aImpl) : mImpl(aImpl)
-  {
-    MOZ_ASSERT(aImpl);
-  }
-
-  NS_IMETHOD Run()
+  NS_IMETHOD Run() MOZ_OVERRIDE
   {
     MOZ_ASSERT(NS_IsMainThread());
 
-    if (mImpl->IsShutdownOnMainThread()) {
+    UnixSocketImpl* impl = GetImpl();
+    if (impl->IsShutdownOnMainThread()) {
       NS_WARNING("CloseSocket has already been called!");
       // Since we've already explicitly closed and the close happened before
       // this, this isn't really an error. Since we've warned, return OK.
       return NS_OK;
     }
 
     // Start from here, same handling flow as calling CloseSocket() from
     // upper layer
-    mImpl->mConsumer->CloseSocket();
+    impl->mConsumer->CloseSocket();
     return NS_OK;
   }
+};
+
+class UnixSocketImplTask : public CancelableTask
+{
+public:
+  UnixSocketImpl* GetImpl() const
+  {
+    return mImpl;
+  }
+  void Cancel() MOZ_OVERRIDE
+  {
+    mImpl = nullptr;
+  }
+  bool IsCanceled() const
+  {
+    return !mImpl;
+  }
+protected:
+  UnixSocketImplTask(UnixSocketImpl* aImpl)
+  : mImpl(aImpl)
+  {
+    MOZ_ASSERT(mImpl);
+  }
 private:
   UnixSocketImpl* mImpl;
 };
 
-class SocketListenTask : public CancelableTask
+class SocketSendTask : public UnixSocketImplTask
 {
-  virtual void Run();
-
-  UnixSocketImpl* mImpl;
 public:
-  SocketListenTask(UnixSocketImpl* aImpl) : mImpl(aImpl) { }
-
-  virtual void Cancel()
+  SocketSendTask(UnixSocketImpl* aImpl,
+                 UnixSocketConsumer* aConsumer,
+                 UnixSocketRawData* aData)
+  : UnixSocketImplTask(aImpl)
+  , mConsumer(aConsumer)
+  , mData(aData)
+  {
+    MOZ_ASSERT(aConsumer);
+    MOZ_ASSERT(aData);
+  }
+  void Run() MOZ_OVERRIDE
   {
     MOZ_ASSERT(!NS_IsMainThread());
-    mImpl = nullptr;
+    MOZ_ASSERT(!IsCanceled());
+
+    UnixSocketImpl* impl = GetImpl();
+    MOZ_ASSERT(!impl->IsShutdownOnIOThread());
+
+    impl->QueueWriteData(mData);
+  }
+private:
+  nsRefPtr<UnixSocketConsumer> mConsumer;
+  UnixSocketRawData* mData;
+};
+
+class SocketListenTask : public UnixSocketImplTask
+{
+public:
+  SocketListenTask(UnixSocketImpl* aImpl)
+  : UnixSocketImplTask(aImpl)
+  { }
+
+  void Run() MOZ_OVERRIDE
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+    if (!IsCanceled()) {
+      GetImpl()->Listen();
+    }
   }
 };
 
-void SocketListenTask::Run()
+class SocketConnectTask : public UnixSocketImplTask
 {
-  MOZ_ASSERT(!NS_IsMainThread());
-
-  if (mImpl) {
-    mImpl->Listen();
-  }
-}
-
-class SocketConnectTask : public Task {
-  virtual void Run();
-
-  UnixSocketImpl* mImpl;
 public:
-  SocketConnectTask(UnixSocketImpl* aImpl) : mImpl(aImpl) { }
-};
+  SocketConnectTask(UnixSocketImpl* aImpl)
+  : UnixSocketImplTask(aImpl)
+  { }
 
-void SocketConnectTask::Run()
-{
-  MOZ_ASSERT(!NS_IsMainThread());
-  mImpl->Connect();
-}
-
-class SocketDelayedConnectTask : public CancelableTask {
-  virtual void Run();
-
-  UnixSocketImpl* mImpl;
-public:
-  SocketDelayedConnectTask(UnixSocketImpl* aImpl) : mImpl(aImpl) { }
-
-  virtual void Cancel()
+  void Run() MOZ_OVERRIDE
   {
-    MOZ_ASSERT(NS_IsMainThread());
-    mImpl = nullptr;
+    MOZ_ASSERT(!NS_IsMainThread());
+    MOZ_ASSERT(!IsCanceled());
+    GetImpl()->Connect();
   }
 };
 
-void SocketDelayedConnectTask::Run()
+class SocketDelayedConnectTask : public UnixSocketImplTask
 {
-  MOZ_ASSERT(NS_IsMainThread());
-  if (!mImpl || mImpl->IsShutdownOnMainThread()) {
-    return;
+public:
+  SocketDelayedConnectTask(UnixSocketImpl* aImpl)
+  : UnixSocketImplTask(aImpl)
+  { }
+
+  void Run() MOZ_OVERRIDE
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+    if (IsCanceled()) {
+      return;
+    }
+    UnixSocketImpl* impl = GetImpl();
+    if (impl->IsShutdownOnMainThread()) {
+      return;
+    }
+    impl->ClearDelayedConnectTask();
+    XRE_GetIOMessageLoop()->PostTask(FROM_HERE, new SocketConnectTask(impl));
   }
-  mImpl->ClearDelayedConnectTask();
-  XRE_GetIOMessageLoop()->PostTask(FROM_HERE, new SocketConnectTask(mImpl));
-}
-
-class ShutdownSocketTask : public Task {
-  virtual void Run();
-
-  UnixSocketImpl* mImpl;
-
-public:
-  ShutdownSocketTask(UnixSocketImpl* aImpl) : mImpl(aImpl) { }
 };
 
-void ShutdownSocketTask::Run()
+class ShutdownSocketTask : public UnixSocketImplTask
 {
-  MOZ_ASSERT(!NS_IsMainThread());
+public:
+  ShutdownSocketTask(UnixSocketImpl* aImpl)
+  : UnixSocketImplTask(aImpl)
+  { }
+
+  void Run() MOZ_OVERRIDE
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+    MOZ_ASSERT(!IsCanceled());
 
-  // At this point, there should be no new events on the IO thread after this
-  // one with the possible exception of a SocketListenTask that
-  // ShutdownOnIOThread will cancel for us. We are now fully shut down, so we
-  // can send a message to the main thread that will delete mImpl safely knowing
-  // that no more tasks reference it.
-  mImpl->ShutdownOnIOThread();
+    UnixSocketImpl* impl = GetImpl();
 
-  nsRefPtr<nsIRunnable> t(new DeleteInstanceRunnable<UnixSocketImpl>(mImpl));
-  nsresult rv = NS_DispatchToMainThread(t);
-  NS_ENSURE_SUCCESS_VOID(rv);
-}
+    // At this point, there should be no new events on the IO thread after this
+    // one with the possible exception of a SocketListenTask that
+    // ShutdownOnIOThread will cancel for us. We are now fully shut down, so we
+    // can send a message to the main thread that will delete impl safely knowing
+    // that no more tasks reference it.
+    impl->ShutdownOnIOThread();
+
+    nsRefPtr<nsIRunnable> r(new DeleteInstanceRunnable<UnixSocketImpl>(impl));
+    nsresult rv = NS_DispatchToMainThread(r);
+    NS_ENSURE_SUCCESS_VOID(rv);
+  }
+};
 
 void
 UnixSocketImpl::FireSocketError()
 {
   MOZ_ASSERT(MessageLoopForIO::current() == GetIOLoop());
 
   // Clean up watchers, statuses, fds
   Close();
 
   // Tell the main thread we've errored
-  nsRefPtr<OnSocketEventTask> t =
-    new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
-  NS_DispatchToMainThread(t);
+  nsRefPtr<OnSocketEventRunnable> r =
+    new OnSocketEventRunnable(this, OnSocketEventRunnable::CONNECT_ERROR);
+  NS_DispatchToMainThread(r);
 }
 
 void
 UnixSocketImpl::Listen()
 {
   MOZ_ASSERT(MessageLoopForIO::current() == GetIOLoop());
   MOZ_ASSERT(mConnector);
 
@@ -551,19 +587,19 @@ UnixSocketImpl::OnAccepted(int aFd)
 
   RemoveWatchers(READ_WATCHER|WRITE_WATCHER);
   Close();
   if (!SetSocketFlags(aFd)) {
     return;
   }
   SetSocket(aFd, SOCKET_IS_CONNECTED);
 
-  nsRefPtr<OnSocketEventTask> t =
-    new OnSocketEventTask(this, OnSocketEventTask::CONNECT_SUCCESS);
-  NS_DispatchToMainThread(t);
+  nsRefPtr<OnSocketEventRunnable> r =
+    new OnSocketEventRunnable(this, OnSocketEventRunnable::CONNECT_SUCCESS);
+  NS_DispatchToMainThread(r);
 
   AddWatchers(READ_WATCHER, true);
   if (!mOutgoingQ.IsEmpty()) {
     AddWatchers(WRITE_WATCHER, false);
   }
 }
 
 void
@@ -579,19 +615,19 @@ UnixSocketImpl::OnConnected()
   }
 
   if (!mConnector->SetUp(GetFd())) {
     NS_WARNING("Could not set up socket!");
     FireSocketError();
     return;
   }
 
-  nsRefPtr<OnSocketEventTask> t =
-    new OnSocketEventTask(this, OnSocketEventTask::CONNECT_SUCCESS);
-  NS_DispatchToMainThread(t);
+  nsRefPtr<OnSocketEventRunnable> r =
+    new OnSocketEventRunnable(this, OnSocketEventRunnable::CONNECT_SUCCESS);
+  NS_DispatchToMainThread(r);
 
   AddWatchers(READ_WATCHER, true);
   if (!mOutgoingQ.IsEmpty()) {
     AddWatchers(WRITE_WATCHER, false);
   }
 }
 
 void
@@ -642,25 +678,26 @@ UnixSocketImpl::OnSocketCanReceiveWithou
         NS_WARNING("Cannot read from network");
 #endif
         // else fall through to error handling on other errno's
       }
 
       // We're done with our descriptors. Ensure that spurious events don't
       // cause us to end up back here.
       RemoveWatchers(READ_WATCHER|WRITE_WATCHER);
-      nsRefPtr<RequestClosingSocketTask> t = new RequestClosingSocketTask(this);
-      NS_DispatchToMainThread(t);
+      nsRefPtr<RequestClosingSocketRunnable> r =
+        new RequestClosingSocketRunnable(this);
+      NS_DispatchToMainThread(r);
       return;
     }
 
     incoming->mSize = ret;
-    nsRefPtr<SocketReceiveTask> t =
-      new SocketReceiveTask(this, incoming.forget());
-    NS_DispatchToMainThread(t);
+    nsRefPtr<SocketReceiveRunnable> r =
+      new SocketReceiveRunnable(this, incoming.forget());
+    NS_DispatchToMainThread(r);
 
     // If ret is less than MAX_READ_SIZE, there's no
     // more data in the socket for us to read now.
     if (ret < ssize_t(MAX_READ_SIZE)) {
       return;
     }
   }
 }
@@ -724,17 +761,17 @@ UnixSocketConsumer::SendSocketData(UnixS
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mImpl) {
     return false;
   }
 
   MOZ_ASSERT(!mImpl->IsShutdownOnMainThread());
   XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
-                                   new SocketSendTask(this, mImpl, aData));
+                                   new SocketSendTask(mImpl, this, aData));
   return true;
 }
 
 bool
 UnixSocketConsumer::SendSocketData(const nsACString& aStr)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mImpl) {
@@ -743,17 +780,17 @@ UnixSocketConsumer::SendSocketData(const
   if (aStr.Length() > MAX_READ_SIZE) {
     return false;
   }
 
   MOZ_ASSERT(!mImpl->IsShutdownOnMainThread());
   UnixSocketRawData* d = new UnixSocketRawData(aStr.BeginReading(),
                                                aStr.Length());
   XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
-                                   new SocketSendTask(this, mImpl, d));
+                                   new SocketSendTask(mImpl, this, d));
   return true;
 }
 
 void
 UnixSocketConsumer::CloseSocket()
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mImpl) {