NaCl cleanup: Remove use of the IMC bootstrap channel

This channel was used for establishing further NaCl IMC connections
across which we sent SRPC messages.  This is no longer needed now that
we don't use SRPC.

 * Stop creating the channel in the browser process
   (nacl_process_host.cc).

 * Remove the handle/FD fields from IPC messages.

 * Stop propagating the renderer-side handle through renderer code
   (service_runtime.cc and LaunchSelLdr()).

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

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

Cr-Commit-Position: refs/heads/master@{#369042}

12 files changed, 10 insertions, 103 deletions

diff --git a/components/nacl/browser/DEPS b/components/nacl/browser/DEPS
index 53ba192..efbdb131 100644
--- a/components/nacl/browser/DEPS
+++ b/components/nacl/browser/DEPS
@@ -3,7 +3,6 @@ include_rules = [
   "+content/public/browser",
   "+content/public/test",
   "+native_client/src/public",
-  "+native_client/src/shared/imc/nacl_imc_c.h",
   "+net",
   "+ppapi/host",
   "+ppapi/proxy",
diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc
index 41e337a..aa4f4c5 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -53,7 +53,6 @@
 #include "content/public/common/sandboxed_process_launcher_delegate.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_switches.h"
-#include "native_client/src/shared/imc/nacl_imc_c.h"
 #include "net/socket/socket_descriptor.h"
 #include "ppapi/host/host_factory.h"
 #include "ppapi/host/ppapi_host.h"
@@ -191,15 +190,6 @@ class NaClSandboxedProcessLauncherDelegate
 #endif  // OS_POSIX
 };
 
-void SetCloseOnExec(NaClHandle fd) {
-#if defined(OS_POSIX)
-  int flags = fcntl(fd, F_GETFD);
-  CHECK_NE(flags, -1);
-  int rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
-  CHECK_EQ(rc, 0);
-#endif
-}
-
 void CloseFile(base::File file) {
   // The base::File destructor will close the file for us.
 }
@@ -353,16 +343,10 @@ NaClProcessHost::~NaClProcessHost() {
         FROM_HERE, base::Bind(&CloseFile, base::Passed(std::move(file))));
   }
 #endif
-  base::File files_to_close[] = {
-      std::move(nexe_file_), std::move(socket_for_renderer_),
-      std::move(socket_for_sel_ldr_),
-  };
   // Open files need to be closed on the blocking pool.
-  for (auto& file : files_to_close) {
-    if (file.IsValid()) {
-      content::BrowserThread::GetBlockingPool()->PostTask(
-          FROM_HERE, base::Bind(&CloseFile, base::Passed(std::move(file))));
-    }
+  if (nexe_file_.IsValid()) {
+    content::BrowserThread::GetBlockingPool()->PostTask(
+        FROM_HERE, base::Bind(&CloseFile, base::Passed(std::move(nexe_file_))));
   }
 
   if (reply_msg_) {
@@ -492,27 +476,6 @@ void NaClProcessHost::Launch(
       delete this;
       return;
     }
-  } 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.
-
-    NaClHandle pair[2];
-    // Create a connected socket
-    if (NaClSocketPair(pair) == -1) {
-      SendErrorToRenderer("NaClSocketPair() failed");
-      delete this;
-      return;
-    }
-    socket_for_renderer_ = base::File(pair[0]);
-    socket_for_sel_ldr_ = base::File(pair[1]);
-    SetCloseOnExec(pair[0]);
-    SetCloseOnExec(pair[1]);
   }
 
   // Create a shared memory region that the renderer and plugin share for
@@ -746,18 +709,8 @@ void NaClProcessHost::ReplyToRenderer(
   }
 #endif
 
