summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/test/nacl/nacl_browsertest.cc3
-rw-r--r--components/nacl/renderer/ppb_nacl_private_impl.cc7
-rw-r--r--components/nacl/renderer/trusted_plugin_channel.cc5
-rw-r--r--components/nacl/renderer/trusted_plugin_channel.h1
-rw-r--r--ppapi/api/private/ppb_nacl_private.idl3
-rw-r--r--ppapi/c/private/ppb_nacl_private.h4
-rw-r--r--ppapi/native_client/src/trusted/plugin/plugin.cc18
-rw-r--r--ppapi/native_client/src/trusted/plugin/plugin.h15
-rw-r--r--ppapi/native_client/src/trusted/plugin/service_runtime.cc49
-rw-r--r--ppapi/native_client/src/trusted/plugin/service_runtime.h9
-rw-r--r--ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c6
11 files changed, 30 insertions, 90 deletions
diff --git a/chrome/test/nacl/nacl_browsertest.cc b/chrome/test/nacl/nacl_browsertest.cc
index 7e26901..1e17317 100644
--- a/chrome/test/nacl/nacl_browsertest.cc
+++ b/chrome/test/nacl/nacl_browsertest.cc
@@ -107,9 +107,6 @@ IN_PROC_BROWSER_TEST_F(NaClBrowserTestNewlib, BadNative) {
#if defined(OS_WIN)
// crbug.com/98721
# define MAYBE_Crash DISABLED_Crash
-#elif defined(OS_LINUX)
-// crbug.com/366334
-# define MAYBE_Crash DISABLED_Crash
#else
# define MAYBE_Crash Crash
#endif
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 58faad7..de005df 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -749,12 +749,6 @@ void ReportLoadAbort(PP_Instance instance) {
load_manager->ReportLoadAbort();
}
-void NexeDidCrash(PP_Instance instance) {
- NexeLoadManager* load_manager = NexeLoadManager::Get(instance);
- if (load_manager)
- load_manager->NexeDidCrash();
-}
-
void InstanceCreated(PP_Instance instance) {
NexeLoadManager::Create(instance);
}
@@ -1666,7 +1660,6 @@ const PPB_NaCl_Private nacl_interface = {
&ReportLoadSuccess,
&ReportLoadError,
&ReportLoadAbort,
- &NexeDidCrash,
&InstanceCreated,
&InstanceDestroyed,
&NaClDebugEnabledForURL,
diff --git a/components/nacl/renderer/trusted_plugin_channel.cc b/components/nacl/renderer/trusted_plugin_channel.cc
index 0aaf6bc..729644c 100644
--- a/components/nacl/renderer/trusted_plugin_channel.cc
+++ b/components/nacl/renderer/trusted_plugin_channel.cc
@@ -46,6 +46,11 @@ bool TrustedPluginChannel::OnMessageReceived(const IPC::Message& msg) {
return handled;
}
+void TrustedPluginChannel::OnChannelError() {
+ if (report_exit_status_)
+ nexe_load_manager_->NexeDidCrash();
+}
+
void TrustedPluginChannel::OnReportExitStatus(int exit_status) {
if (report_exit_status_)
nexe_load_manager_->set_exit_status(exit_status);
diff --git a/components/nacl/renderer/trusted_plugin_channel.h b/components/nacl/renderer/trusted_plugin_channel.h
index fd261aa..5f49f12 100644
--- a/components/nacl/renderer/trusted_plugin_channel.h
+++ b/components/nacl/renderer/trusted_plugin_channel.h
@@ -35,6 +35,7 @@ class TrustedPluginChannel : public IPC::Listener {
// Listener implementation.
virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+ virtual void OnChannelError() OVERRIDE;
void OnReportExitStatus(int exit_status);
diff --git a/ppapi/api/private/ppb_nacl_private.idl b/ppapi/api/private/ppb_nacl_private.idl
index e777527..3924ca2 100644
--- a/ppapi/api/private/ppb_nacl_private.idl
+++ b/ppapi/api/private/ppb_nacl_private.idl
@@ -275,9 +275,6 @@ interface PPB_NaCl_Private {
/* Reports that loading a nexe was aborted. */
void ReportLoadAbort([in] PP_Instance instance);
- /* Reports that the nexe has crashed. */
- void NexeDidCrash([in] PP_Instance instance);
-
/* Performs internal setup when an instance is created. */
void InstanceCreated([in] PP_Instance instance);
diff --git a/ppapi/c/private/ppb_nacl_private.h b/ppapi/c/private/ppb_nacl_private.h
index b28dff2..5ba08d9 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 Wed Aug 27 13:55:13 2014. */
+/* From private/ppb_nacl_private.idl modified Thu Sep 4 07:46:02 2014. */
#ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
#define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
@@ -304,8 +304,6 @@ struct PPB_NaCl_Private_1_0 {
const char* error_message);
/* Reports that loading a nexe was aborted. */
void (*ReportLoadAbort)(PP_Instance instance);
- /* Reports that the nexe has crashed. */
- void (*NexeDidCrash)(PP_Instance instance);
/* Performs internal setup when an instance is created. */
void (*InstanceCreated)(PP_Instance instance);
/* Performs internal cleanup when an instance is destroyed. */
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index ea86558..e7b2568 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -72,7 +72,6 @@ bool Plugin::LoadHelperNaClModuleInternal(NaClSubprocess* subprocess,
pp_instance(),
false, // No main_service_runtime.
false, // No non-SFI mode (i.e. in SFI-mode).
- pp::BlockUntilComplete(),
pp::BlockUntilComplete());
subprocess->set_service_runtime(service_runtime);
@@ -128,8 +127,7 @@ void Plugin::LoadNaClModule(PP_NaClFileInfo file_info,
bool enable_dyncode_syscalls,
bool enable_exception_handling,
bool enable_crash_throttling,
- const pp::CompletionCallback& init_done_cb,
- const pp::CompletionCallback& crash_cb) {
+ const pp::CompletionCallback& init_done_cb) {
CHECK(pp::Module::Get()->core()->IsMainThread());
// Before forking a new sel_ldr process, ensure that we do not leak
// the ServiceRuntime object for an existing subprocess, and that any
@@ -149,7 +147,7 @@ void Plugin::LoadNaClModule(PP_NaClFileInfo file_info,
enable_crash_throttling);
ErrorInfo error_info;
ServiceRuntime* service_runtime = new ServiceRuntime(
- this, pp_instance(), true, uses_nonsfi_mode, init_done_cb, crash_cb);
+ this, pp_instance(), true, uses_nonsfi_mode, init_done_cb);
main_subprocess_.set_service_runtime(service_runtime);
if (NULL == service_runtime) {
error_info.SetReport(
@@ -336,8 +334,7 @@ void Plugin::NexeFileDidOpen(int32_t pp_error) {
true, /* enable_dyncode_syscalls */
true, /* enable_exception_handling */
false, /* enable_crash_throttling */
- callback_factory_.NewCallback(&Plugin::NexeFileDidOpenContinuation),
- callback_factory_.NewCallback(&Plugin::NexeDidCrash));
+ callback_factory_.NewCallback(&Plugin::NexeFileDidOpenContinuation));
}
void Plugin::NexeFileDidOpenContinuation(int32_t pp_error) {
@@ -355,12 +352,6 @@ void Plugin::NexeFileDidOpenContinuation(int32_t pp_error) {
NaClLog(4, "Leaving NexeFileDidOpenContinuation\n");
}
-void Plugin::NexeDidCrash(int32_t pp_error) {
- PLUGIN_PRINTF(("Plugin::NexeDidCrash (pp_error=%" NACL_PRId32 ")\n",
- pp_error));
- nacl_interface_->NexeDidCrash(pp_instance());
-}
-
void Plugin::BitcodeDidTranslate(int32_t pp_error) {
PLUGIN_PRINTF(("Plugin::BitcodeDidTranslate (pp_error=%" NACL_PRId32 ")\n",
pp_error));
@@ -382,8 +373,7 @@ void Plugin::BitcodeDidTranslate(int32_t pp_error) {
false, /* enable_dyncode_syscalls */
false, /* enable_exception_handling */
true, /* enable_crash_throttling */
- callback_factory_.NewCallback(&Plugin::BitcodeDidTranslateContinuation),
- callback_factory_.NewCallback(&Plugin::NexeDidCrash));
+ callback_factory_.NewCallback(&Plugin::BitcodeDidTranslateContinuation));
}
void Plugin::BitcodeDidTranslateContinuation(int32_t pp_error) {
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h
index a55b62d..8b89a88 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin.h
@@ -93,8 +93,7 @@ class Plugin : public pp::Instance {
bool enable_dyncode_syscalls,
bool enable_exception_handling,
bool enable_crash_throttling,
- const pp::CompletionCallback& init_done_cb,
- const pp::CompletionCallback& crash_cb);
+ const pp::CompletionCallback& init_done_cb);
// Finish hooking interfaces up, after low-level initialization is
// complete.
@@ -173,18 +172,6 @@ class Plugin : public pp::Instance {
void NexeFileDidOpen(int32_t pp_error);
void NexeFileDidOpenContinuation(int32_t pp_error);
- // Callback used when the reverse channel closes. This is an
- // asynchronous event that might turn into a JavaScript error or
- // crash event -- this is controlled by the two state variables
- // nacl_ready_state_ and nexe_error_reported_: If an error or crash
- // had already been reported, no additional crash event is
- // generated. If no error has been reported but nacl_ready_state_
- // is not DONE, then the loadend event has not been reported, and we
- // enqueue an error event followed by loadend. If nacl_ready_state_
- // is DONE, then we are in the post-loadend (we need temporal
- // predicate symbols), and we enqueue a crash event.
- void NexeDidCrash(int32_t pp_error);
-
// Callback used when a .nexe is translated from bitcode. If the translation
// is successful, the file descriptor is opened and can be passed to sel_ldr
// with the sandbox on.
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index b162f62..4787162 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -58,14 +58,12 @@ PluginReverseInterface::PluginReverseInterface(
nacl::WeakRefAnchor* anchor,
PP_Instance pp_instance,
ServiceRuntime* service_runtime,
- pp::CompletionCallback init_done_cb,
- pp::CompletionCallback crash_cb)
+ pp::CompletionCallback init_done_cb)
: anchor_(anchor),
pp_instance_(pp_instance),
service_runtime_(service_runtime),
shutting_down_(false),
- init_done_cb_(init_done_cb),
- crash_cb_(crash_cb) {
+ init_done_cb_(init_done_cb) {
NaClXMutexCtor(&mu_);
NaClXCondVarCtor(&cv_);
}
@@ -222,18 +220,7 @@ void PluginReverseInterface::StreamAsFile_MainThreadContinuation(
}
void PluginReverseInterface::ReportCrash() {
- NaClLog(4, "PluginReverseInterface::ReportCrash\n");
-
- if (crash_cb_.pp_completion_callback().func != NULL) {
- NaClLog(4, "PluginReverseInterface::ReportCrash: invoking CB\n");
- pp::Module::Get()->core()->CallOnMainThread(0, crash_cb_, PP_OK);
- // Clear the callback to avoid it gets invoked twice.
- crash_cb_ = pp::CompletionCallback();
- } else {
- NaClLog(1,
- "PluginReverseInterface::ReportCrash:"
- " crash_cb_ not valid, skipping\n");
- }
+ // This is now handled through Chromium IPC.
}
void PluginReverseInterface::ReportExitStatus(int exit_status) {
@@ -250,8 +237,7 @@ ServiceRuntime::ServiceRuntime(Plugin* plugin,
PP_Instance pp_instance,
bool main_service_runtime,
bool uses_nonsfi_mode,
- pp::CompletionCallback init_done_cb,
- pp::CompletionCallback crash_cb)
+ pp::CompletionCallback init_done_cb)
: plugin_(plugin),
pp_instance_(pp_instance),
main_service_runtime_(main_service_runtime),
@@ -259,7 +245,7 @@ ServiceRuntime::ServiceRuntime(Plugin* plugin,
reverse_service_(NULL),
anchor_(new nacl::WeakRefAnchor()),
rev_interface_(new PluginReverseInterface(anchor_, pp_instance, this,
- init_done_cb, crash_cb)),
+ init_done_cb)),
start_sel_ldr_done_(false),
start_nexe_done_(false),
nexe_started_ok_(false),
@@ -487,24 +473,21 @@ bool ServiceRuntime::StartNexeInternal() {
}
void ServiceRuntime::ReapLogs() {
- // On a load failure the service runtime does not crash itself to
+ // TODO(teravest): We should allow the NaCl process to crash itself when a
+ // module fails to start, and remove the call to RemoteLog() here. The
+ // reverse channel is no longer needed for crash reporting.
+ //
+ // The reasoning behind the current code behavior follows:
+ // On a load failure the NaCl process 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.
+ // we induce a NaCl process crash here.
RemoteLog(LOG_FATAL, "reap logs\n");
- if (NULL == reverse_service_) {
- // No crash detector thread.
- NaClLog(LOG_ERROR, "scheduling to get crash log\n");
- // Invoking rev_interface's method is workaround to avoid crash_cb
- // gets called twice or more. We should clean this up later.
- rev_interface_->ReportCrash();
- NaClLog(LOG_ERROR, "should fire soon\n");
- } else {
- NaClLog(LOG_ERROR, "Reverse service thread will pick up crash log\n");
- }
+
+ // TODO(teravest): Release subprocess_ here since it's no longer needed. It
+ // was previously kept around to collect crash log output from the bootstrap
+ // channel.
}
void ServiceRuntime::ReportLoadError(const ErrorInfo& error_info) {
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index 2b92727..2fccc7e 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -96,8 +96,7 @@ class PluginReverseInterface: public nacl::ReverseInterface {
PluginReverseInterface(nacl::WeakRefAnchor* anchor,
PP_Instance pp_instance,
ServiceRuntime* service_runtime,
- pp::CompletionCallback init_done_cb,
- pp::CompletionCallback crash_cb);
+ pp::CompletionCallback init_done_cb);
virtual ~PluginReverseInterface();
@@ -139,20 +138,16 @@ class PluginReverseInterface: public nacl::ReverseInterface {
bool shutting_down_;
pp::CompletionCallback init_done_cb_;
- pp::CompletionCallback crash_cb_;
};
// ServiceRuntime abstracts a NativeClient sel_ldr instance.
class ServiceRuntime {
public:
- // TODO(sehr): This class should also implement factory methods, using the
- // Start method below.
ServiceRuntime(Plugin* plugin,
PP_Instance pp_instance,
bool main_service_runtime,
bool uses_nonsfi_mode,
- pp::CompletionCallback init_done_cb,
- pp::CompletionCallback crash_cb);
+ pp::CompletionCallback init_done_cb);
// The destructor terminates the sel_ldr process.
~ServiceRuntime();
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 73c18d8..ab1beda 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
@@ -3492,11 +3492,6 @@ static void Pnacl_M25_PPB_NaCl_Private_ReportLoadAbort(PP_Instance instance) {
iface->ReportLoadAbort(instance);
}
-static void Pnacl_M25_PPB_NaCl_Private_NexeDidCrash(PP_Instance instance) {
- const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
- iface->NexeDidCrash(instance);
-}
-
static void Pnacl_M25_PPB_NaCl_Private_InstanceCreated(PP_Instance instance) {
const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
iface->InstanceCreated(instance);
@@ -5408,7 +5403,6 @@ static const struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = {
.ReportLoadSuccess = (void (*)(PP_Instance instance, uint64_t loaded_bytes, uint64_t total_bytes))&Pnacl_M25_PPB_NaCl_Private_ReportLoadSuccess,
.ReportLoadError = (void (*)(PP_Instance instance, PP_NaClError error, const char* error_message))&Pnacl_M25_PPB_NaCl_Private_ReportLoadError,
.ReportLoadAbort = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_ReportLoadAbort,
- .NexeDidCrash = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_NexeDidCrash,
.InstanceCreated = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_InstanceCreated,
.InstanceDestroyed = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_InstanceDestroyed,
.NaClDebugEnabledForURL = (PP_Bool (*)(const char* alleged_nmf_url))&Pnacl_M25_PPB_NaCl_Private_NaClDebugEnabledForURL,