Pepper: Report NaCl exit status over Chromium IPC.

This change uses a NaCl embedder interface for getting the exit status inside
the loader process instead of receiving that information over SRPC.

The IPC message introduced here must be synchronous so that we're guaranteed to
report the exit status before the loader process exits.

BUG=397161
TEST=NaClBrowserTest.ExitStatus*
R=dmichael@chromium.org, jschuh@chromium.org, mseaborn@chromium.org

Review URL: https://codereview.chromium.org/484783002

Cr-Commit-Position: refs/heads/master@{#291480}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291480 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 100 insertions, 90 deletions

diff --git a/components/nacl.gyp b/components/nacl.gyp
index 500d597..3d3c301 100644
--- a/components/nacl.gyp
+++ b/components/nacl.gyp
@@ -181,6 +181,7 @@
             '..',
           ],
           'dependencies': [
+            'nacl_common',
             '../content/content.gyp:content_renderer',
             '../ppapi/native_client/src/trusted/plugin/plugin.gyp:nacl_trusted_plugin',
             '../third_party/jsoncpp/jsoncpp.gyp:jsoncpp',
@@ -410,6 +411,8 @@
                 'nacl/common/nacl_constants.h',
                 'nacl/common/nacl_messages.cc',
                 'nacl/common/nacl_messages.h',
+                'nacl/common/nacl_renderer_messages.h',
+                'nacl/common/nacl_renderer_messages.cc',
                 'nacl/common/nacl_types.cc',
                 'nacl/common/nacl_types.h',
               ],
@@ -496,6 +499,8 @@
         'nacl/common/nacl_nonsfi_util.cc',
         'nacl/common/nacl_nonsfi_util.h',
         'nacl/common/nacl_process_type.h',
+        'nacl/common/nacl_renderer_messages.h',
+        'nacl/common/nacl_renderer_messages.cc',
         'nacl/common/nacl_sandbox_type_mac.h',
         'nacl/common/nacl_types.cc',
         'nacl/common/nacl_types.h',
diff --git a/components/nacl/common/nacl_renderer_messages.cc b/components/nacl/common/nacl_renderer_messages.cc
new file mode 100644
index 0000000..0950901
--- /dev/null
+++ b/components/nacl/common/nacl_renderer_messages.cc
@@ -0,0 +1,33 @@
+// Copyright (c) 2014 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.
+
+// Get basic type definitions.
+#define IPC_MESSAGE_IMPL
+#include "components/nacl/common/nacl_renderer_messages.h"
+
+// Generate constructors.
+#include "ipc/struct_constructor_macros.h"
+#include "components/nacl/common/nacl_renderer_messages.h"
+
+// Generate destructors.
+#include "ipc/struct_destructor_macros.h"
+#include "components/nacl/common/nacl_renderer_messages.h"
+
+// Generate param traits write methods.
+#include "ipc/param_traits_write_macros.h"
+namespace IPC {
+#include "components/nacl/common/nacl_renderer_messages.h"
+}  // namespace IPC
+
+// Generate param traits read methods.
+#include "ipc/param_traits_read_macros.h"
+namespace IPC {
+#include "components/nacl/common/nacl_renderer_messages.h"
+}  // namespace IPC
+
+// Generate param traits log methods.
+#include "ipc/param_traits_log_macros.h"
+namespace IPC {
+#include "components/nacl/common/nacl_renderer_messages.h"
+}  // namespace IPC
diff --git a/components/nacl/common/nacl_renderer_messages.h b/components/nacl/common/nacl_renderer_messages.h
new file mode 100644
index 0000000..e07a082
--- /dev/null
+++ b/components/nacl/common/nacl_renderer_messages.h
@@ -0,0 +1,15 @@
+// Copyright 2014 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.
+
+// Defines messages between the renderer and the NaCl process.
+
+// Multiply-included message file, no traditional include guard.
+#include "ipc/ipc_message_macros.h"
+
+#define IPC_MESSAGE_START NaClHostMsgStart
+
+// This message must be synchronous to ensure that the exit status is sent from
+// NaCl to the renderer before the NaCl process exits very soon after.
+IPC_SYNC_MESSAGE_CONTROL1_0(NaClRendererMsg_ReportExitStatus,
+                            int /* exit_status */)
diff --git a/components/nacl/loader/nacl_listener.cc b/components/nacl/loader/nacl_listener.cc
index 26585b1..c0e3c5e 100644
--- a/components/nacl/loader/nacl_listener.cc
+++ b/components/nacl/loader/nacl_listener.cc
@@ -19,6 +19,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/rand_util.h"
 #include "components/nacl/common/nacl_messages.h"