-  // First, create an |imc_channel_handle| for the renderer.
-  IPC::PlatformFileForTransit imc_handle_for_renderer =
-      IPC::TakeFileHandleForProcess(std::move(socket_for_renderer_),
-                                    nacl_host_message_filter_->PeerHandle());
-  if (imc_handle_for_renderer == IPC::InvalidPlatformFileForTransit()) {
-    // Failed to create the handle.
-    SendErrorToRenderer("imc_channel_handle creation failed.");
-    return;
-  }
-
-  // Hereafter, we always send an IPC message with handles including imc_handle
-  // created above which, on Windows, are not closable in this process.
+  // Hereafter, we always send an IPC message with handles created above
+  // which, on Windows, are not closable in this process.
   std::string error_message;
   base::SharedMemoryHandle crash_info_shmem_renderer_handle;
   if (!crash_info_shmem_.ShareToProcess(nacl_host_message_filter_->PeerHandle(),
@@ -773,8 +726,7 @@ void NaClProcessHost::ReplyToRenderer(
 
   const ChildProcessData& data = process_->GetData();
   SendMessageToRenderer(
-      NaClLaunchResult(imc_handle_for_renderer,
-                       ppapi_channel_handle.release(),
+      NaClLaunchResult(ppapi_channel_handle.release(),
                        trusted_channel_handle.release(),
                        manifest_service_channel_handle.release(),
                        base::GetProcId(data.handle),
@@ -897,11 +849,6 @@ bool NaClProcessHost::StartNaClExecution() {
       NaClBrowser::GetDelegate()->URLMatchesDebugPatterns(manifest_url_);
   if (uses_nonsfi_mode_) {
     // Currently, non-SFI mode is supported only on Linux.
-#if defined(OS_LINUX)
-    // In non-SFI mode, we do not use SRPC. Make sure that the socketpair is
-    // not created.
-    DCHECK(!socket_for_sel_ldr_.IsValid());
-#endif
     if (enable_nacl_debug) {
       base::ProcessId pid = base::GetProcId(process_->GetData().handle);
       LOG(WARNING) << "nonsfi nacl plugin running in " << pid;
@@ -913,12 +860,6 @@ bool NaClProcessHost::StartNaClExecution() {
     params.enable_debug_stub = enable_nacl_debug;
 
     const ChildProcessData& data = process_->GetData();
-    params.imc_bootstrap_handle = IPC::TakeFileHandleForProcess(
-        std::move(socket_for_sel_ldr_), data.handle);
-    if (params.imc_bootstrap_handle == IPC::InvalidPlatformFileForTransit()) {
-      return false;
-    }
-
     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!
diff --git a/components/nacl/browser/nacl_process_host.h b/components/nacl/browser/nacl_process_host.h
index 88d2978..aec4918 100644
--- a/components/nacl/browser/nacl_process_host.h
+++ b/components/nacl/browser/nacl_process_host.h
@@ -260,9 +260,6 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   // reporting crash information.
   base::SharedMemory crash_info_shmem_;
 
-  base::File socket_for_renderer_;
-  base::File socket_for_sel_ldr_;
-
   base::WeakPtrFactory<NaClProcessHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NaClProcessHost);
diff --git a/components/nacl/common/nacl_host_messages.h b/components/nacl/common/nacl_host_messages.h
index d3b419f..2e4d829 100644
--- a/components/nacl/common/nacl_host_messages.h
+++ b/components/nacl/common/nacl_host_messages.h
@@ -39,7 +39,6 @@ IPC_STRUCT_TRAITS_BEGIN(nacl::NaClLaunchParams)
 IPC_STRUCT_TRAITS_END()
 
 IPC_STRUCT_TRAITS_BEGIN(nacl::NaClLaunchResult)
-  IPC_STRUCT_TRAITS_MEMBER(imc_channel_handle)
   IPC_STRUCT_TRAITS_MEMBER(ppapi_ipc_channel_handle)
   IPC_STRUCT_TRAITS_MEMBER(trusted_ipc_channel_handle)
   IPC_STRUCT_TRAITS_MEMBER(manifest_service_ipc_channel_handle)
diff --git a/components/nacl/common/nacl_messages.h b/components/nacl/common/nacl_messages.h
index 9598d0d..42188e6 100644
--- a/components/nacl/common/nacl_messages.h
+++ b/components/nacl/common/nacl_messages.h
@@ -21,7 +21,6 @@
 IPC_STRUCT_TRAITS_BEGIN(nacl::NaClStartParams)
   IPC_STRUCT_TRAITS_MEMBER(nexe_file)
   IPC_STRUCT_TRAITS_MEMBER(nexe_file_path_metadata)
-  IPC_STRUCT_TRAITS_MEMBER(imc_bootstrap_handle)
   IPC_STRUCT_TRAITS_MEMBER(irt_handle)
 #if defined(OS_MACOSX)
   IPC_STRUCT_TRAITS_MEMBER(mac_shm_fd)
diff --git a/components/nacl/common/nacl_types.cc b/components/nacl/common/nacl_types.cc
index 8957a8e..996d252 100644
--- a/components/nacl/common/nacl_types.cc
+++ b/components/nacl/common/nacl_types.cc
@@ -10,7 +10,6 @@ namespace nacl {
 
 NaClStartParams::NaClStartParams()
     : nexe_file(IPC::InvalidPlatformFileForTransit()),
-      imc_bootstrap_handle(IPC::InvalidPlatformFileForTransit()),
       irt_handle(IPC::InvalidPlatformFileForTransit()),
 #if defined(OS_MACOSX)
       mac_shm_fd(IPC::InvalidPlatformFileForTransit()),
@@ -88,8 +87,7 @@ NaClLaunchParams::~NaClLaunchParams() {
 }
 
 NaClLaunchResult::NaClLaunchResult()
-    : imc_channel_handle(IPC::InvalidPlatformFileForTransit()),
-      ppapi_ipc_channel_handle(),
+    : ppapi_ipc_channel_handle(),
       trusted_ipc_channel_handle(),
       plugin_pid(base::kNullProcessId),
       plugin_child_id(0),
@@ -97,15 +95,13 @@ NaClLaunchResult::NaClLaunchResult()
 }
 
 NaClLaunchResult::NaClLaunchResult(
-    const IPC::PlatformFileForTransit& imc_channel_handle,
     const IPC::ChannelHandle& ppapi_ipc_channel_handle,
     const IPC::ChannelHandle& trusted_ipc_channel_handle,
     const IPC::ChannelHandle& manifest_service_ipc_channel_handle,
     base::ProcessId plugin_pid,
     int plugin_child_id,
     base::SharedMemoryHandle crash_info_shmem_handle)
-    : imc_channel_handle(imc_channel_handle),
-      ppapi_ipc_channel_handle(ppapi_ipc_channel_handle),
+    : ppapi_ipc_channel_handle(ppapi_ipc_channel_handle),
       trusted_ipc_channel_handle(trusted_ipc_channel_handle),
       manifest_service_ipc_channel_handle(manifest_service_ipc_channel_handle),
       plugin_pid(plugin_pid),
diff --git a/components/nacl/common/nacl_types.h b/components/nacl/common/nacl_types.h
index 6146b36..b904879 100644
--- a/components/nacl/common/nacl_types.h
+++ b/components/nacl/common/nacl_types.h
@@ -73,7 +73,6 @@ struct NaClStartParams {
   // Used only as a key for validation caching.
   base::FilePath nexe_file_path_metadata;
 
-  IPC::PlatformFileForTransit imc_bootstrap_handle;
   IPC::PlatformFileForTransit irt_handle;
 #if defined(OS_MACOSX)
   IPC::PlatformFileForTransit mac_shm_fd;
@@ -150,7 +149,6 @@ struct NaClLaunchParams {
 struct NaClLaunchResult {
   NaClLaunchResult();
   NaClLaunchResult(
-      const IPC::PlatformFileForTransit& imc_channel_handle,
       const IPC::ChannelHandle& ppapi_ipc_channel_handle,
       const IPC::ChannelHandle& trusted_ipc_channel_handle,
       const IPC::ChannelHandle& manifest_service_ipc_channel_handle,
@@ -159,9 +157,6 @@ struct NaClLaunchResult {
       base::SharedMemoryHandle crash_info_shmem_handle);
   ~NaClLaunchResult();
 
-  // For plugin loader <-> renderer IMC communication.
-  IPC::PlatformFileForTransit imc_channel_handle;
-
   // For plugin <-> renderer PPAPI communication.
   IPC::ChannelHandle ppapi_ipc_channel_handle;
 
diff --git a/components/nacl/loader/nonsfi/nonsfi_listener.cc b/components/nacl/loader/nonsfi/nonsfi_listener.cc
index 27d322e..2352fee 100644
--- a/components/nacl/loader/nonsfi/nonsfi_listener.cc
+++ b/components/nacl/loader/nonsfi/nonsfi_listener.cc
@@ -112,7 +112,6 @@ void NonSfiListener::OnStart(const nacl::NaClStartParams& params) {
   CHECK(!params.enable_debug_stub);
   CHECK(params.debug_stub_server_bound_socket.fd == -1);
 
-  CHECK(params.imc_bootstrap_handle == IPC::InvalidPlatformFileForTransit());
   CHECK(params.irt_handle == IPC::InvalidPlatformFileForTransit());
   CHECK(params.debug_stub_server_bound_socket ==
         IPC::InvalidPlatformFileForTransit());
diff --git a/components/nacl/renderer/DEPS b/components/nacl/renderer/DEPS
index 72eb985..e4e3368 100644
--- a/components/nacl/renderer/DEPS
+++ b/components/nacl/renderer/DEPS
@@ -1,6 +1,5 @@
 include_rules = [
   "+content/public/renderer",
-  "+native_client/src/public/imc_types.h",  # for NaClHandle
   # for NaClErrorCode
   "+native_client/src/trusted/service_runtime/nacl_error_code.h",
   "+net",
diff --git a/components/nacl/renderer/plugin/service_runtime.cc b/components/nacl/renderer/plugin/service_runtime.cc
index b8a9d91..f0f78c1 100644
--- a/components/nacl/renderer/plugin/service_runtime.cc
+++ b/components/nacl/renderer/plugin/service_runtime.cc
@@ -11,7 +11,6 @@
 #include <utility>
 
 #include "base/compiler_specific.h"
-#include "base/files/file.h"
 #include "base/logging.h"
 #include "components/nacl/renderer/plugin/plugin.h"
 #include "components/nacl/renderer/plugin/plugin_error.h"
@@ -21,7 +20,6 @@
 #include "native_client/src/include/nacl_scoped_ptr.h"
 #include "native_client/src/include/portability_io.h"
 #include "native_client/src/include/portability_string.h"
-#include "native_client/src/public/imc_types.h"
 #include "native_client/src/trusted/nonnacl_util/sel_ldr_launcher.h"
 #include "native_client/src/trusted/service_runtime/nacl_error_code.h"
 #include "ppapi/c/pp_errors.h"
@@ -56,7 +54,6 @@ void ServiceRuntime::StartSelLdr(const SelLdrStartParams& params,
     return;
   }
 
-  NaClHandle bootstrap_channel;
   GetNaClInterface()->LaunchSelLdr(
       pp_instance_,
       PP_FromBool(main_service_runtime_),
@@ -64,15 +61,10 @@ void ServiceRuntime::StartSelLdr(const SelLdrStartParams& params,
       &params.file_info,
       PP_FromBool(uses_nonsfi_mode_),
       params.process_type,
-      &bootstrap_channel,
       &translator_channel_,
       &process_id_,
       callback.pp_completion_callback());
   subprocess_.reset(tmp_subprocess.release());
-
-  // TODO(mseaborn): Remove the bootstrap channel entirely, since we no
-  // longer use it.  For now, ensure this handle/FD does not get leaked.
-  base::File closer(bootstrap_channel);
 }
 
 void ServiceRuntime::ReportLoadError(const ErrorInfo& error_info) {
diff --git a/components/nacl/renderer/ppb_nacl_private.h b/components/nacl/renderer/ppb_nacl_private.h
index 11484dbb..c44c62a 100644
--- a/components/nacl/renderer/ppb_nacl_private.h
+++ b/components/nacl/renderer/ppb_nacl_private.h
@@ -209,9 +209,8 @@ struct PP_NaClFileInfo {
  * @{
  */
 struct PPB_NaCl_Private {
-  /* Launches NaCl's sel_ldr process.  Returns PP_EXTERNAL_PLUGIN_OK on success
-   * and writes a NaClHandle to imc_handle. Returns PP_EXTERNAL_PLUGIN_FAILED on
-   * failure.
+  /* Launches NaCl's sel_ldr process.  Returns PP_EXTERNAL_PLUGIN_OK on success.
+   * Returns PP_EXTERNAL_PLUGIN_FAILED on failure.
    * The |nexe_file_info| is currently used only in non-SFI mode. It is the
    * file handle for the main nexe file, which should be initially loaded.
    * LaunchSelLdr takes the ownership of the file handle.
@@ -226,7 +225,6 @@ struct PPB_NaCl_Private {
                        const struct PP_NaClFileInfo* nexe_file_info,
                        PP_Bool uses_nonsfi_mode,
                        PP_NaClAppProcessType process_type,
-                       void* imc_handle,
                        scoped_ptr<IPC::SyncChannel>* translator_channel,
                        base::ProcessId* process_id,
                        struct PP_CompletionCallback callback);
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index b86f560..1a40235 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -49,7 +49,6 @@
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/renderer_ppapi_host.h"
-#include "native_client/src/public/imc_types.h"
 #include "net/base/data_url.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_util.h"
@@ -405,7 +404,6 @@ void LaunchSelLdr(PP_Instance instance,
                   const PP_NaClFileInfo* nexe_file_info,
                   PP_Bool uses_nonsfi_mode,
                   PP_NaClAppProcessType pp_process_type,
-                  void* imc_handle,
                   scoped_ptr<IPC::SyncChannel>* translator_channel,
                   base::ProcessId* process_id,
                   PP_CompletionCallback callback) {
@@ -506,7 +504,6 @@ void LaunchSelLdr(PP_Instance instance,
     // Even on error, some FDs/handles may be passed to here.
     // We must release those resources.
     // See also nacl_process_host.cc.
-    IPC::PlatformFileForTransitToFile(launch_result.imc_channel_handle);
     base::SharedMemory::CloseHandle(launch_result.crash_info_shmem_handle);
 
     if (PP_ToBool(main_service_runtime)) {
@@ -547,10 +544,6 @@ void LaunchSelLdr(PP_Instance instance,
     }
   }
 
-  *(static_cast<NaClHandle*>(imc_handle)) =
-      IPC::PlatformFileForTransitToPlatformFile(
-          launch_result.imc_channel_handle);
-
   // Store the crash information shared memory handle.
   load_manager->set_crash_info_shmem_handle(
       launch_result.crash_info_shmem_handle);