NaCl: clean up nexe loading logic in trusted plugin.

ppapi/native_client/src/trusted/plugin/service_runtime.h

 /* -*- c++ -*- */
 /*
  * 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.
  */
 
 // A class containing information regarding a socket connection to a
 // service runtime instance.
 
@@ skipping 163 matching lines @@
   // actually start. Returns |false| if timed out waiting for the process
   // to start. Otherwise, returns |true| if StartSelLdr is complete
   // (either successfully or unsuccessfully).
   bool WaitForSelLdrStart();
 
   // Signal to waiting threads that StartSelLdr is complete (either
   // successfully or unsuccessfully).
   void SignalStartSelLdrDone();
 
   // If starting the nexe from a background thread, wait for the nexe to
-  // actually start.
-  void WaitForNexeStart();
+  // actually start. Returns |true| is the nexe started successfully.
+  bool WaitForNexeStart();
 
   // Signal to waiting threads that LoadNexeAndStart is complete (either
   // successfully or unsuccessfully).
-  void SignalNexeStarted();
+  void SignalNexeStarted(bool ok);
 
   // Establish an SrpcClient to the sel_ldr instance and load the nexe.
   // The nexe to be started is passed through |file_info|.
-  // Upon completion |callback| is invoked with status code.
   // This function must be called on the main thread.
-  void LoadNexeAndStart(PP_NaClFileInfo file_info,
-                        const pp::CompletionCallback& callback);
+  void LoadNexeAndStart(PP_NaClFileInfo file_info);

[hidehiko, 2014/06/20 04:21:38]

nit: Could you also add short comment that this function can be called only
once? Otherwise, your change around SignalNexeStarted() won't work.

[Nick Bray (chromium), 2014/06/20 17:10:42]

Done.

 
   // Starts the application channel to the nexe.
   SrpcClient* SetupAppChannel();
 
-  bool Log(int severity, const nacl::string& msg);
+  bool RemoteLog(int severity, const nacl::string& msg);
   Plugin* plugin() const { return plugin_; }
   void Shutdown();
 
   // exit_status is -1 when invalid; when we set it, we will ensure
   // that it is non-negative (the portion of the exit status from the
   // nexe that is transferred is the low 8 bits of the argument to the
   // exit syscall).
   int exit_status();  // const, but grabs mutex etc.
   void set_exit_status(int exit_status);
 
   nacl::string GetCrashLogOutput();
 
   bool main_service_runtime() const { return main_service_runtime_; }
 
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(ServiceRuntime);
-  struct LoadNexeAndStartData;
-  void LoadNexeAndStartAfterLoadModule(
-      LoadNexeAndStartData* data, int32_t pp_error);
-  void DidLoadNexeAndStart(LoadNexeAndStartData* data, int32_t pp_error);
+  bool LoadNexeAndStartInternal(PP_NaClFileInfo file_info);

[hidehiko, 2014/06/20 04:21:38]

nit: "const PP_NaClFileInfo&".

[Nick Bray (chromium), 2014/06/20 17:10:42]

Done.

 
   bool SetupCommandChannel();
   bool InitReverseService();
-  void LoadModule(PP_NaClFileInfo file_info,
-                  pp::CompletionCallback callback);
-  void DidLoadModule(pp::CompletionCallback callback, int32_t pp_error);
+  bool LoadModule(PP_NaClFileInfo file_info);

[hidehiko, 2014/06/20 04:21:38]

nit: "const PP_NaClFileInfo&".

[Nick Bray (chromium), 2014/06/20 17:10:42]

Done.

   bool StartModule();
+  void ReapLogs();
 
   NaClSrpcChannel command_channel_;
   Plugin* plugin_;
   bool main_service_runtime_;
   bool uses_nonsfi_mode_;
   nacl::ReverseService* reverse_service_;
   nacl::scoped_ptr<nacl::SelLdrLauncherBase> subprocess_;
 
   nacl::WeakRefAnchor* anchor_;
 
   PluginReverseInterface* rev_interface_;
 
   // Mutex and CondVar to protect start_sel_ldr_done_ and nexe_started_.
   NaClMutex mu_;
   NaClCondVar cond_;
   bool start_sel_ldr_done_;
-  bool nexe_started_;
+  bool start_nexe_done_;
+  bool nexe_started_ok_;
 };
 
 }  // namespace plugin
 
 #endif  // NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_SERVICE_RUNTIME_H_