Pepper: Remove ErrorInfo from SelLdrStartParams.

This cleans up the usage of ErrorInfo to shorten its scope in several places.
This will make life easier during the trusted plugin refactor, when
establishing the connection between the plugin and renderer has to be made
asynchronous.

This also makes SelLdrStartParams not have any members that are written to,
simplifying how the struct is used.

BUG=333950

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

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

3 files changed, 33 insertions, 34 deletions

diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index f7ee74b..d497fb3 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -293,16 +293,18 @@ bool Plugin::LoadNaClModuleCommon(nacl::DescWrapper* wrapper,
                                   const SelLdrStartParams& params,
                                   const pp::CompletionCallback& init_done_cb,
                                   const pp::CompletionCallback& crash_cb) {
+  ErrorInfo error_info;
   ServiceRuntime* new_service_runtime =
       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)));
-  if (NULL == new_service_runtime) {
-    params.error_info->SetReport(
+  if (should_report_uma && NULL == new_service_runtime) {
+    error_info.SetReport(
         ERROR_SEL_LDR_INIT,
         "sel_ldr init failure " + subprocess->description());
+    ReportLoadError(error_info);
     return false;
   }
 
@@ -328,14 +330,10 @@ bool Plugin::LoadNaClModuleCommon(nacl::DescWrapper* wrapper,
 
   // Now actually load the nexe, which can happen on a background thread.
   bool nexe_loaded = new_service_runtime->LoadNexeAndStart(wrapper,
-                                                           params.error_info,
                                                            crash_cb);
   PLUGIN_PRINTF(("Plugin::LoadNaClModuleCommon (nexe_loaded=%d)\n",
                  nexe_loaded));
-  if (!nexe_loaded) {
-    return false;
-  }
-  return true;
+  return nexe_loaded;
 }
 
 void Plugin::StartSelLdrOnMainThread(int32_t pp_error,
@@ -360,28 +358,24 @@ void Plugin::LoadNaClModule(nacl::DescWrapper* wrapper,
                             bool enable_crash_throttling,
                             const pp::CompletionCallback& init_done_cb,
                             const pp::CompletionCallback& crash_cb) {
-  ErrorInfo error_info;
   // Before forking a new sel_ldr process, ensure that we do not leak
   // the ServiceRuntime object for an existing subprocess, and that any
   // associated listener threads do not go unjoined because if they
   // outlive the Plugin object, they will not be memory safe.
   ShutDownSubprocesses();
   SelLdrStartParams params(manifest_base_url(),
-                           &error_info,
                            true /* uses_irt */,
                            true /* uses_ppapi */,
                            enable_dev_interfaces_,
                            enable_dyncode_syscalls,
                            enable_exception_handling,
                            enable_crash_throttling);
-  if (!LoadNaClModuleCommon(wrapper, &main_subprocess_, manifest_.get(),
-                            true /* should_report_uma */,
-                            params, init_done_cb, crash_cb)) {
-    ReportLoadError(error_info);
-    return;
+  if (LoadNaClModuleCommon(wrapper, &main_subprocess_, manifest_.get(),
+                           true /* should_report_uma */,
+                           params, init_done_cb, crash_cb)) {
+    PLUGIN_PRINTF(("Plugin::LoadNaClModule (%s)\n",
+                   main_subprocess_.detailed_description().c_str()));
   }
-  PLUGIN_PRINTF(("Plugin::LoadNaClModule (%s)\n",
-                 main_subprocess_.detailed_description().c_str()));
 }
 
 bool Plugin::LoadNaClModuleContinuationIntern(ErrorInfo* error_info) {
@@ -440,7 +434,6 @@ NaClSubprocess* Plugin::LoadHelperNaClModule(nacl::DescWrapper* wrapper,
   // TODO(jvoung): See if we still need the uses_ppapi variable, now that
   // LaunchSelLdr always happens on the main thread.
   SelLdrStartParams params(manifest_base_url(),
-                           error_info,
                            false /* uses_irt */,
                            false /* uses_ppapi */,
                            enable_dev_interfaces_,
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index 1e23013..9bc7b71 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -584,9 +584,13 @@ bool ServiceRuntime::StartSelLdr(const SelLdrStartParams& params) {
       tmp_subprocess(new SelLdrLauncherChrome());
   if (NULL == tmp_subprocess.get()) {
     NaClLog(LOG_ERROR, "ServiceRuntime::Start (subprocess create failed)\n");
-    params.error_info->SetReport(
-        ERROR_SEL_LDR_CREATE_LAUNCHER,
-        "ServiceRuntime: failed to create sel_ldr launcher");
+    if (main_service_runtime_) {
+      ErrorInfo error_info;
+      error_info.SetReport(
+          ERROR_SEL_LDR_CREATE_LAUNCHER,
+          "ServiceRuntime: failed to create sel_ldr launcher");
+      plugin_->ReportLoadError(error_info);
+    }
     return false;
   }
   nacl::string error_message;
@@ -601,10 +605,14 @@ bool ServiceRuntime::StartSelLdr(const SelLdrStartParams& params) {
                                        &error_message);
   if (!started) {
     NaClLog(LOG_ERROR, "ServiceRuntime::Start (start failed)\n");
-    params.error_info->SetReportWithConsoleOnlyError(
-        ERROR_SEL_LDR_LAUNCH,
-        "ServiceRuntime: failed to start",
-        error_message);
+    if (main_service_runtime_) {
+      ErrorInfo error_info;
+      error_info.SetReportWithConsoleOnlyError(
+          ERROR_SEL_LDR_LAUNCH,
+          "ServiceRuntime: failed to start",
+          error_message);
+      plugin_->ReportLoadError(error_info);
+    }
     return false;
   }
 
@@ -627,14 +635,17 @@ void ServiceRuntime::SignalStartSelLdrDone() {
 }
 
 bool ServiceRuntime::LoadNexeAndStart(nacl::DescWrapper* nacl_desc,
-                                      ErrorInfo* error_info,
                                       const pp::CompletionCallback& crash_cb) {
   NaClLog(4, "ServiceRuntime::LoadNexeAndStart (nacl_desc=%p)\n",
           reinterpret_cast<void*>(nacl_desc));
-  bool ok = LoadModule(nacl_desc, error_info) &&
-            InitReverseService(error_info) &&
-            StartModule(error_info);
+  ErrorInfo error_info;
+  bool ok = LoadModule(nacl_desc, &error_info) &&
+            InitReverseService(&error_info) &&
+            StartModule(&error_info);
   if (!ok) {
+    if (main_service_runtime_) {
+      plugin_->ReportLoadError(error_info);
+    }
     // On a load failure the service runtime does not crash itself to
     // avoid a race where the no-more-senders error on the reverse
     // channel esrvice thread might cause the crash-detection logic to
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index 80b496b..30200b8 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -49,7 +49,6 @@ class ServiceRuntime;
 // creation templates aren't overwhelmed with too many parameters.
 struct SelLdrStartParams {
   SelLdrStartParams(const nacl::string& url,
-                    ErrorInfo* error_info,
                     bool uses_irt,
                     bool uses_ppapi,
                     bool enable_dev_interfaces,
@@ -57,7 +56,6 @@ struct SelLdrStartParams {
                     bool enable_exception_handling,
                     bool enable_crash_throttling)
       : url(url),
-        error_info(error_info),
         uses_irt(uses_irt),
         uses_ppapi(uses_ppapi),
         enable_dev_interfaces(enable_dev_interfaces),
@@ -66,7 +64,6 @@ struct SelLdrStartParams {
         enable_crash_throttling(enable_crash_throttling) {
   }
   nacl::string url;
-  ErrorInfo* error_info;
   bool uses_irt;
   bool uses_ppapi;
   bool enable_dev_interfaces;
@@ -248,10 +245,8 @@ class ServiceRuntime {
 
   // Establish an SrpcClient to the sel_ldr instance and load the nexe.
   // The nexe to be started is passed through |nacl_file_desc|.
-  // On success, returns true. On failure, returns false and |error_string|
-  // is set to something describing the error.
+  // On success, returns true. On failure, returns false.
   bool LoadNexeAndStart(nacl::DescWrapper* nacl_file_desc,
-                        ErrorInfo* error_info,
                         const pp::CompletionCallback& crash_cb);
 
   // Starts the application channel to the nexe.