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

src/trusted/service_runtime/sel_main_chrome.c

 /*
  * Copyright (c) 2012 The Native Client Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "native_client/src/public/chrome_main.h"
 
 #include "native_client/src/include/build_config.h"
 #include "native_client/src/include/portability.h"
@@ skipping 89 matching lines @@
 #endif
 #if NACL_WINDOWS
   args->debug_stub_server_port_selected_handler_func = NULL;
 #endif
   args->create_memory_object_func = NULL;
   args->validation_cache = NULL;
 #if NACL_WINDOWS
   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
 #if NACL_LINUX
   args->prereserved_sandbox_size = 0;
 #endif
   args->nexe_desc = NULL;
 
   args->argc = default_argc;
   args->argv = (char **) default_argv;
@@ skipping 244 matching lines @@
     if (!NaClDebugInit(nap)) {
       goto done;
     }
 #if NACL_WINDOWS
     if (NULL != args->debug_stub_server_port_selected_handler_func) {
       args->debug_stub_server_port_selected_handler_func(nap->debug_stub_port);
     }
 #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) {
     NaClBlockIfCommandChannelExists(nap);
   } else {
     /*
      * Don't return LOAD_OK if we had some failure loading.
      */
     errcode = LOAD_INTERNAL;
   }
@@ skipping 43 matching lines @@
 int NaClChromeMainStart(struct NaClApp *nap,
                         struct NaClChromeMainArgs *args,
                         int *exit_status) {
   int load_ok = LOAD_OK == LoadApp(nap, args);
   if (load_ok) {
     *exit_status = StartApp(nap, args);
   }
   free(args);
   return load_ok;
 }