Do not create SRPC channel for NaCl in non-SFI mode.

This is a clean-up CL.
We have switched from SRPC to Chrome IPC for loading initial nexe
for NaCL in non-SFI mode by r279069. So, we no longer need to
create SRPC channel.

BUG=333950
TEST=Ran browser_tests --gtest_filter=*NonSfi*:*NonSFI* locally and trybots.
CQ_EXTRA_TRYBOTS=tryserver.chromium:linux_rel_precise32

Review URL: https://codereview.chromium.org/350673004

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

2 files changed, 66 insertions, 67 deletions

diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc
index 728fff6..e174e83 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -448,31 +448,28 @@ void NaClProcessHost::Launch(
       delete this;
       return;
     }
-  }
-
-  // TODO(hidehiko): We no longer use imc socket channel for non-SFI mode.
-  // Do not create it.
+  } else {
+    // Rather than creating a socket pair in the renderer, and passing
+    // one side through the browser to sel_ldr, socket pairs are created
+    // in the browser and then passed to the renderer and sel_ldr.
+    //
+    // This is mainly for the benefit of Windows, where sockets cannot
+    // be passed in messages, but are copied via DuplicateHandle().
+    // This means the sandboxed renderer cannot send handles to the
+    // browser process.
 
-  // Rather than creating a socket pair in the renderer, and passing
-  // one side through the browser to sel_ldr, socket pairs are created
-  // in the browser and then passed to the renderer and sel_ldr.
-  //
-  // This is mainly for the benefit of Windows, where sockets cannot
-  // be passed in messages, but are copied via DuplicateHandle().
-  // This means the sandboxed renderer cannot send handles to the
-  // browser process.
-
-  NaClHandle pair[2];
-  // Create a connected socket
-  if (NaClSocketPair(pair) == -1) {
-    SendErrorToRenderer("NaClSocketPair() failed");
-    delete this;
-    return;
+    NaClHandle pair[2];
+    // Create a connected socket
+    if (NaClSocketPair(pair) == -1) {
+      SendErrorToRenderer("NaClSocketPair() failed");
+      delete this;
+      return;
+    }
+    internal_->socket_for_renderer = pair[0];
+    internal_->socket_for_sel_ldr = pair[1];
+    SetCloseOnExec(pair[0]);
+    SetCloseOnExec(pair[1]);
   }
-  internal_->socket_for_renderer = pair[0];
-  internal_->socket_for_sel_ldr = pair[1];
-  SetCloseOnExec(pair[0]);
-  SetCloseOnExec(pair[1]);
 
   // Launch the process
   if (!LaunchSelLdr()) {
@@ -829,6 +826,9 @@ bool NaClProcessHost::StartNaClExecution() {
     // constructor, it is not automatically handled in its destructor as RAII.
     params.nexe_file =
         base::FileDescriptor(nexe_file_.GetPlatformFile(), true);
+    // In non-SFI mode, we do not use SRPC. Make sure that the socketpair is
+    // not created.
+    DCHECK_EQ(internal_->socket_for_sel_ldr, NACL_INVALID_HANDLE);
 #endif
   } else {
     params.validation_cache_enabled = nacl_browser->ValidationCacheIsEnabled();
@@ -839,64 +839,65 @@ bool NaClProcessHost::StartNaClExecution() {
         NaClBrowser::GetDelegate()->URLMatchesDebugPatterns(manifest_url_);
     params.uses_irt = uses_irt_;
     params.enable_dyncode_syscalls = enable_dyncode_syscalls_;
-  }
-
-  const ChildProcessData& data = process_->GetData();
-  if (!ShareHandleToSelLdr(data.handle,
-                           internal_->socket_for_sel_ldr, true,
-                           &params.handles)) {
-    return false;
-  }
 
-  if (params.uses_irt) {
-    const base::File& irt_file = nacl_browser->IrtFile();
-    CHECK(irt_file.IsValid());
-    // Send over the IRT file handle.  We don't close our own copy!
-    if (!ShareHandleToSelLdr(data.handle, irt_file.GetPlatformFile(), false,
+    const ChildProcessData& data = process_->GetData();
+    if (!ShareHandleToSelLdr(data.handle,
+                             internal_->socket_for_sel_ldr, true,
                              &params.handles)) {
       return false;
     }
-  }
+
+    if (params.uses_irt) {
+      const base::File& irt_file = nacl_browser->IrtFile();
+      CHECK(irt_file.IsValid());
+      // Send over the IRT file handle.  We don't close our own copy!
+      if (!ShareHandleToSelLdr(data.handle, irt_file.GetPlatformFile(), false,
+                               &params.handles)) {
+        return false;
+      }
+    }
 
 #if defined(OS_MACOSX)
-  // For dynamic loading support, NaCl requires a file descriptor that
-  // was created in /tmp, since those created with shm_open() are not
-  // mappable with PROT_EXEC.  Rather than requiring an extra IPC
-  // round trip out of the sandbox, we create an FD here.
-  base::SharedMemory memory_buffer;
-  base::SharedMemoryCreateOptions options;
-  options.size = 1;
-  options.executable = true;
-  if (!memory_buffer.Create(options)) {
-    DLOG(ERROR) << "Failed to allocate memory buffer";
-    return false;
-  }
-  FileDescriptor memory_fd;
-  memory_fd.fd = dup(memory_buffer.handle().fd);
-  if (memory_fd.fd < 0) {
-    DLOG(ERROR) << "Failed to dup() a file descriptor";
-    return false;
-  }
-  memory_fd.auto_close = true;
-  params.handles.push_back(memory_fd);
+    // For dynamic loading support, NaCl requires a file descriptor that
+    // was created in /tmp, since those created with shm_open() are not
+    // mappable with PROT_EXEC.  Rather than requiring an extra IPC
+    // round trip out of the sandbox, we create an FD here.
+    base::SharedMemory memory_buffer;
+    base::SharedMemoryCreateOptions options;
+    options.size = 1;
+    options.executable = true;
+    if (!memory_buffer.Create(options)) {
+      DLOG(ERROR) << "Failed to allocate memory buffer";
+      return false;
+    }
+    FileDescriptor memory_fd;
+    memory_fd.fd = dup(memory_buffer.handle().fd);
+    if (memory_fd.fd < 0) {
+      DLOG(ERROR) << "Failed to dup() a file descriptor";
+      return false;
+    }
+    memory_fd.auto_close = true;
+    params.handles.push_back(memory_fd);
 #endif
 
 #if defined(OS_POSIX)
-  if (params.enable_debug_stub) {
-    net::SocketDescriptor server_bound_socket = GetDebugStubSocketHandle();
-    if (server_bound_socket != net::kInvalidSocket) {
-      params.debug_stub_server_bound_socket =
-          FileDescriptor(server_bound_socket, true);
+    if (params.enable_debug_stub) {
+      net::SocketDescriptor server_bound_socket = GetDebugStubSocketHandle();
+      if (server_bound_socket != net::kInvalidSocket) {
+        params.debug_stub_server_bound_socket =
+            FileDescriptor(server_bound_socket, true);
+      }
     }
-  }
 #endif
+  }
 
   // Here we are about to send the IPC, so release file descriptors to delegate
   // the ownership to the message.
   if (uses_nonsfi_mode_) {
     nexe_file_.TakePlatformFile();
+  } else {
+    internal_->socket_for_sel_ldr = NACL_INVALID_HANDLE;
   }
-  internal_->socket_for_sel_ldr = NACL_INVALID_HANDLE;
 
   process_->Send(new NaClProcessMsg_Start(params));
   return true;
diff --git a/components/nacl/loader/nacl_listener.cc b/components/nacl/loader/nacl_listener.cc
index 9658ab5..427dd8c1 100644
--- a/components/nacl/loader/nacl_listener.cc
+++ b/components/nacl/loader/nacl_listener.cc
@@ -470,9 +470,7 @@ void NaClListener::StartNonSfi(const nacl::NaClStartParams& params) {
   CHECK(params.debug_stub_server_bound_socket.fd == -1);
 
   CHECK(!params.uses_irt);
-  // TODO(hidehiko): Currently imc bootstrap handle is still sent to the
-  // plugin. Get rid of this.
-  CHECK(params.handles.size() == 1);
+  CHECK(params.handles.empty());
 
   CHECK(params.nexe_file != IPC::InvalidPlatformFileForTransit());
   nacl::nonsfi::MainStart(