Convert over to channel handles

This hides some of the internals of the posix channels from users, and gets rid
of several #ifdef POSIX blocks. Generally simplifies usage of channels xplatform.

BUG=none
TEST=build

Review URL: http://codereview.chromium.org/5598010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68621 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 92 insertions, 81 deletions

diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
index 1b5d73a..d07e3e5 100644
--- a/ipc/ipc_channel.h
+++ b/ipc/ipc_channel.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include "base/compiler_specific.h"
+#include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_message.h"
 
 namespace IPC {
@@ -54,7 +55,10 @@ class Channel : public Message::Sender {
 
   // Initialize a Channel.
   //
-  // |channel_id| identifies the communication Channel.
+  // |channel_handle| identifies the communication Channel. For POSIX, if
+  // the file descriptor in the channel handle is != -1, the channel takes
+  // ownership of the file descriptor and will close it appropriately, otherwise
+  // it will create a new descriptor internally.
   // |mode| specifies whether this Channel is to operate in server mode or
   // client mode.  In server mode, the Channel is responsible for setting up the
   // IPC object, whereas in client mode, the Channel merely connects to the
@@ -62,7 +66,8 @@ class Channel : public Message::Sender {
   // |listener| receives a callback on the current thread for each newly
   // received message.
   //
-  Channel(const std::string& channel_id, Mode mode, Listener* listener);
+  Channel(const IPC::ChannelHandle &channel_handle, Mode mode,
+          Listener* listener);
 
   ~Channel();
 
diff --git a/ipc/ipc_channel_handle.h b/ipc/ipc_channel_handle.h
index dc6957f..1b9a385 100644
--- a/ipc/ipc_channel_handle.h
+++ b/ipc/ipc_channel_handle.h
@@ -29,18 +29,19 @@ namespace IPC {
 struct ChannelHandle {
   // Note that serialization for this object is defined in the ParamTraits
   // template specialization in ipc_message_utils.h.
-  std::string name;
-#if defined(OS_POSIX)
-  base::FileDescriptor socket;
-#endif
-
   ChannelHandle() {}
+  ChannelHandle(const std::string& n) : name(n) {}
+  ChannelHandle(const char* n) : name(n) {}
 #if defined(OS_POSIX)
   ChannelHandle(const std::string& n, const base::FileDescriptor& s)
       : name(n), socket(s) {}
-#else
-  ChannelHandle(const std::string& n) : name(n) {}
-#endif
+#endif  // defined(OS_POSIX)
+
+  std::string name;
+#if defined(OS_POSIX)
+  base::FileDescriptor socket;
+#endif  // defined(OS_POSIX)
+
 };
 
 }  // namespace IPC
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index 781175e3..4a32f44 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -79,6 +79,11 @@ class PipeMap {
     return Singleton<PipeMap>::get();
   }
 
+  ~PipeMap() {
+    // Shouldn't have left over pipes.
+    DCHECK(map_.size() == 0);
+  }
+
   // Lookup a given channel id. Return -1 if not found.
   int Lookup(const std::string& channel_id) {
     AutoLock locked(lock_);
@@ -274,8 +279,8 @@ bool SocketWriteErrorIsRecoverable() {
 }  // namespace
 //------------------------------------------------------------------------------
 
-Channel::ChannelImpl::ChannelImpl(const std::string& channel_id, Mode mode,
-                                  Listener* listener)
+Channel::ChannelImpl::ChannelImpl(const IPC::ChannelHandle& channel_handle,
+                                  Mode mode, Listener* listener)
     : mode_(mode),
       is_blocked_on_write_(false),
       message_send_bytes_written_(0),
@@ -297,9 +302,9 @@ Channel::ChannelImpl::ChannelImpl(const std::string& channel_id, Mode mode,
   if (mode_ == MODE_NAMED_CLIENT)
     mode_ = MODE_CLIENT;
 
-  if (!CreatePipe(channel_id, mode_)) {
+  if (!CreatePipe(channel_handle, mode_)) {
     // The pipe may have been closed already.
-    PLOG(WARNING) << "Unable to create pipe named \"" << channel_id
+    LOG(WARNING) << "Unable to create pipe named \"" << channel_handle.name
                   << "\" in " << (mode_ == MODE_SERVER ? "server" : "client")
                   << " mode";
   }
@@ -349,16 +354,16 @@ bool SocketPair(int* fd1, int* fd2) {
   return true;
 }
 
-bool Channel::ChannelImpl::CreatePipe(const std::string& channel_id,
+bool Channel::ChannelImpl::CreatePipe(const IPC::ChannelHandle &channel_handle,
                                       Mode mode) {
   DCHECK(server_listen_pipe_ == -1 && pipe_ == -1);
-
+  pipe_name_ = channel_handle.name;
+  pipe_ = channel_handle.socket.fd;
   if (uses_fifo_) {
     // This only happens in unit tests; see the comment above PipeMap.
     // TODO(playmobil): We shouldn't need to create fifos on disk.
     // TODO(playmobil): If we do, they should be in the user data directory.
     // TODO(playmobil): Cleanup any stale fifos.
-    pipe_name_ = channel_id;
     if (mode == MODE_SERVER) {
       if (!CreateServerFifo(pipe_name_, &server_listen_pipe_)) {
         return false;
@@ -376,8 +381,9 @@ bool Channel::ChannelImpl::CreatePipe(const std::string& channel_id,
     // 2) It's the initial IPC channel:
     //   2a) Server side: create the pipe.
     //   2b) Client side: Pull the pipe out of the GlobalDescriptors set.
-    pipe_name_ = channel_id;
-    pipe_ = ChannelNameToFD(pipe_name_);
+    if (pipe_ < 0) {
+      pipe_ = ChannelNameToFD(pipe_name_);
+    }
     if (pipe_ < 0) {
       // Initial IPC channel.
       if (mode == MODE_SERVER) {
@@ -1065,9 +1071,9 @@ void Channel::ChannelImpl::Close() {
 
 //------------------------------------------------------------------------------
 // Channel's methods simply call through to ChannelImpl.
-Channel::Channel(const std::string& channel_id, Mode mode,
+Channel::Channel(const IPC::ChannelHandle& channel_handle, Mode mode,
                  Listener* listener)
-    : channel_impl_(new ChannelImpl(channel_id, mode, listener)) {
+    : channel_impl_(new ChannelImpl(channel_handle, mode, listener)) {
 }
 
 Channel::~Channel() {
diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
index b7818a2..77a993e 100644
--- a/ipc/ipc_channel_posix.h
+++ b/ipc/ipc_channel_posix.h
@@ -19,27 +19,13 @@
 
 namespace IPC {
 
-// Store that channel name |name| is available via socket |socket|.
-// Used when the channel has been precreated by another process on
-// our behalf and they've just shipped us the socket.
-void AddChannelSocket(const std::string& name, int socket);
-
-// Remove the channel name mapping, and close the corresponding socket.
-void RemoveAndCloseChannelSocket(const std::string& name);
-
-// Returns true if a channel named |name| is available.
-bool ChannelSocketExists(const std::string& name);
-
-// Construct a socket pair appropriate for IPC: UNIX domain, nonblocking.
-// Returns false on error.
-bool SocketPair(int* fd1, int* fd2);
-
 // An implementation of ChannelImpl for POSIX systems that works via
 // socketpairs.  See the .cc file for an overview of the implementation.
 class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
  public:
   // Mirror methods of Channel, see ipc_channel.h for description.
-  ChannelImpl(const std::string& channel_id, Mode mode, Listener* listener);
+  ChannelImpl(const IPC::ChannelHandle &channel_handle, Mode mode,
+              Listener* listener);
   ~ChannelImpl();
   bool Connect();
   void Close();
@@ -48,7 +34,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   int GetClientFileDescriptor() const;
 
  private:
-  bool CreatePipe(const std::string& channel_id, Mode mode);
+  bool CreatePipe(const IPC::ChannelHandle &channel_handle, Mode mode);
 
   bool ProcessIncomingMessages();
   bool ProcessOutgoingMessages();
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index 8450c28..fd5144c 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -65,16 +65,15 @@ ChannelProxy::Context::Context(Channel::Listener* listener,
     : listener_message_loop_(MessageLoop::current()),
       listener_(listener),
       ipc_message_loop_(ipc_message_loop),
-      channel_(NULL),
       peer_pid_(0),
       channel_connected_called_(false) {
 }
 
-void ChannelProxy::Context::CreateChannel(const std::string& id,
+void ChannelProxy::Context::CreateChannel(const IPC::ChannelHandle& handle,
                                           const Channel::Mode& mode) {
-  DCHECK(channel_ == NULL);
-  channel_id_ = id;
-  channel_ = new Channel(id, mode, this);
+  DCHECK(channel_.get() == NULL);
+  channel_id_ = handle.name;
+  channel_.reset(new Channel(handle, mode, this));
 }
 
 bool ChannelProxy::Context::TryFilters(const Message& message) {
@@ -154,14 +153,14 @@ void ChannelProxy::Context::OnChannelOpened() {
   }
 
   for (size_t i = 0; i < filters_.size(); ++i)
-    filters_[i]->OnFilterAdded(channel_);
+    filters_[i]->OnFilterAdded(channel_.get());
 }
 
 // Called on the IPC::Channel thread
 void ChannelProxy::Context::OnChannelClosed() {
   // It's okay for IPC::ChannelProxy::Close to be called more than once, which
   // would result in this branch being taken.
-  if (!channel_)
+  if (!channel_.get())
     return;
 
   for (size_t i = 0; i < filters_.size(); ++i) {
@@ -172,8 +171,7 @@ void ChannelProxy::Context::OnChannelClosed() {
   // We don't need the filters anymore.
   filters_.clear();
 
-  delete channel_;
-  channel_ = NULL;
+  channel_.reset();
 
   // Balance with the reference taken during startup.  This may result in
   // self-destruction.
@@ -182,7 +180,7 @@ void ChannelProxy::Context::OnChannelClosed() {
 
 // Called on the IPC::Channel thread
 void ChannelProxy::Context::OnSendMessage(Message* message) {
-  if (!channel_) {
+  if (!channel_.get()) {
     delete message;
     OnChannelClosed();
     return;
@@ -204,8 +202,8 @@ void ChannelProxy::Context::OnAddFilter() {
 
     // If the channel has already been created, then we need to send this
     // message so that the filter gets access to the Channel.
-    if (channel_)
-      filters[i]->OnFilterAdded(channel_);
+    if (channel_.get())
+      filters[i]->OnFilterAdded(channel_.get());
     // Ditto for the peer process id.
     if (peer_pid_)
       filters[i]->OnChannelConnected(peer_pid_);
@@ -278,38 +276,49 @@ void ChannelProxy::Context::OnDispatchError() {
 
 //-----------------------------------------------------------------------------
 
-ChannelProxy::ChannelProxy(const std::string& channel_id,
+ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
                            Channel::Mode mode,
                            Channel::Listener* listener,
                            MessageLoop* ipc_thread)
     : context_(new Context(listener, ipc_thread)) {
-  Init(channel_id, mode, ipc_thread, true);
+  Init(channel_handle, mode, ipc_thread, true);
 }
 
-ChannelProxy::ChannelProxy(const std::string& channel_id,
+ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
                            Channel::Mode mode,
                            MessageLoop* ipc_thread,
                            Context* context,
                            bool create_pipe_now)
     : context_(context) {
-  Init(channel_id, mode, ipc_thread, create_pipe_now);
+  Init(channel_handle, mode, ipc_thread, create_pipe_now);
 }
 
 ChannelProxy::~ChannelProxy() {
   Close();
 }
 
-void ChannelProxy::Init(const std::string& channel_id, Channel::Mode mode,
-                        MessageLoop* ipc_thread_loop, bool create_pipe_now) {
+void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
+                        Channel::Mode mode, MessageLoop* ipc_thread_loop,
+                        bool create_pipe_now) {
+#if defined(OS_POSIX)
+  // When we are creating a server on POSIX, we need its file descriptor
+  // to be created immediately so that it can be accessed and passed
+  // to other processes. Forcing it to be created immediately avoids
+  // race conditions that may otherwise arise.
+  if (mode == Channel::MODE_SERVER) {
+    create_pipe_now = true;
+  }
+#endif  // defined(OS_POSIX)
+
   if (create_pipe_now) {
     // Create the channel immediately.  This effectively sets up the
     // low-level pipe so that the client can connect.  Without creating
     // the pipe immediately, it is possible for a listener to attempt
     // to connect and get an error since the pipe doesn't exist yet.
-    context_->CreateChannel(channel_id, mode);
+    context_->CreateChannel(channel_handle, mode);
   } else {
     context_->ipc_message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-        context_.get(), &Context::CreateChannel, channel_id, mode));
+        context_.get(), &Context::CreateChannel, channel_handle, mode));
   }
 
   // complete initialization on the background thread
@@ -360,8 +369,9 @@ void ChannelProxy::ClearIPCMessageLoop() {
 // ChannelProxy::Init().
 // We assume that IPC::Channel::GetClientFileDescriptorMapping() is thread-safe.
 int ChannelProxy::GetClientFileDescriptor() const {
-  Channel *channel = context_.get()->channel_;
-  DCHECK(channel); // Channel must have been created first.
+  Channel *channel = context_.get()->channel_.get();
+  // Channel must have been created first.
+  DCHECK(channel) << context_.get()->channel_id_;
   return channel->GetClientFileDescriptor();
 }
 #endif
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index a85841d..a256ab6 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -10,7 +10,9 @@
 
 #include "base/lock.h"
 #include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "ipc/ipc_channel.h"
+#include "ipc/ipc_channel_handle.h"
 
 class MessageLoop;
 
@@ -97,7 +99,7 @@ class ChannelProxy : public Message::Sender {
     }
   };
 
-  // Initializes a channel proxy.  The channel_id and mode parameters are
+  // Initializes a channel proxy.  The channel_handle and mode parameters are
   // passed directly to the underlying IPC::Channel.  The listener is called on
   // the thread that creates the ChannelProxy.  The filter's OnMessageReceived
   // method is called on the thread where the IPC::Channel is running.  The
@@ -105,7 +107,7 @@ class ChannelProxy : public Message::Sender {
   // on the background thread.  Any message not handled by the filter will be
   // dispatched to the listener.  The given message loop indicates where the
   // IPC::Channel should be created.
-  ChannelProxy(const std::string& channel_id,
+  ChannelProxy(const IPC::ChannelHandle& channel_handle,
                Channel::Mode mode,
                Channel::Listener* listener,
                MessageLoop* ipc_thread_loop);
@@ -143,8 +145,6 @@ class ChannelProxy : public Message::Sender {
 
 #if defined(OS_POSIX)
   // Calls through to the underlying channel's methods.
-  // TODO(playmobil): For now this is only implemented in the case of
-  // create_pipe_now = true, we need to figure this out for the latter case.
   int GetClientFileDescriptor() const;
 #endif  // defined(OS_POSIX)
 
@@ -153,7 +153,7 @@ class ChannelProxy : public Message::Sender {
   // A subclass uses this constructor if it needs to add more information
   // to the internal state.  If create_pipe_now is true, the pipe is created
   // immediately.  Otherwise it's created on the IO thread.
-  ChannelProxy(const std::string& channel_id,
+  ChannelProxy(const IPC::ChannelHandle& channel_handle,
                Channel::Mode mode,
                MessageLoop* ipc_thread_loop,
                Context* context,
@@ -201,7 +201,8 @@ class ChannelProxy : public Message::Sender {
     friend class SendTask;
 
     // Create the Channel
-    void CreateChannel(const std::string& id, const Channel::Mode& mode);
+    void CreateChannel(const IPC::ChannelHandle& channel_handle,
+                       const Channel::Mode& mode);
 
     // Methods called on the IO thread.
     void OnSendMessage(Message* message_ptr);
@@ -219,7 +220,7 @@ class ChannelProxy : public Message::Sender {
     // List of filters.  This is only accessed on the IPC thread.
     std::vector<scoped_refptr<MessageFilter> > filters_;
     MessageLoop* ipc_message_loop_;
-    Channel* channel_;
+    scoped_ptr<Channel> channel_;
     std::string channel_id_;
     int peer_pid_;
     bool channel_connected_called_;
@@ -236,7 +237,7 @@ class ChannelProxy : public Message::Sender {
  private:
   friend class SendTask;
 
-  void Init(const std::string& channel_id, Channel::Mode mode,
+  void Init(const IPC::ChannelHandle& channel_handle, Channel::Mode mode,
             MessageLoop* ipc_thread_loop, bool create_pipe_now);
 
   // By maintaining this indirection (ref-counted) to our internal state, we
diff --git a/ipc/ipc_channel_win.cc b/ipc/ipc_channel_win.cc
index 31d1d185..28a880d 100644
--- a/ipc/ipc_channel_win.cc
+++ b/ipc/ipc_channel_win.cc
@@ -97,8 +97,8 @@ Channel::ChannelImpl::State::~State() {
 
 //------------------------------------------------------------------------------
 
-Channel::ChannelImpl::ChannelImpl(const std::string& channel_id, Mode mode,
-                                  Listener* listener)
+Channel::ChannelImpl::ChannelImpl(const IPC::ChannelHandle &channel_handle,
+                                  Mode mode, Listener* listener)
     : ALLOW_THIS_IN_INITIALIZER_LIST(input_state_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(output_state_(this)),
       pipe_(INVALID_HANDLE_VALUE),
@@ -106,9 +106,9 @@ Channel::ChannelImpl::ChannelImpl(const std::string& channel_id, Mode mode,
       waiting_connect_(mode == MODE_SERVER),
       processing_incoming_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
-  if (!CreatePipe(channel_id, mode)) {
+  if (!CreatePipe(channel_handle, mode)) {
     // The pipe may have been closed already.
-    LOG(WARNING) << "Unable to create pipe named \"" << channel_id <<
+    LOG(WARNING) << "Unable to create pipe named \"" << channel_handle.name <<
                     "\" in " << (mode == 0 ? "server" : "client") << " mode.";
   }
 }
@@ -175,10 +175,10 @@ const std::wstring Channel::ChannelImpl::PipeName(
   return ss.str();
 }
 
-bool Channel::ChannelImpl::CreatePipe(const std::string& channel_id,
+bool Channel::ChannelImpl::CreatePipe(const IPC::ChannelHandle &channel_handle,
                                       Mode mode) {
   DCHECK(pipe_ == INVALID_HANDLE_VALUE);
-  const std::wstring pipe_name = PipeName(channel_id);
+  const std::wstring pipe_name = PipeName(channel_handle.name);
   if (mode == MODE_SERVER) {
     SECURITY_ATTRIBUTES security_attributes = {0};
     security_attributes.bInheritHandle = FALSE;
@@ -469,9 +469,9 @@ void Channel::ChannelImpl::OnIOCompleted(MessageLoopForIO::IOContext* context,
 
 //------------------------------------------------------------------------------
 // Channel's methods simply call through to ChannelImpl.
-Channel::Channel(const std::string& channel_id, Mode mode,
+Channel::Channel(const IPC::ChannelHandle &channel_handle, Mode mode,
                  Listener* listener)
-    : channel_impl_(new ChannelImpl(channel_id, mode, listener)) {
+    : channel_impl_(new ChannelImpl(channel_handle, mode, listener)) {
 }
 
 Channel::~Channel() {
diff --git a/ipc/ipc_channel_win.h b/ipc/ipc_channel_win.h
index ccaa6f4..82b70db 100644
--- a/ipc/ipc_channel_win.h
+++ b/ipc/ipc_channel_win.h
@@ -21,7 +21,8 @@ namespace IPC {
 class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
  public:
   // Mirror methods of Channel, see ipc_channel.h for description.
-  ChannelImpl(const std::string& channel_id, Mode mode, Listener* listener);
+  ChannelImpl(const IPC::ChannelHandle &channel_handle, Mode mode,
+              Listener* listener);
   ~ChannelImpl();
   bool Connect();
   void Close();
@@ -29,7 +30,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
   bool Send(Message* message);
  private:
   const std::wstring PipeName(const std::string& channel_id) const;
-  bool CreatePipe(const std::string& channel_id, Mode mode);
+  bool CreatePipe(const IPC::ChannelHandle &channel_handle, Mode mode);
 
   bool ProcessConnection();
   bool ProcessIncomingMessages(MessageLoopForIO::IOContext* context,
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index e77846c..7425a9b 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -355,14 +355,14 @@ void SyncChannel::SyncContext::OnWaitableEventSignaled(WaitableEvent* event) {
 
 
 SyncChannel::SyncChannel(
-    const std::string& channel_id,
+    const IPC::ChannelHandle& channel_handle,
     Channel::Mode mode,
     Channel::Listener* listener,
     MessageLoop* ipc_message_loop,
     bool create_pipe_now,
     WaitableEvent* shutdown_event)
     : ChannelProxy(
-          channel_id, mode, ipc_message_loop,
+          channel_handle, mode, ipc_message_loop,
           new SyncContext(listener, ipc_message_loop, shutdown_event),
           create_pipe_now),
       sync_messages_with_no_timeout_allowed_(true) {
diff --git a/ipc/ipc_sync_channel.h b/ipc/ipc_sync_channel.h
index 3435042..fce2e38 100644
--- a/ipc/ipc_sync_channel.h
+++ b/ipc/ipc_sync_channel.h
@@ -13,6 +13,7 @@
 #include "base/lock.h"
 #include "base/ref_counted.h"
 #include "base/waitable_event_watcher.h"
+#include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "ipc/ipc_sync_message.h"
 
@@ -34,7 +35,7 @@ class MessageReplyDeserializer;
 class SyncChannel : public ChannelProxy,
                     public base::WaitableEventWatcher::Delegate {
  public:
-  SyncChannel(const std::string& channel_id,
+  SyncChannel(const IPC::ChannelHandle& channel_handle,
               Channel::Mode mode,
               Channel::Listener* listener,
               MessageLoop* ipc_message_loop,