Non-SFI: move socketpair() from plugin process to browser process.

components/nacl/loader/nonsfi/nonsfi_listener.cc

Index: components/nacl/loader/nonsfi/nonsfi_listener.cc
diff --git a/components/nacl/loader/nonsfi/nonsfi_listener.cc b/components/nacl/loader/nonsfi/nonsfi_listener.cc
index 5f212babe28493ceace4e5cca3420c6ea80fc720..7ed219a0865d082156224447570f918641bdd562 100644
--- a/components/nacl/loader/nonsfi/nonsfi_listener.cc
+++ b/components/nacl/loader/nonsfi/nonsfi_listener.cc
@@ -95,48 +95,33 @@ void NonSfiListener::OnStart(const nacl::NaClStartParams& params) {
     // the plugin binary), the FD needs to be passed to the hosts. So, here
     // we create raw FD pairs, and pass the client side FDs to the hosts,

[Mark Seaborn, 2015/04/15 20:52:05]

Update comment?  We're not creating the raw FD pairs here any more.

[hidehiko, 2015/05/12 18:47:51]

Done.

     // and the server side FDs to the plugin.
-    int browser_server_ppapi_fd;
-    int browser_client_ppapi_fd;
-    int renderer_server_ppapi_fd;
-    int renderer_client_ppapi_fd;
-    int manifest_service_server_fd;
-    int manifest_service_client_fd;
-    if (!IPC::SocketPair(
-            &browser_server_ppapi_fd, &browser_client_ppapi_fd) ||
-        !IPC::SocketPair(
-            &renderer_server_ppapi_fd, &renderer_client_ppapi_fd) ||
-        !IPC::SocketPair(
-            &manifest_service_server_fd, &manifest_service_client_fd)) {
-      LOG(ERROR) << "Failed to create sockets for IPC.";
-      return;
-    }
 
     // Set the plugin IPC channel FDs.
-    ppapi::SetIPCFileDescriptors(browser_server_ppapi_fd,
-                                 renderer_server_ppapi_fd,
-                                 manifest_service_server_fd);
+    ppapi::SetIPCFileDescriptors(params.browser_server_ppapi_fd.fd,
+                                 params.renderer_server_ppapi_fd.fd,
+                                 params.manifest_service_server_fd.fd);
     ppapi::StartUpPlugin();
 
     // Send back to the client side IPC channel FD to the host.
-    browser_handle.socket =
-        base::FileDescriptor(browser_client_ppapi_fd, true);
-    ppapi_renderer_handle.socket =
-        base::FileDescriptor(renderer_client_ppapi_fd, true);
-    manifest_service_handle.socket =
-        base::FileDescriptor(manifest_service_client_fd, true);
+    browser_handle.socket = params.browser_client_ppapi_fd;
+    ppapi_renderer_handle.socket = params.renderer_client_ppapi_fd;
+    manifest_service_handle.socket = params.manifest_service_client_fd;
   }
-
   // TODO(teravest): Do we plan on using this renderer handle for nexe loading
   // for non-SFI? Right now, passing an empty channel handle instead causes
   // hangs, so we'll keep it.
+  IPC::ChannelHandle trusted_channel_handle =
+      IPC::Channel::GenerateVerifiedChannelID("nacl");
+  trusted_channel_handle.socket = params.trusted_channel_server_fd;
   trusted_listener_ = new NaClTrustedListener(
-      IPC::Channel::GenerateVerifiedChannelID("nacl"),
+      trusted_channel_handle,
       io_thread_.message_loop_proxy().get(),
       &shutdown_event_);
+  trusted_channel_handle.socket = params.trusted_channel_client_fd;
   if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(

[Mark Seaborn, 2015/04/15 20:52:05]

Notice that you're receiving the client FDs from the browser process and
immediately sending them back to the browser process.  (See other comment...)

[Mark Seaborn, 2015/05/12 06:46:10]

This was the main feedback I had.  Maybe you could do this simplification and we
can get this change committed?

[hidehiko, 2015/05/12 18:47:51]

Done.

           browser_handle,
           ppapi_renderer_handle,
-          trusted_listener_->TakeClientChannelHandle(),
+          trusted_channel_handle,
           manifest_service_handle)))
     LOG(ERROR) << "Failed to send IPC channel handle to NaClProcessHost.";