mac/linux: rework plugin channel file descriptor creation

This CL fixes a bug where the same renderer could open several channels to the same plugin process, which end up having the same name. This CL makes it that there is only one channel for each (plugin, renderer) pair, just like on Windows.
The socketpair is created in the plugin process (which can ensure that only one channel per renderer gets created), and sends the renderer side through the browser process.

Note: this should essentially be a noop on Windows.

Review URL: http://codereview.chromium.org/149062
Patch from Antoine Labour <piman@google.com>.

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

4 files changed, 59 insertions, 27 deletions

diff --git a/chrome/plugin/plugin_channel.cc b/chrome/plugin/plugin_channel.cc
index b9e7a52..6264af5 100644
--- a/chrome/plugin/plugin_channel.cc
+++ b/chrome/plugin/plugin_channel.cc
@@ -18,18 +18,11 @@
 #endif
 
 PluginChannel* PluginChannel::GetPluginChannel(
-    int process_id, MessageLoop* ipc_message_loop, int channel_fd) {
+    int process_id, MessageLoop* ipc_message_loop) {
   // map renderer's process id to a (single) channel to that process
   std::string channel_name = StringPrintf(
       "%d.r%d", base::GetCurrentProcId(), process_id);
 
-#if defined(OS_POSIX)
-  // If we were provided an already-open channel, associate it with
-  // the channel name in this process's name<->socket map.
-  if (channel_fd > 0)
-    IPC::AddChannelSocket(channel_name, channel_fd);
-#endif
-
   return static_cast<PluginChannel*>(PluginChannelBase::GetChannel(
       channel_name,
       IPC::Channel::MODE_SERVER,
@@ -38,8 +31,13 @@ PluginChannel* PluginChannel::GetPluginChannel(
       false));
 }
 
-PluginChannel::PluginChannel() : renderer_handle_(0), in_send_(0),
-                                 off_the_record_(false) {
+PluginChannel::PluginChannel()
+    : renderer_handle_(0),
+#if defined(OS_POSIX)
+      renderer_fd_(-1),
+#endif
+      in_send_(0),
+      off_the_record_(false) {
   SendUnblockingOnlyDuringDispatch();
   ChildProcess::current()->AddRefProcess();
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
@@ -49,6 +47,12 @@ PluginChannel::PluginChannel() : renderer_handle_(0), in_send_(0),
 PluginChannel::~PluginChannel() {
   if (renderer_handle_)
     base::CloseProcessHandle(renderer_handle_);
+#if defined(OS_POSIX)
+  // If we still have the FD, close it.
+  if (renderer_fd_ != -1) {
+    close(renderer_fd_);
+  }
+#endif
   ChildProcess::current()->ReleaseProcess();
 }
 
@@ -144,3 +148,16 @@ void PluginChannel::CleanUp() {
 
   plugin_stubs_.clear();
 }
+
+bool PluginChannel::Init(MessageLoop* ipc_message_loop, bool create_pipe_now) {
+#if defined(OS_POSIX)
+  // This gets called when the PluginChannel is initially created. At this
+  // point, create the socketpair and assign the plugin side FD to the channel
+  // name. Keep the renderer side FD as a member variable in the PluginChannel
+  // to be able to transmit it through IPC.
+  int plugin_fd;
+  IPC::SocketPair(&plugin_fd, &renderer_fd_);
+  IPC::AddChannelSocket(channel_name(), plugin_fd);
+#endif
+  return PluginChannelBase::Init(ipc_message_loop, create_pipe_now);
+}
diff --git a/chrome/plugin/plugin_channel.h b/chrome/plugin/plugin_channel.h
index 21373fb..02b619e 100644
--- a/chrome/plugin/plugin_channel.h
+++ b/chrome/plugin/plugin_channel.h
@@ -19,8 +19,7 @@ class PluginChannel : public PluginChannelBase {
   // POSIX only: If |channel_fd| > 0, use that file descriptor for the
   // channel socket.
   static PluginChannel* GetPluginChannel(int process_id,
-                                         MessageLoop* ipc_message_loop,
-                                         int channel_fd);
+                                         MessageLoop* ipc_message_loop);
 
   ~PluginChannel();
 
@@ -30,6 +29,18 @@ class PluginChannel : public PluginChannelBase {
   base::ProcessHandle renderer_handle() const { return renderer_handle_; }
   int GenerateRouteID();
 
+#if defined(OS_POSIX)
+  // When first created, the PluginChannel gets assigned the file descriptor
+  // for the renderer.
+  // After the first time we pass it through the IPC, we don't need it anymore,
+  // and we close it. At that time, we reset renderer_fd_ to -1.
+  int DisownRendererFd() {
+    int value = renderer_fd_;
+    renderer_fd_ = -1;
+    return value;
+  }
+#endif
+
   bool in_send() { return in_send_ != 0; }
 
   bool off_the_record() { return off_the_record_; }
@@ -42,6 +53,9 @@ class PluginChannel : public PluginChannelBase {
 
   virtual void CleanUp();
 
+  // Overrides PluginChannelBase::Init.
+  virtual bool Init(MessageLoop* ipc_message_loop, bool create_pipe_now);
+
  private:
   // Called on the plugin thread
   PluginChannel();
@@ -59,6 +73,12 @@ class PluginChannel : public PluginChannelBase {
   // Handle to the renderer process who is on the other side of the channel.
   base::ProcessHandle renderer_handle_;
 
+#if defined(OS_POSIX)
+  // FD for the renderer end of the pipe. It is stored until we send it over
+  // IPC after which it is cleared. It will be closed by the IPC mechanism.
+  int renderer_fd_;
+#endif
+
   int in_send_;  // Tracks if we're in a Send call.
   bool log_messages_;  // True if we should log sent and received messages.
   bool off_the_record_; // True if the renderer is in off the record mode.
diff --git a/chrome/plugin/plugin_thread.cc b/chrome/plugin/plugin_thread.cc
index 2f9c2ec..2701f5b 100644
--- a/chrome/plugin/plugin_thread.cc
+++ b/chrome/plugin/plugin_thread.cc
@@ -103,24 +103,23 @@ void PluginThread::CleanUp() {
 }
 
 void PluginThread::OnCreateChannel(
-#if defined(OS_POSIX)
-    base::FileDescriptor socket,
-#endif
     int process_id,
     bool off_the_record) {
-  int fd = -1;
-#if defined(OS_POSIX)
-  fd = socket.fd;
-#endif
-  std::string channel_name;
   scoped_refptr<PluginChannel> channel =
-      PluginChannel::GetPluginChannel(process_id, owner_loop(), fd);
+      PluginChannel::GetPluginChannel(process_id, owner_loop());
+  IPC::ChannelHandle channel_handle;
   if (channel.get()) {
-    channel_name = channel->channel_name();
+    channel_handle.name = channel->channel_name();
+#if defined(OS_POSIX)
+    // On POSIX, pass the renderer-side FD. Also mark it as auto-close so that
+    // it gets closed after it has been sent.
+    int renderer_fd = channel->DisownRendererFd();
+    channel_handle.socket = base::FileDescriptor(renderer_fd, true);
+#endif
     channel->set_off_the_record(off_the_record);
   }
 
-  Send(new PluginProcessHostMsg_ChannelCreated(channel_name));
+  Send(new PluginProcessHostMsg_ChannelCreated(channel_handle));
 }
 
 void PluginThread::OnPluginMessage(const std::vector<unsigned char> &data) {
diff --git a/chrome/plugin/plugin_thread.h b/chrome/plugin/plugin_thread.h
index 911940a..e9074aa 100644
--- a/chrome/plugin/plugin_thread.h
+++ b/chrome/plugin/plugin_thread.h
@@ -37,11 +37,7 @@ class PluginThread : public ChildThread {
   virtual void CleanUp();
 
   // Callback for when a channel has been created.
-  // On POSIX, |socket| is the channel endpoint socket.
   void OnCreateChannel(
-#if defined(OS_POSIX)
-      base::FileDescriptor socket,
-#endif
       int process_id,
       bool off_the_record);
   void OnPluginMessage(const std::vector<uint8> &data);