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

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

 // 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.
 
 #ifdef _MSC_VER
 // Do not warn about use of std::copy with raw pointers.
 #pragma warning(disable : 4996)
 #endif
 
 #include "native_client/src/trusted/plugin/plugin.h"
@@ skipping 596 matching lines @@
                  static_cast<void*>(new_service_runtime)));
   if (NULL == new_service_runtime) {
     error_info->SetReport(ERROR_SEL_LDR_INIT,
                           "sel_ldr init failure " + subprocess->description());
     return false;
   }
 
   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) {
     return false;
   }
 
   // Try to start the Chrome IPC-based proxy.
   if (nacl_interface_->StartPpapiProxy(pp_instance())) {
     using_ipc_proxy_ = true;
     // We need to explicitly schedule this here. It is normally called in
@@ skipping 511 matching lines @@
         static_cast<float>(ready_time_ - init_time_) / NACL_MICROS_PER_MILLI);
 
     ReportLoadSuccess(LENGTH_IS_COMPUTABLE, nexe_size_, nexe_size_);
   } else {
     NaClLog(4, "NexeFileDidOpenContinuation: failed.");
     ReportLoadError(error_info);
   }
   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);
 }
 
 void Plugin::CopyCrashLogToJsConsole() {
   nacl::string fatal_msg(main_service_runtime()->GetCrashLogOutput());
   size_t ix_start = 0;
   size_t ix_end;
 
@@ skipping 24 matching lines @@
     PLUGIN_PRINTF((("Plugin::NexeDidCrash: nexe exited with status %d"
                     " so this is a \"controlled crash\".\n"),
                    exit_status));
   }
   // If the crash occurs during load, we just want to report an error
   // that fits into our load progress event grammar.  If the crash
   // occurs after loaded/loadend, then we use ReportDeadNexe to send a
   // "crash" event.
   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;
+  } else {
+    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);
+    }
   }
 
-  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);
-  }
-
+  // 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();
 }
 
 void Plugin::BitcodeDidTranslate(int32_t pp_error) {
   PLUGIN_PRINTF(("Plugin::BitcodeDidTranslate (pp_error=%"NACL_PRId32")\n",
                  pp_error));
   if (pp_error != PP_OK) {
     // Error should have been reported by pnacl. Just return.
     PLUGIN_PRINTF(("Plugin::BitcodeDidTranslate error in Pnacl\n"));
     return;
@@ skipping 725 matching lines @@
                                           static_cast<uint32_t>(text.size()));
   const PPB_Console_Dev* console_interface =
       static_cast<const PPB_Console_Dev*>(
           module->GetBrowserInterface(PPB_CONSOLE_DEV_INTERFACE));
   console_interface->LogWithSource(pp_instance(), PP_LOGLEVEL_LOG, prefix, str);
   var_interface->Release(prefix);
   var_interface->Release(str);
 }
 
 }  // namespace plugin