Ensure that errors that occur during load will get crash log info to JS console

ppapi/native_client/src/trusted/plugin/plugin.cc

Index: ppapi/native_client/src/trusted/plugin/plugin.cc
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 496f1357cec2ce4570a32ae3d657802c691c7145..bd03c5d9d4c86a3829a6956cce3269923d3e3dac 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -614,7 +614,8 @@ bool Plugin::LoadNaClModuleCommon(nacl::DescWrapper* wrapper,
   bool service_runtime_started =
       new_service_runtime->Start(wrapper,
                                  error_info,
-                                 manifest_base_url());
+                                 manifest_base_url(),
+                                 crash_cb);
   PLUGIN_PRINTF(("Plugin::LoadNaClModuleCommon (service_runtime_started=%d)\n",
                  service_runtime_started));
   if (!service_runtime_started) {
@@ -1146,7 +1147,7 @@ void Plugin::NexeFileDidOpenContinuation(int32_t pp_error) {
   NaClLog(4, "Leaving NexeFileDidOpenContinuation\n");
 }
 
-static void LogLineToConsole(Plugin* plugin, nacl::string one_line) {
+static void LogLineToConsole(Plugin* plugin, const nacl::string& one_line) {
   PLUGIN_PRINTF(("LogLineToConsole: %s\n",
                  one_line.c_str()));
   plugin->AddToConsole(one_line);
@@ -1192,18 +1193,21 @@ void Plugin::NexeDidCrash(int32_t pp_error) {
   if (nexe_error_reported()) {
     PLUGIN_PRINTF(("Plugin::NexeDidCrash: error already reported;"
                    " suppressing\n"));

[bbudge, 2012/07/25 20:42:27]

This will now CopyCrashLogToJsConsole() instead of return. Couldn't that now be
called multiple times?

[bsy, 2012/07/25 22:07:23]

yes, but that's safe.  i added to the comment just above the
CopyCrashLogToJsConsole below.

-    return;
-  }
-
-  if (nacl_ready_state() == DONE) {
-    ReportDeadNexe();
   } else {
-    ErrorInfo error_info;
-    error_info.SetReport(ERROR_START_PROXY_CRASH,  // Not quite right.
-                         "Nexe crashed during startup");
-    ReportLoadError(error_info);
+    if (nacl_ready_state() == DONE) {
+      ReportDeadNexe();
+    } else {
+      ErrorInfo error_info;
+      error_info.SetReport(ERROR_START_PROXY_CRASH,  // Not quite right.

[bbudge, 2012/07/25 20:42:27]

I realize this was here before, but can we improve this comment? I'm not sure
what this means.

[bsy, 2012/07/25 22:07:23]

Done.

+                           "Nexe crashed during startup");
+      ReportLoadError(error_info);
+    }
   }
 
+  // In all cases, try to grab the crash log.  The first error
+  // reported may have come from the start_module RPC reply indicating
+  // a validation error or something similar, which wouldn't grab the
+  // crash log.
   CopyCrashLogToJsConsole();
 }