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

ppapi/native_client/src/trusted/plugin/service_runtime.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.
  */
 
 #define NACL_LOG_MODULE_NAME "Plugin::ServiceRuntime"
 
 #include "native_client/src/trusted/plugin/service_runtime.h"
 
@@ skipping 570 matching lines @@
   PLUGIN_PRINTF(("ServiceRuntime: got 0x%"NACL_PRIxPTR"\n",
                  (uintptr_t) out_conn_cap));
   nacl::DescWrapper* conn_cap = plugin_->wrapper_factory()->MakeGenericCleanup(
       out_conn_cap);
   if (conn_cap == NULL) {
     error_info->SetReport(ERROR_SEL_LDR_COMMUNICATION_WRAPPER,
                           "ServiceRuntime: wrapper allocation failure");
     return false;
   }
   out_conn_cap = NULL;  // ownership passed
+  PLUGIN_PRINTF(("ServiceRuntime::InitCommunication"
+                 " starting reverse service\n"));
   reverse_service_ = new nacl::ReverseService(conn_cap, rev_interface_->Ref());
   if (!reverse_service_->Start()) {
     error_info->SetReport(ERROR_SEL_LDR_COMMUNICATION_REV_SERVICE,
                           "ServiceRuntime: starting reverse services failed");
     return false;
   }
 
   // start the module.  otherwise we cannot connect for multimedia
   // subsystem since that is handled by user-level code (not secure!)
   // in libsrpc.
@@ skipping 16 matching lines @@
   if (LOAD_OK != load_status) {
     error_info->SetReport(
         ERROR_SEL_LDR_START_STATUS,
         NaClErrorString(static_cast<NaClErrorCode>(load_status)));
     return false;
   }
   return true;
 }
 
 bool ServiceRuntime::Start(nacl::DescWrapper* nacl_desc,
-                           ErrorInfo* error_info, const nacl::string& url) {
+                           ErrorInfo* error_info, const nacl::string& url,
+                           pp::CompletionCallback crash_cb) {
   PLUGIN_PRINTF(("ServiceRuntime::Start (nacl_desc=%p)\n",
                  reinterpret_cast<void*>(nacl_desc)));
 
 #ifdef NACL_STANDALONE
   nacl::scoped_ptr<nacl::SelLdrLauncherStandalone>
       tmp_subprocess(new nacl::SelLdrLauncherStandalone());
 #else
   nacl::scoped_ptr<SelLdrLauncherChrome>
       tmp_subprocess(new SelLdrLauncherChrome());
 #endif
@@ skipping 10 matching lines @@
 #endif
   if (!started) {
     PLUGIN_PRINTF(("ServiceRuntime::Start (start failed)\n"));
     error_info->SetReport(ERROR_SEL_LDR_LAUNCH,
                           "ServiceRuntime: failed to start");
     return false;
   }
 
   subprocess_.reset(tmp_subprocess.release());
   if (!InitCommunication(nacl_desc, error_info)) {
-    subprocess_.reset(NULL);
+    // The subprocess_ member variable is needed to collect crash log
+    // output after the error is reported.  We induce a service
+    // runtime crash here, since on a load failure the service runtime
+    // does not crash itself to avoid the race where the
+    // no-more-senders error on the reverse channel service thread
+    // might cause the crash-detection logic to kick in before the
+    // start_module RPC reply has been received.

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

This comment is hard for me to understand. How about this?

// 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 service thread
// might cause the crash-detection logic to kick in before the start_module
// RPC reply has been received. So we induce a service runtime crash here.
// We do not release subprocess_ since it's needed to collect crash log
// output after the error is reported.

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

Done.

+    Log(LOG_FATAL, "reap logs");
+    if (NULL == reverse_service_) {
+      // No crash detector thread.
+      PLUGIN_PRINTF(("scheduling to get crash log\n"));
+      pp::Module::Get()->core()->CallOnMainThread(0, crash_cb, PP_OK);
+      PLUGIN_PRINTF(("should fire soon\n"));
+    } else {
+      PLUGIN_PRINTF(("Reverse service thread will pick up crash log\n"));
+    }
     return false;
   }
 
   PLUGIN_PRINTF(("ServiceRuntime::Start (return 1)\n"));
   return true;
 }
 
 SrpcClient* ServiceRuntime::SetupAppChannel() {
   PLUGIN_PRINTF(("ServiceRuntime::SetupAppChannel (subprocess_=%p)\n",
                  reinterpret_cast<void*>(subprocess_.get())));
@@ skipping 75 matching lines @@
 
 nacl::string ServiceRuntime::GetCrashLogOutput() {
   if (NULL != subprocess_.get()) {
     return subprocess_->GetCrashLogOutput();
   } else {
     return "";
   }
 }
 
 }  // namespace plugin