Move the pnacl translation nexes fully off the main thread.

BUG=none
TEST=run_pnacl_example_browser_test
Review URL: https://chromiumcodereview.appspot.com/9138047

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

6 files changed, 34 insertions, 30 deletions

diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 1fe7ca9..6f1949d 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -650,11 +650,13 @@ void Plugin::ShutDownSubprocesses() {
 bool Plugin::LoadNaClModuleCommon(nacl::DescWrapper* wrapper,
                                   NaClSubprocess* subprocess,
                                   const Manifest* manifest,
+                                  bool should_report_uma,
                                   ErrorInfo* error_info,
                                   pp::CompletionCallback init_done_cb,
                                   pp::CompletionCallback crash_cb) {
   ServiceRuntime* new_service_runtime =
-      new(std::nothrow) ServiceRuntime(this, manifest, init_done_cb, crash_cb);
+      new ServiceRuntime(this, manifest, should_report_uma, init_done_cb,
+                         crash_cb);
   subprocess->set_service_runtime(new_service_runtime);
   PLUGIN_PRINTF(("Plugin::LoadNaClModuleCommon (service_runtime=%p)\n",
                  static_cast<void*>(new_service_runtime)));
@@ -731,7 +733,7 @@ bool Plugin::LoadNaClModule(nacl::DescWrapper* wrapper,
   // outlive the Plugin object, they will not be memory safe.
   ShutDownSubprocesses();
   if (!(LoadNaClModuleCommon(wrapper, &main_subprocess_, manifest_.get(),
-                             error_info, init_done_cb, crash_cb))) {
+                             true, error_info, init_done_cb, crash_cb))) {
     return false;
   }
   PLUGIN_PRINTF(("Plugin::LoadNaClModule (%s)\n",
@@ -761,8 +763,10 @@ NaClSubprocessId Plugin::LoadHelperNaClModule(nacl::DescWrapper* wrapper,
     return kInvalidNaClSubprocessId;
   }
 
+  // Do not report UMA stats for translator-related nexes.
+  // TODO(sehr): define new UMA stats for translator related nexe events.
   if (!(LoadNaClModuleCommon(wrapper, nacl_subprocess.get(), manifest,
-                             error_info,
+                             false, error_info,
                              pp::BlockUntilComplete(),
                              pp::BlockUntilComplete())
         // We need not wait for the init_done callback.  We can block
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h
index 94a1307..014f175 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin.h
@@ -1,5 +1,5 @@
 // -*- c++ -*-
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -382,6 +382,7 @@ class Plugin : public pp::InstancePrivate {
   bool LoadNaClModuleCommon(nacl::DescWrapper* wrapper,
                             NaClSubprocess* subprocess,
                             const Manifest* manifest,
+                            bool should_report_uma,
                             ErrorInfo* error_info,
                             pp::CompletionCallback init_done_cb,
                             pp::CompletionCallback crash_cb);
diff --git a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
index 27e9e85..ecea98b 100644
--- a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
+++ b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
@@ -372,8 +372,6 @@ PnaclCoordinator::PnaclCoordinator(
     const pp::CompletionCallback& translate_notify_callback)
   : plugin_(plugin),
     translate_notify_callback_(translate_notify_callback),
-    llc_subprocess_(NULL),
-    ld_subprocess_(NULL),
     subprocesses_should_die_(false),
     file_system_(new pp::FileSystem(plugin, PP_FILESYSTEMTYPE_LOCALTEMPORARY)),
     manifest_(new ExtensionManifest(plugin->url_util())),
@@ -526,19 +524,6 @@ void PnaclCoordinator::RunTranslate(int32_t pp_error) {
     return;
   }
   pexe_wrapper_.reset(plugin_->wrapper_factory()->MakeFileDesc(fd, O_RDONLY));
-  // It would really be nice if we could create subprocesses from other than
-  // the main thread.  Until we can, we create them both up front.
-  // TODO(sehr): allow creation of subrpocesses from other threads.
-  llc_subprocess_ = StartSubprocess(kLlcUrl, manifest_.get());
-  if (llc_subprocess_ == NULL) {
-    ReportPpapiError(PP_ERROR_FAILED);
-    return;
-  }
-  ld_subprocess_ = StartSubprocess(kLdUrl, ld_manifest_.get());
-  if (ld_subprocess_ == NULL) {
-    ReportPpapiError(PP_ERROR_FAILED);
-    return;
-  }
   // Invoke llc followed by ld off the main thread.  This allows use of
   // blocking RPCs that would otherwise block the JavaScript main thread.
   translate_done_cb_ =
@@ -580,11 +565,16 @@ void WINAPI PnaclCoordinator::DoTranslateThread(void* arg) {
   Plugin* plugin = coordinator->plugin_;
   BrowserInterface* browser_interface = plugin->browser_interface();
 
+  nacl::scoped_ptr<NaClSubprocess> llc_subprocess(
+      coordinator->StartSubprocess(kLlcUrl, coordinator->manifest_.get()));
+  if (llc_subprocess == NULL) {
+    coordinator->TranslateFailed("Compile process could not be created.");
+  }
   // Run LLC.
   SrpcParams params;
   nacl::DescWrapper* llc_out_file = coordinator->obj_file_->write_wrapper();
   if (!PnaclSrpcLib::InvokeSrpcMethod(browser_interface,
-                                      coordinator->llc_subprocess_,
+                                      llc_subprocess.get(),
                                       "RunWithDefaultCommandLine",
                                       "hh",
                                       &params,
@@ -600,16 +590,22 @@ void WINAPI PnaclCoordinator::DoTranslateThread(void* arg) {
                  " is_shared_library=%d, soname='%s', lib_dependencies='%s')\n",
                  arg, is_shared_library, soname.c_str(),
                  lib_dependencies.c_str()));
+  // Shut down the llc subprocess.
+  llc_subprocess.release();
   if (coordinator->SubprocessesShouldDie()) {
     PLUGIN_PRINTF((
         "PnaclCoordinator::DoTranslateThread: killed by coordinator.\n"));
     NaClThreadExit(1);
   }
-  NaClSubprocess* ld_subprocess = coordinator->ld_subprocess_;
+  nacl::scoped_ptr<NaClSubprocess> ld_subprocess(
+      coordinator->StartSubprocess(kLdUrl, coordinator->ld_manifest_.get()));
+  if (ld_subprocess == NULL) {
+    coordinator->TranslateFailed("Link process could not be created.");
+  }
   nacl::DescWrapper* ld_in_file = coordinator->obj_file_->read_wrapper();
   nacl::DescWrapper* ld_out_file = coordinator->nexe_file_->write_wrapper();
   if (!PnaclSrpcLib::InvokeSrpcMethod(browser_interface,
-                                      ld_subprocess,
+                                      ld_subprocess.get(),
                                       "RunWithDefaultCommandLine",
                                       "hhiCC",
                                       &params,
@@ -621,6 +617,8 @@ void WINAPI PnaclCoordinator::DoTranslateThread(void* arg) {
     coordinator->TranslateFailed("link failed.");
   }
   PLUGIN_PRINTF(("PnaclCoordinator: link (coordinator=%p) succeeded\n", arg));
+  // Shut down the ld subprocess.
+  ld_subprocess.release();
   if (coordinator->SubprocessesShouldDie()) {
     PLUGIN_PRINTF((
         "PnaclCoordinator::DoTranslateThread: killed by coordinator.\n"));
diff --git a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h
index 73a25c1..77eedda 100644
--- a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h
+++ b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h
@@ -246,12 +246,7 @@ class PnaclCoordinator {
   pp::CompletionCallbackFactory<PnaclCoordinator,
                                 PnaclRefCount> callback_factory_;
 
-  // Helper subprocesses loaded by the plugin (deleted by the plugin).
-  // A nacl sandbox running the llc nexe.
-  NaClSubprocess* llc_subprocess_;
-  // A nacl sandbox running the ld nexe.
-  NaClSubprocess* ld_subprocess_;
-  // True if the translation thread and subprocesses should exit.
+  // True if the translation thread and related subprocesses should exit.
   bool subprocesses_should_die_;
   // Used to guard and publish subprocesses_should_die_.
   struct NaClMutex subprocess_mu_;
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index bc9da60..230b45f 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -384,9 +384,11 @@ void PluginReverseInterface::ReportExitStatus(int exit_status) {
 
 ServiceRuntime::ServiceRuntime(Plugin* plugin,
                                const Manifest* manifest,
+                               bool should_report_uma,
                                pp::CompletionCallback init_done_cb,
                                pp::CompletionCallback crash_cb)
     : plugin_(plugin),
+      should_report_uma_(should_report_uma),
       browser_interface_(plugin->browser_interface()),
       reverse_service_(NULL),
       subprocess_(NULL),
@@ -495,7 +497,9 @@ bool ServiceRuntime::InitCommunication(nacl::DescWrapper* nacl_desc,
   }
   PLUGIN_PRINTF(("ServiceRuntime::InitCommunication (load_status=%d)\n",
                  load_status));
-  plugin_->ReportSelLdrLoadStatus(load_status);
+  if (should_report_uma_) {
+    plugin_->ReportSelLdrLoadStatus(load_status);
+  }
   if (LOAD_OK != load_status) {
     error_info->SetReport(
         ERROR_SEL_LDR_START_STATUS,
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index 4d0c74b..e8589a7 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -1,6 +1,6 @@
 /* -*- c++ -*- */
 /*
- * Copyright (c) 2011 The Chromium Authors. All rights reserved.
+ * Copyright (c) 2012 The Chromium Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
@@ -162,6 +162,7 @@ class ServiceRuntime {
   // Start method below.
   ServiceRuntime(Plugin* plugin,
                  const Manifest* manifest,
+                 bool should_report_uma,
                  pp::CompletionCallback init_done_cb,
                  pp::CompletionCallback crash_cb);
   // The destructor terminates the sel_ldr process.
@@ -197,6 +198,7 @@ class ServiceRuntime {
 
   NaClSrpcChannel command_channel_;
   Plugin* plugin_;
+  bool should_report_uma_;
   BrowserInterface* browser_interface_;
   nacl::ReverseService* reverse_service_;
   nacl::scoped_ptr<nacl::SelLdrLauncher> subprocess_;