PNaCl: Use Chrome IPC to talk to the linker process, instead of SRPC

This is the second-to-last remaining use of SRPC.

 * Change pnacl_translate_thread.cc to send its request using Chrome
   IPC instead of SRPC.  Similarly, change
   irt_pnacl_translator_link.cc to receive its request this way.

 * Add plumbing so that pnacl_translate_thread.cc can acquire a Chrome
   IPC channel object.  Change LaunchSelLdr() (in ppb_nacl_private.h)
   to return this channel, and plumb it through service_runtime.h too.

 * Extend nacl_message_scanner.cc to handle a couple of things our new
   IPC message needs:
    * File handles inside of std::vector<>s.
    * File handles inside of sync messages.  (Previously, only sync
      replies were handled.)

 * Change a BUILD.gn to account for #includes of
   content/public/common/sandbox_init.h and
   ppapi/proxy/{serialized_handle.h,ppapi_messages.h}.

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

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

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

9 files changed, 153 insertions, 46 deletions

diff --git a/components/nacl/renderer/plugin/BUILD.gn b/components/nacl/renderer/plugin/BUILD.gn
index f4d2a82..5c3d22e 100644
--- a/components/nacl/renderer/plugin/BUILD.gn
+++ b/components/nacl/renderer/plugin/BUILD.gn
@@ -21,6 +21,7 @@ source_set("nacl_trusted_plugin") {
 
   deps = [
     "//base",
+    "//content/public/common",
     "//media:shared_memory_support",
     "//native_client/src/shared/gio",
     "//native_client/src/shared/imc",
@@ -33,6 +34,7 @@ source_set("nacl_trusted_plugin") {
     "//ppapi/c",
     "//ppapi/cpp:objects",
     "//ppapi/cpp/private:internal_module",
+    "//ppapi/proxy:ipc",
   ]
 
   if (is_linux) {
diff --git a/components/nacl/renderer/plugin/pnacl_translate_thread.cc b/components/nacl/renderer/plugin/pnacl_translate_thread.cc
index af71b69..85a5b88 100644
--- a/components/nacl/renderer/plugin/pnacl_translate_thread.cc
+++ b/components/nacl/renderer/plugin/pnacl_translate_thread.cc
@@ -15,9 +15,12 @@
 #include "components/nacl/renderer/plugin/srpc_params.h"
 #include "components/nacl/renderer/plugin/temporary_file.h"
 #include "components/nacl/renderer/plugin/utility.h"
+#include "content/public/common/sandbox_init.h"
 #include "native_client/src/shared/platform/nacl_sync_raii.h"
 #include "native_client/src/trusted/desc/nacl_desc_wrapper.h"
+#include "ppapi/c/ppb_file_io.h"
 #include "ppapi/cpp/var.h"
+#include "ppapi/proxy/ppapi_messages.h"
 
 namespace plugin {
 namespace {
@@ -86,7 +89,8 @@ PnaclTranslateThread::PnaclTranslateThread()
       num_threads_(0),
       nexe_file_(NULL),
       coordinator_error_info_(NULL),
-      coordinator_(NULL) {
+      coordinator_(NULL),
+      ld_channel_peer_pid_(base::kNullProcessId) {
   NaClXMutexCtor(&subprocess_mu_);
   NaClXMutexCtor(&cond_mu_);
   NaClXCondVarCtor(&buffer_cond_);
@@ -126,6 +130,13 @@ void PnaclTranslateThread::RunCompile(
   DCHECK(compiler_subprocess_->service_runtime());
   compiler_subprocess_active_ = true;
 
+  // Free this IPC channel now to make sure that it does not get freed on
+  // the child thread when the child thread calls Shutdown().
+  // TODO(mseaborn): Convert DoCompile() to using Chrome IPC instead of SRPC,
+  // the same way DoLink() has been converted.  Then we will use this IPC
+  // channel instead of just freeing it here.
+  compiler_subprocess_->service_runtime()->TakeTranslatorChannel();
+
   compile_finished_callback_ = compile_finished_callback;
   translate_thread_.reset(new NaClThread);
   if (translate_thread_ == NULL) {
@@ -148,6 +159,17 @@ void PnaclTranslateThread::RunLink() {
   DCHECK(ld_subprocess_->service_runtime());
   ld_subprocess_active_ = true;
 
+  // Take ownership of this IPC channel to make sure that it does not get
+  // freed on the child thread when the child thread calls Shutdown().
+  ld_channel_ = ld_subprocess_->service_runtime()->TakeTranslatorChannel();
+  // ld_channel_ is a IPC::SyncChannel, which is not thread-safe and cannot be
+  // used directly by the child thread, so create a SyncMessageFilter which
+  // can be used by the child thread.
+  ld_channel_filter_ = ld_channel_->CreateSyncMessageFilter();
+  // Make a copy of the process ID, again to avoid any thread-safety issues
+  // involved in accessing ld_subprocess_ on the child thread.
+  ld_channel_peer_pid_ = ld_subprocess_->service_runtime()->get_process_id();
+
   // Tear down the previous thread.
   // TODO(jvoung): Use base/threading or something where we can have a
   // persistent thread and easily post tasks to that persistent thread.
@@ -186,6 +208,31 @@ void PnaclTranslateThread::EndStream() {
   NaClXMutexUnlock(&cond_mu_);
 }
 
+ppapi::proxy::SerializedHandle PnaclTranslateThread::GetHandleForSubprocess(
+    TempFile* file, int32_t open_flags) {
+  IPC::PlatformFileForTransit file_for_transit;
+
+#if defined(OS_WIN)
+  if (!content::BrokerDuplicateHandle(
+          file->GetFileHandle(),
+          ld_channel_peer_pid_,
+          &file_for_transit,
+          0,  // desired_access is 0 since we're using DUPLICATE_SAME_ACCESS.
+          DUPLICATE_SAME_ACCESS)) {
+    return ppapi::proxy::SerializedHandle();
+  }
+#else
+  file_for_transit = base::FileDescriptor(dup(file->GetFileHandle()), true);
+#endif
+
+  // Using 0 disables any use of quota enforcement for this file handle.
+  PP_Resource file_io = 0;
+
+  ppapi::proxy::SerializedHandle handle;
+  handle.set_file_handle(file_for_transit, open_flags, file_io);
+  return handle;
+}
+
 void WINAPI PnaclTranslateThread::DoCompileThread(void* arg) {
   PnaclTranslateThread* translator =
       reinterpret_cast<PnaclTranslateThread*>(arg);
@@ -348,60 +395,42 @@ void PnaclTranslateThread::DoLink() {
     // and now, just leave now.
     if (!ld_subprocess_active_)
       return;
-    // Now that we are in helper thread, we can do the the blocking
-    // StartSrpcServices operation.
-    if (!ld_subprocess_->StartSrpcServices()) {
-      TranslateFailed(
-          PP_NACL_ERROR_SRPC_CONNECTION_FAIL,
-          "SRPC connection failure for " + ld_subprocess_->description());
-      return;
-    }
   }
 
-  SrpcParams params;
-  std::vector<nacl::DescWrapper*> ld_in_files;
-  size_t i;
-  for (i = 0; i < obj_files_->size(); i++) {
-    // Reset object file for reading first.
-    if (!(*obj_files_)[i]->Reset()) {
+  // Reset object files for reading first.  We do this before duplicating
+  // handles/FDs to prevent any handle/FD leaks in case any of the Reset()
+  // calls fail.
+  for (TempFile* obj_file : *obj_files_) {
+    if (!obj_file->Reset()) {
       TranslateFailed(PP_NACL_ERROR_PNACL_LD_SETUP,
                       "Link process could not reset object file");
+      return;
     }
-    ld_in_files.push_back((*obj_files_)[i]->read_wrapper());
   }
-  for (; i < PnaclCoordinator::kMaxTranslatorObjectFiles; i++)
-    ld_in_files.push_back(invalid_desc_wrapper_);
 
-  nacl::DescWrapper* ld_out_file = nexe_file_->write_wrapper();
+  ppapi::proxy::SerializedHandle nexe_file =
+      GetHandleForSubprocess(nexe_file_, PP_FILEOPENFLAG_WRITE);
+  std::vector<ppapi::proxy::SerializedHandle> ld_input_files;
+  for (TempFile* obj_file : *obj_files_) {
+    ld_input_files.push_back(
+        GetHandleForSubprocess(obj_file, PP_FILEOPENFLAG_READ));
+  }
+
   int64_t link_start_time = NaClGetTimeOfDayMicroseconds();
-  // Run LD.
-  bool success = ld_subprocess_->InvokeSrpcMethod(
-      "RunWithSplit",
-      "ihhhhhhhhhhhhhhhhh",
-      &params,
-      static_cast<int>(obj_files_->size()),
-      ld_in_files[0]->desc(),
-      ld_in_files[1]->desc(),
-      ld_in_files[2]->desc(),
-      ld_in_files[3]->desc(),
-      ld_in_files[4]->desc(),
-      ld_in_files[5]->desc(),
-      ld_in_files[6]->desc(),
-      ld_in_files[7]->desc(),
-      ld_in_files[8]->desc(),
-      ld_in_files[9]->desc(),
-      ld_in_files[10]->desc(),
-      ld_in_files[11]->desc(),
-      ld_in_files[12]->desc(),
-      ld_in_files[13]->desc(),
-      ld_in_files[14]->desc(),
-      ld_in_files[15]->desc(),
-      ld_out_file->desc());
+  bool success = false;
+  bool sent = ld_channel_filter_->Send(
+      new PpapiMsg_PnaclTranslatorLink(ld_input_files, nexe_file, &success));
+  if (!sent) {
+    TranslateFailed(PP_NACL_ERROR_PNACL_LD_INTERNAL,
+                    "link failed: reply not received from linker.");
+    return;
+  }
   if (!success) {
     TranslateFailed(PP_NACL_ERROR_PNACL_LD_INTERNAL,
-                    "link failed.");
+                    "link failed: linker returned failure status.");
     return;
   }
+
   GetNaClInterface()->LogTranslateTime(
       "NaCl.Perf.PNaClLoadTime.LinkTime",
       NaClGetTimeOfDayMicroseconds() - link_start_time);
diff --git a/components/nacl/renderer/plugin/pnacl_translate_thread.h b/components/nacl/renderer/plugin/pnacl_translate_thread.h
index 7db7e7d..cbb0ed0 100644
--- a/components/nacl/renderer/plugin/pnacl_translate_thread.h
+++ b/components/nacl/renderer/plugin/pnacl_translate_thread.h
@@ -17,6 +17,7 @@
 #include "native_client/src/shared/platform/nacl_sync_checked.h"
 #include "native_client/src/shared/platform/nacl_threads.h"
 #include "ppapi/cpp/completion_callback.h"
+#include "ppapi/proxy/serialized_handle.h"
 
 struct PP_PNaClOptions;
 
@@ -84,6 +85,9 @@ class PnaclTranslateThread {
   bool started() const { return coordinator_ != NULL; }
 
  private:
+  ppapi::proxy::SerializedHandle GetHandleForSubprocess(TempFile* file,
+                                                        int32_t open_flags);
+
   // Helper thread entry point for compilation. Takes a pointer to
   // PnaclTranslateThread and calls DoCompile().
   static void WINAPI DoCompileThread(void* arg);
@@ -149,6 +153,15 @@ class PnaclTranslateThread {
   PP_PNaClOptions* pnacl_options_;
   std::string architecture_attributes_;
   PnaclCoordinator* coordinator_;
+
+  // This IPC::SyncChannel can only be used and freed by the parent thread.
+  scoped_ptr<IPC::SyncChannel> ld_channel_;
+  // This IPC::SyncMessageFilter can be used by the child thread.
+  scoped_refptr<IPC::SyncMessageFilter> ld_channel_filter_;
+  // PID of the subprocess, needed for copying handles to the subprocess on
+  // Windows.  This is used by the child thread.
+  base::ProcessId ld_channel_peer_pid_;
+
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(PnaclTranslateThread);
 };
diff --git a/components/nacl/renderer/plugin/service_runtime.cc b/components/nacl/renderer/plugin/service_runtime.cc
index 9c4e2b4..81479d3 100644
--- a/components/nacl/renderer/plugin/service_runtime.cc
+++ b/components/nacl/renderer/plugin/service_runtime.cc
@@ -38,7 +38,8 @@ ServiceRuntime::ServiceRuntime(Plugin* plugin,
       pp_instance_(pp_instance),
       main_service_runtime_(main_service_runtime),
       uses_nonsfi_mode_(uses_nonsfi_mode),
-      bootstrap_channel_(NACL_INVALID_HANDLE) {
+      bootstrap_channel_(NACL_INVALID_HANDLE),
+      process_id_(base::kNullProcessId) {
 }
 
 bool ServiceRuntime::SetupCommandChannel() {
@@ -91,6 +92,8 @@ void ServiceRuntime::StartSelLdr(const SelLdrStartParams& params,
       PP_FromBool(uses_nonsfi_mode_),
       params.process_type,
       &bootstrap_channel_,
+      &translator_channel_,
+      &process_id_,
       callback.pp_completion_callback());
   subprocess_.reset(tmp_subprocess.release());
 }
diff --git a/components/nacl/renderer/plugin/service_runtime.h b/components/nacl/renderer/plugin/service_runtime.h
index c3a659c..cc966a4 100644
--- a/components/nacl/renderer/plugin/service_runtime.h
+++ b/components/nacl/renderer/plugin/service_runtime.h
@@ -12,7 +12,10 @@
 #define COMPONENTS_NACL_RENDERER_PLUGIN_SERVICE_RUNTIME_H_
 
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/process/process_handle.h"
 #include "components/nacl/renderer/plugin/utility.h"
+#include "ipc/ipc_sync_channel.h"
 #include "native_client/src/include/nacl_macros.h"
 #include "native_client/src/include/nacl_scoped_ptr.h"
 #include "native_client/src/public/imc_types.h"
@@ -70,6 +73,14 @@ class ServiceRuntime {
 
   bool main_service_runtime() const { return main_service_runtime_; }
 
+  scoped_ptr<IPC::SyncChannel> TakeTranslatorChannel() {
+    return scoped_ptr<IPC::SyncChannel>(translator_channel_.release());
+  }
+
+  // Returns the PID of the subprocess.  This PID is needed for copying
+  // handles to the subprocess on Windows.
+  base::ProcessId get_process_id() { return process_id_; }
+
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(ServiceRuntime);
 
@@ -84,6 +95,9 @@ class ServiceRuntime {
   nacl::scoped_ptr<SelLdrLauncherChrome> subprocess_;
 
   NaClHandle bootstrap_channel_;
+
+  scoped_ptr<IPC::SyncChannel> translator_channel_;
+  base::ProcessId process_id_;
 };
 
 }  // namespace plugin
diff --git a/components/nacl/renderer/plugin/temporary_file.cc b/components/nacl/renderer/plugin/temporary_file.cc
index 10502ea..bd942c5 100644
--- a/components/nacl/renderer/plugin/temporary_file.cc
+++ b/components/nacl/renderer/plugin/temporary_file.cc
@@ -62,7 +62,13 @@ bool TempFile::Reset() {
 }
 
 PP_FileHandle TempFile::TakeFileHandle() {
+  DCHECK(file_handle_.IsValid());
   return file_handle_.TakePlatformFile();
 }
 
+PP_FileHandle TempFile::GetFileHandle() {
+  DCHECK(file_handle_.IsValid());
+  return file_handle_.GetPlatformFile();
+}
+
 }  // namespace plugin
diff --git a/components/nacl/renderer/plugin/temporary_file.h b/components/nacl/renderer/plugin/temporary_file.h
index 4941e98..4e4e9af 100644
--- a/components/nacl/renderer/plugin/temporary_file.h
+++ b/components/nacl/renderer/plugin/temporary_file.h
@@ -61,6 +61,11 @@ class TempFile {
   // valid after TakeFileHandle() is called.
   PP_FileHandle TakeFileHandle();
 
+  // Returns a handle to the file, without transferring ownership of it.
+  // This handle remains valid until the TempFile object is destroyed or
+  // TakeFileHandle() is called.
+  PP_FileHandle GetFileHandle();
+
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(TempFile);
 
diff --git a/components/nacl/renderer/ppb_nacl_private.h b/components/nacl/renderer/ppb_nacl_private.h
index 2683652..26ec98e 100644
--- a/components/nacl/renderer/ppb_nacl_private.h
+++ b/components/nacl/renderer/ppb_nacl_private.h
@@ -5,6 +5,9 @@
 #ifndef COMPONENTS_NACL_RENDERER_PPB_NACL_PRIVATE_H_
 #define COMPONENTS_NACL_RENDERER_PPB_NACL_PRIVATE_H_
 
+#include "base/memory/scoped_ptr.h"
+#include "base/process/process_handle.h"
+#include "ipc/ipc_sync_channel.h"
 #include "ppapi/c/pp_bool.h"
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/pp_instance.h"
@@ -214,6 +217,8 @@ struct PPB_NaCl_Private {
    * LaunchSelLdr takes the ownership of the file handle.
    * The |uses_nonsfi_mode| flag indicates whether or not nonsfi-mode should
    * be used with the binary pointed by the url.
+   * |translator_channel| and |process_id| are filled out when launching PNaCl
+   * translator processes.
    */
   void (*LaunchSelLdr)(PP_Instance instance,
                        PP_Bool main_service_runtime,
@@ -222,6 +227,8 @@ struct PPB_NaCl_Private {
                        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);
   /* On POSIX systems, this function returns the file descriptor of
    * /dev/urandom.  On non-POSIX systems, this function returns 0.
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 6cbdfe4..7f87ddc 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -389,6 +389,16 @@ NaClAppProcessType PP_ToNaClAppProcessType(
   return static_cast<NaClAppProcessType>(pp_process_type);
 }
 
+// A dummy IPC::Listener object with a no-op message handler.  We use
+// this with an IPC::SyncChannel where we only send synchronous
+// messages and don't need to handle any messages other than sync
+// replies.
+class NoOpListener : public IPC::Listener {
+ public:
+  bool OnMessageReceived(const IPC::Message& message) override { return false; }
+  void OnChannelError() override {}
+};
+
 // Launch NaCl's sel_ldr process.
 void LaunchSelLdr(PP_Instance instance,
                   PP_Bool main_service_runtime,
@@ -397,6 +407,8 @@ void LaunchSelLdr(PP_Instance instance,
                   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) {
   CHECK(ppapi::PpapiGlobals::Get()->GetMainThreadMessageLoop()->
             BelongsToCurrentThread());
@@ -516,8 +528,24 @@ void LaunchSelLdr(PP_Instance instance,
 
   // Don't save instance_info if channel handle is invalid.
   if (IsValidChannelHandle(instance_info.channel_handle)) {
-    NaClPluginInstance* nacl_plugin_instance = GetNaClPluginInstance(instance);
-    nacl_plugin_instance->instance_info.reset(new InstanceInfo(instance_info));
+    if (process_type == kPNaClTranslatorProcessType) {
+      // Return an IPC channel which allows communicating with a PNaCl
+      // translator process.
+      *translator_channel = IPC::SyncChannel::Create(
+          instance_info.channel_handle,
+          IPC::Channel::MODE_CLIENT,
+          new NoOpListener,
+          content::RenderThread::Get()->GetIOMessageLoopProxy(),
+          true,
+          content::RenderThread::Get()->GetShutdownEvent());
+      *process_id = launch_result.plugin_pid;
+    } else {
+      // Save the channel handle for when StartPpapiProxy() is called.
+      NaClPluginInstance* nacl_plugin_instance =
+          GetNaClPluginInstance(instance);
+      nacl_plugin_instance->instance_info.reset(
+          new InstanceInfo(instance_info));
+    }
   }
 
   *(static_cast<NaClHandle*>(imc_handle)) =