+#include "components/nacl/common/nacl_renderer_messages.h"
 #include "components/nacl/loader/nacl_ipc_adapter.h"
 #include "components/nacl/loader/nacl_validation_db.h"
 #include "components/nacl/loader/nacl_validation_query.h"
@@ -418,5 +419,11 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
   nexe_file_info.file_token.hi = params.nexe_token_hi;
   args->nexe_desc = NaClDescIoFromFileInfo(nexe_file_info, NACL_ABI_O_RDONLY);
 
-  NaClChromeMainStartApp(nap, args);
+  int exit_status;
+  if (!NaClChromeMainStart(nap, args, &exit_status))
+    NaClExit(1);
+
+  // Report the plugin's exit status if the application started successfully.
+  trusted_listener_->Send(new NaClRendererMsg_ReportExitStatus(exit_status));
+  NaClExit(exit_status);
 }
diff --git a/components/nacl/renderer/nexe_load_manager.cc b/components/nacl/renderer/nexe_load_manager.cc
index 512700a..0844e1a 100644
--- a/components/nacl/renderer/nexe_load_manager.cc
+++ b/components/nacl/renderer/nexe_load_manager.cc
@@ -97,6 +97,7 @@ NexeLoadManager::NexeLoadManager(
       plugin_instance_(content::PepperPluginInstance::Get(pp_instance)),
       crash_info_shmem_handle_(base::SharedMemory::NULLHandle()),
       weak_factory_(this) {
+  set_exit_status(-1);
   SetLastError("");
   HistogramEnumerateOsArch(GetSandboxArch());
   if (plugin_instance_) {
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 73c6c66..44af080 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -406,10 +406,13 @@ void LaunchSelLdr(PP_Instance instance,
 
   // Create the trusted plugin channel.
   if (IsValidChannelHandle(launch_result.trusted_ipc_channel_handle)) {
+    bool report_exit_status = PP_ToBool(main_service_runtime);
     scoped_ptr<TrustedPluginChannel> trusted_plugin_channel(
         new TrustedPluginChannel(
+            load_manager,
             launch_result.trusted_ipc_channel_handle,
-            content::RenderThread::Get()->GetShutdownEvent()));
+            content::RenderThread::Get()->GetShutdownEvent(),
+            report_exit_status));
     load_manager->set_trusted_plugin_channel(trusted_plugin_channel.Pass());
   } else {
     PostPPCompletionCallback(callback, PP_ERROR_FAILED);
@@ -788,21 +791,6 @@ PP_NaClReadyState GetNaClReadyState(PP_Instance instance) {
   return PP_NACL_READY_STATE_UNSENT;
 }
 
-int32_t GetExitStatus(PP_Instance instance) {
-  NexeLoadManager* load_manager = NexeLoadManager::Get(instance);
-  DCHECK(load_manager);
-  if (load_manager)
-    return load_manager->exit_status();
-  return -1;
-}
-
-void SetExitStatus(PP_Instance instance, int32_t exit_status) {
-  NexeLoadManager* load_manager = NexeLoadManager::Get(instance);
-  DCHECK(load_manager);
-  if (load_manager)
-    return load_manager->set_exit_status(exit_status);
-}
-
 void Vlog(const char* message) {
   VLOG(1) << message;
 }
@@ -1703,8 +1691,6 @@ const PPB_NaCl_Private nacl_interface = {
   &GetSandboxArch,
   &LogToConsole,
   &GetNaClReadyState,
-  &GetExitStatus,
-  &SetExitStatus,
   &Vlog,
   &InitializePlugin,
   &GetNexeSize,
diff --git a/components/nacl/renderer/trusted_plugin_channel.cc b/components/nacl/renderer/trusted_plugin_channel.cc
index 29e8aff..0aaf6bc 100644
--- a/components/nacl/renderer/trusted_plugin_channel.cc
+++ b/components/nacl/renderer/trusted_plugin_channel.cc
@@ -5,15 +5,22 @@
 #include "components/nacl/renderer/trusted_plugin_channel.h"
 
 #include "base/callback_helpers.h"
+#include "components/nacl/common/nacl_renderer_messages.h"
+#include "components/nacl/renderer/nexe_load_manager.h"
 #include "content/public/renderer/render_thread.h"
 #include "ipc/ipc_sync_channel.h"
+#include "ipc/ipc_message_macros.h"
 #include "ppapi/c/pp_errors.h"
 
 namespace nacl {
 
 TrustedPluginChannel::TrustedPluginChannel(
+    NexeLoadManager* nexe_load_manager,
     const IPC::ChannelHandle& handle,
-    base::WaitableEvent* shutdown_event) {
+    base::WaitableEvent* shutdown_event,
+    bool report_exit_status)
+    : nexe_load_manager_(nexe_load_manager),
+      report_exit_status_(report_exit_status) {
   channel_ = IPC::SyncChannel::Create(
       handle,
       IPC::Channel::MODE_CLIENT,
@@ -30,8 +37,18 @@ bool TrustedPluginChannel::Send(IPC::Message* message) {
   return channel_->Send(message);
 }
 
-bool TrustedPluginChannel::OnMessageReceived(const IPC::Message& message) {
-  return false;
+bool TrustedPluginChannel::OnMessageReceived(const IPC::Message& msg) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(TrustedPluginChannel, msg)
+    IPC_MESSAGE_HANDLER(NaClRendererMsg_ReportExitStatus, OnReportExitStatus);
+    IPC_MESSAGE_UNHANDLED(handled = false);
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void TrustedPluginChannel::OnReportExitStatus(int exit_status) {
+  if (report_exit_status_)
+    nexe_load_manager_->set_exit_status(exit_status);
 }
 
 }  // namespace nacl
diff --git a/components/nacl/renderer/trusted_plugin_channel.h b/components/nacl/renderer/trusted_plugin_channel.h
index cba26d6..fd261aa 100644
--- a/components/nacl/renderer/trusted_plugin_channel.h
+++ b/components/nacl/renderer/trusted_plugin_channel.h
@@ -21,11 +21,14 @@ class SyncChannel;
 }  // namespace IPC
 
 namespace nacl {
+class NexeLoadManager;
 
 class TrustedPluginChannel : public IPC::Listener {
  public:
-  TrustedPluginChannel(const IPC::ChannelHandle& handle,
-                       base::WaitableEvent* shutdown_event);
+  TrustedPluginChannel(NexeLoadManager* nexe_load_manager,
+                       const IPC::ChannelHandle& handle,
+                       base::WaitableEvent* shutdown_event,
+                       bool report_exit_status);
   virtual ~TrustedPluginChannel();
 
   bool Send(IPC::Message* message);
@@ -33,8 +36,14 @@ class TrustedPluginChannel : public IPC::Listener {
   // Listener implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
+  void OnReportExitStatus(int exit_status);
+
  private:
+  // Non-owning pointer. This is safe because the TrustedPluginChannel is owned
+  // by the NexeLoadManager pointed to here.
+  NexeLoadManager* nexe_load_manager_;
   scoped_ptr<IPC::SyncChannel> channel_;
+  bool report_exit_status_;
 
   DISALLOW_COPY_AND_ASSIGN(TrustedPluginChannel);
 };
diff --git a/ppapi/api/private/ppb_nacl_private.idl b/ppapi/api/private/ppb_nacl_private.idl
index 8d9038c..cb5cc4e 100644
--- a/ppapi/api/private/ppb_nacl_private.idl
+++ b/ppapi/api/private/ppb_nacl_private.idl
@@ -301,13 +301,6 @@ interface PPB_NaCl_Private {
   /* Returns the NaCl readiness status for this instance. */
   PP_NaClReadyState GetNaClReadyState([in] PP_Instance instance);
 
-  /* Returns the exit status of the plugin process. */
-  int32_t GetExitStatus([in] PP_Instance instance);
-
-  /* Sets the exit status of the plugin process. */
-  void SetExitStatus([in] PP_Instance instance,
-                     [in] int32_t exit_status);
-
   /* Logs the message via VLOG. */
   void Vlog([in] str_t message);
 
diff --git a/ppapi/c/private/ppb_nacl_private.h b/ppapi/c/private/ppb_nacl_private.h
index 67d2939..3e5b71d 100644
--- a/ppapi/c/private/ppb_nacl_private.h
+++ b/ppapi/c/private/ppb_nacl_private.h
@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From private/ppb_nacl_private.idl modified Thu Aug 14 11:48:23 2014. */
+/* From private/ppb_nacl_private.idl modified Thu Aug 21 11:14:10 2014. */
 
 #ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
 #define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
@@ -322,10 +322,6 @@ struct PPB_NaCl_Private_1_0 {
   void (*LogToConsole)(PP_Instance instance, const char* message);
   /* Returns the NaCl readiness status for this instance. */
   PP_NaClReadyState (*GetNaClReadyState)(PP_Instance instance);
-  /* Returns the exit status of the plugin process. */
-  int32_t (*GetExitStatus)(PP_Instance instance);
-  /* Sets the exit status of the plugin process. */
-  void (*SetExitStatus)(PP_Instance instance, int32_t exit_status);
   /* Logs the message via VLOG. */
   void (*Vlog)(const char* message);
   /* Initializes internal state for a NaCl plugin. */
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 6104b90..ea86558 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -275,9 +275,6 @@ Plugin::Plugin(PP_Instance pp_instance)
   // Notify PPB_NaCl_Private that the instance is created before altering any
   // state that it tracks.
   nacl_interface_->InstanceCreated(pp_instance);
-  // We call set_exit_status() here to ensure that the 'exitStatus' property is
-  // set. This can only be called when nacl_interface_ is not NULL.
-  set_exit_status(-1);
   nexe_file_info_ = kInvalidNaClFileInfo;
 }
 
@@ -452,24 +449,4 @@ bool Plugin::DocumentCanRequest(const std::string& url) {
   return pp::URLUtil_Dev::Get()->DocumentCanRequest(this, pp::Var(url));
 }
 
-void Plugin::set_exit_status(int exit_status) {
-  pp::Core* core = pp::Module::Get()->core();
-  if (core->IsMainThread()) {
-    SetExitStatusOnMainThread(PP_OK, exit_status);
-  } else {
-    pp::CompletionCallback callback =
-        callback_factory_.NewCallback(&Plugin::SetExitStatusOnMainThread,
-                                      exit_status);
-    core->CallOnMainThread(0, callback, 0);
-  }
-}
-
-void Plugin::SetExitStatusOnMainThread(int32_t pp_error,
-                                       int exit_status) {
-  DCHECK(pp::Module::Get()->core()->IsMainThread());
-  DCHECK(nacl_interface_);
-  nacl_interface_->SetExitStatus(pp_instance(), exit_status);
-}
-
-
 }  // namespace plugin
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h
index ce3273d..a55b62d 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin.h
@@ -127,9 +127,6 @@ class Plugin : public pp::Instance {
   // document to request the URL using CORS even if this function returns false.
   bool DocumentCanRequest(const std::string& url);
 
-  // set_exit_status may be called off the main thread.
-  void set_exit_status(int exit_status);
-
   const PPB_NaCl_Private* nacl_interface() const { return nacl_interface_; }
   pp::UMAPrivate& uma_interface() { return uma_interface_; }
 
@@ -206,8 +203,6 @@ class Plugin : public pp::Instance {
   // Processes the JSON manifest string and starts loading the nexe.
   void ProcessNaClManifest(const nacl::string& manifest_json);
 
-  void SetExitStatusOnMainThread(int32_t pp_error, int exit_status);
-
   // Keep track of the NaCl module subprocess that was spun up in the plugin.
   NaClSubprocess main_subprocess_;
 
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index d570f4c..ffce563 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -238,7 +238,8 @@ void PluginReverseInterface::ReportCrash() {
 }
 
 void PluginReverseInterface::ReportExitStatus(int exit_status) {
-  service_runtime_->set_exit_status(exit_status);
+  // We do nothing here; reporting exit status is handled through a separate
+  // embedder interface.
 }
 
 int64_t PluginReverseInterface::RequestQuotaForWrite(
@@ -582,10 +583,4 @@ ServiceRuntime::~ServiceRuntime() {
   NaClMutexDtor(&mu_);
 }
 
-void ServiceRuntime::set_exit_status(int exit_status) {
-  nacl::MutexLocker take(&mu_);
-  if (main_service_runtime_)
-    plugin_->set_exit_status(exit_status & 0xff);
-}
-
 }  // namespace plugin
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index dc70330..2b92727 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -190,13 +190,6 @@ class ServiceRuntime {
   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);
-
   bool main_service_runtime() const { return main_service_runtime_; }
 
  private:
diff --git a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
index 65c5a70..a742eb6 100644
--- a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
+++ b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
@@ -3482,16 +3482,6 @@ static PP_NaClReadyState Pnacl_M25_PPB_NaCl_Private_GetNaClReadyState(PP_Instanc
   return iface->GetNaClReadyState(instance);
 }
 
-static int32_t Pnacl_M25_PPB_NaCl_Private_GetExitStatus(PP_Instance instance) {
-  const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
-  return iface->GetExitStatus(instance);
-}
-
-static void Pnacl_M25_PPB_NaCl_Private_SetExitStatus(PP_Instance instance, int32_t exit_status) {
-  const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
-  iface->SetExitStatus(instance, exit_status);
-}
-
 static void Pnacl_M25_PPB_NaCl_Private_Vlog(const char* message) {
   const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
   iface->Vlog(message);
@@ -5374,8 +5364,6 @@ static const struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = {
     .GetSandboxArch = (const char* (*)(void))&Pnacl_M25_PPB_NaCl_Private_GetSandboxArch,
     .LogToConsole = (void (*)(PP_Instance instance, const char* message))&Pnacl_M25_PPB_NaCl_Private_LogToConsole,
     .GetNaClReadyState = (PP_NaClReadyState (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_GetNaClReadyState,
-    .GetExitStatus = (int32_t (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_GetExitStatus,
-    .SetExitStatus = (void (*)(PP_Instance instance, int32_t exit_status))&Pnacl_M25_PPB_NaCl_Private_SetExitStatus,
     .Vlog = (void (*)(const char* message))&Pnacl_M25_PPB_NaCl_Private_Vlog,
     .InitializePlugin = (void (*)(PP_Instance instance, uint32_t argc, const char* argn[], const char* argv[]))&Pnacl_M25_PPB_NaCl_Private_InitializePlugin,
     .GetNexeSize = (int64_t (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_GetNexeSize,