Connect PPAPI IPC channels for non-SFI mode.

components/nacl/loader/nacl_listener.cc

Index: components/nacl/loader/nacl_listener.cc
diff --git a/components/nacl/loader/nacl_listener.cc b/components/nacl/loader/nacl_listener.cc
index 807d36764e49bfe4212de92aba1d050b973d1e46..6d00d0fef6f87a7c2328bd792c8a545edc78f05a 100644
--- a/components/nacl/loader/nacl_listener.cc
+++ b/components/nacl/loader/nacl_listener.cc
@@ -268,22 +268,67 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
     return;
   }
   if (params.enable_ipc_proxy) {
-    // Create the PPAPI IPC channels between the NaCl IRT and the hosts
-    // (browser/renderer) processes. The IRT uses these channels to communicate
-    // with the host and to initialize the IPC dispatchers.
-    IPC::ChannelHandle browser_handle =
-        IPC::Channel::GenerateVerifiedChannelID("nacl");
-    SetUpIPCAdapter(&browser_handle, io_thread_.message_loop_proxy(),
-                    nap, NACL_CHROME_DESC_BASE);
-
-    IPC::ChannelHandle renderer_handle =
-        IPC::Channel::GenerateVerifiedChannelID("nacl");
-    SetUpIPCAdapter(&renderer_handle, io_thread_.message_loop_proxy(),
-                    nap, NACL_CHROME_DESC_BASE + 1);
-
-    if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(
-            browser_handle, renderer_handle)))
-      LOG(ERROR) << "Failed to send IPC channel handle to NaClProcessHost.";
+#if defined(OS_LINUX)
+    if (params.enable_nonsfi_mode) {

[Mark Seaborn, 2014/02/07 23:54:10]

You should be able to use this new SocketPair()-based code path for all
OS_POSIX, not conditional on enable_nonsfi_mode.  This would help reduce the
number of conditionals and code paths to test.

[hidehiko, 2014/02/10 08:18:34]

Just for clarification:
Even if we use raw SocketPair approach for the OS_POSIX,
we still need to have "if (enable_nonsfi_mode) {...}" check somehow, because it
is necessary to set up NaClIPCAdapter for SFI-mode, but on the other hand it is
necessary to set global FD var for non-SFI-mode.
Also, we need to keep the current code path (or something similar) for OS_WIN,
so IMHO we couldn't simplify much the code around here, in terms of reducing the
condition?
WDYT?

This is just a working demo CL, so I'll address it in a new CL for the code
review.

[Mark Seaborn, 2014/02/10 18:57:35]

Oops, yes, I see what you mean.  Your current code is OK.

+      // In non-SFI mode, we neither intercept nor rewrite the message by
+      // IPCAdapter, and the channels are connected between the plugin and
+      // the hosts directly. So, the IPC::Channel instances will be created in
+      // the plugin side, because the IPC::Listener needs to be live on the
+      // plugin's main thread. However, on the initialization (i.e. before
+      // loading the plugin binary), the FD is needed to be passed to the
+      // hosts. So, here we create raw FD pairs, and pass the clients side FDs
+      // to the hosts, 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;
+      if (!IPC::SocketPair(
+              &browser_server_ppapi_fd, &browser_client_ppapi_fd) ||
+          !IPC::SocketPair(
+              &renderer_server_ppapi_fd, &renderer_client_ppapi_fd)) {
+        LOG(ERROR) << "Failed to create sockets for IPC.";
+        return;
+      }
+
+      // Set the plugin IPC channel FD.
+      nacl::nonsfi::SetIPCFileDescriptors(
+          browser_server_ppapi_fd, renderer_server_ppapi_fd);
+
+      // Send back to the client side IPC channel FD to the host.
+      IPC::ChannelHandle browser_handle =
+          IPC::Channel::GenerateVerifiedChannelID("nacl");
+      browser_handle.socket =
+          base::FileDescriptor(browser_client_ppapi_fd, true);
+
+      IPC::ChannelHandle renderer_handle =
+          IPC::Channel::GenerateVerifiedChannelID("nacl");
+      renderer_handle.socket =
+          base::FileDescriptor(renderer_client_ppapi_fd, true);
+
+      if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(
+              browser_handle, renderer_handle)))
+        LOG(ERROR) << "Failed to send IPC channel handle to NaClProcessHost.";
+    } else {
+#endif
+      // Create the PPAPI IPC channels between the NaCl IRT and the hosts
+      // (browser/renderer) processes. The IRT uses these channels to
+      // communicate with the host and to initialize the IPC dispatchers.
+      IPC::ChannelHandle browser_handle =
+          IPC::Channel::GenerateVerifiedChannelID("nacl");
+      SetUpIPCAdapter(&browser_handle, io_thread_.message_loop_proxy(),
+                      nap, NACL_CHROME_DESC_BASE);
+
+      IPC::ChannelHandle renderer_handle =
+          IPC::Channel::GenerateVerifiedChannelID("nacl");
+      SetUpIPCAdapter(&renderer_handle, io_thread_.message_loop_proxy(),
+                      nap, NACL_CHROME_DESC_BASE + 1);
+
+      if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(
+              browser_handle, renderer_handle)))
+        LOG(ERROR) << "Failed to send IPC channel handle to NaClProcessHost.";
+#if defined(OS_LINUX)
+    }
+#endif
   }
 
   std::vector<nacl::FileDescriptor> handles = params.handles;