Add a load_status callback hook that is invoked at the end of chrome LoadApp.

src/trusted/service_runtime/sel_main_chrome.c

Index: src/trusted/service_runtime/sel_main_chrome.c
diff --git a/src/trusted/service_runtime/sel_main_chrome.c b/src/trusted/service_runtime/sel_main_chrome.c
index c4a013e802cdde0a7cab5e50c098101ee0fa0ff3..da0d6740cf03da96f03a31802f6d678e5b47912f 100644
--- a/src/trusted/service_runtime/sel_main_chrome.c
+++ b/src/trusted/service_runtime/sel_main_chrome.c
@@ -107,6 +107,7 @@ struct NaClChromeMainArgs *NaClChromeMainArgsCreate(void) {
   args->broker_duplicate_handle_func = NULL;
   args->attach_debug_exception_handler_func = NULL;
 #endif
+  args->load_status_handler_func = NULL;
 #if NACL_LINUX || NACL_OSX
   args->number_of_cores = -1;  /* unknown */
 #endif
@@ -371,17 +372,35 @@ static int LoadApp(struct NaClApp *nap, struct NaClChromeMainArgs *args) {
 #endif
   }
 
+  if (args->load_status_handler_func != NULL) {
+    args->load_status_handler_func(LOAD_OK);
+  }
   return LOAD_OK;
 
 done:
   fflush(stdout);
 
   /*
-   * If there is a secure command channel, we sent an RPC reply with
-   * the reason that the nexe was rejected.  If we exit now, that
-   * reply may still be in-flight and the various channel closure (esp
-   * reverse channel) may be detected first.  This would result in a
-   * crash being reported, rather than the error in the RPC reply.
+   * If there is a load status callback, call that now. TODO(jvoung): remove
+   * NaClBlockIfCommandChannelExists() and use the callback to indicate
+   * the load_status.

[Mark Seaborn, 2015/04/18 01:14:14]

Nit: "... after Chromium no longer calls start_module"?

[jvoung (off chromium), 2015/04/20 18:49:47]

Done.

Hmm there is still a race condition if I leave out the
NaClBlockIfCommandChannelExists actually.

What happens is if there is a load error, then the initialization state is set
to ERROR, which makes NaClWaitForStartModuleCommand return immediately. Then
this process reports the error to the renderer and the loader exits.

The renderer/plugin currently will get the load error and report it -- sending a
progress event. However it will continue to try and do "StartNexe" anyway,
because that part isn't yet blocked on knowing the initial load status before
attempting StartNexe. So it will start setting up the command channel, but that
will fail without NaClBlockIfCommandChannelExists because the loader has already
exited. Then it will try to report a second error / send a second progress
event, but that will fail/cause confusion because an error has already been
reported.

So would it make sense to keep this NaClBlockIfCommandChannelExists for now
(whether or not there is a callback). Do the chrome side change for the
callback's initial setup. Remove the NaClWaitForStartModuleCommand on the NaCl
side. Change the chrome side again to wait for a load status before doing
SetupCommand and also remove that side's start_module call?

+   */
+  if (args->load_status_handler_func != NULL) {
+    /* Don't return LOAD_OK if we had some failure loading. */
+    if (LOAD_OK == errcode) {
+      errcode = LOAD_INTERNAL;
+    }
+    args->load_status_handler_func(errcode);
+    NaClLog(LOG_ERROR, "reap logs\n");

[Mark Seaborn, 2015/04/18 01:14:14]

Can you put a less cryptic message here, please?

[jvoung (off chromium), 2015/04/20 18:49:47]

Done.

+    NaClLogRunAbortBehavior();
+    return errcode;
+  }
+  /*
+   * If there is no load status callback, but there is a secure command channel,
+   * we sent an RPC reply with the reason that the nexe was rejected.
+   * If we exit now, that reply may still be in-flight and the various
+   * channel closure (esp reverse channel) may be detected first.  This would
+   * result in a crash being reported, rather than the error in the RPC reply.
    * Instead, we wait for the hard-shutdown on the command channel.
    */
   if (LOAD_OK != errcode) {