Re-enable crash throttling for PNaCl processes.

This version of the change only applies to sel_ldr processes created on behalf
of a pnacl application. This includes the translator nexes and the translated
application nexe.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=359
R=bbudge@chromium.org, jln@chromium.org, jvoung@chromium.org, mseaborn@chromium.org, yzshen@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218341 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 39 insertions, 63 deletions

diff --git a/ppapi/api/private/ppb_nacl_private.idl b/ppapi/api/private/ppb_nacl_private.idl
index bf2918b..9a67404 100644
--- a/ppapi/api/private/ppb_nacl_private.idl
+++ b/ppapi/api/private/ppb_nacl_private.idl
@@ -35,6 +35,9 @@ interface PPB_NaCl_Private {
    * will be able to use dynamic code system calls (e.g., mmap with PROT_EXEC).
    * The |enable_exception_handling| flag indicates whether or not the nexe
    * will be able to use hardware exception handling.
+   * The |enable_crash_throttling| flag indicates whether or not crashes of
+   * the nexe contribute to crash throttling statisics and whether nexe starts
+   * are throttled by crash throttling.
    */
   PP_ExternalPluginResult LaunchSelLdr([in] PP_Instance instance,
                                        [in] str_t alleged_url,
@@ -43,6 +46,7 @@ interface PPB_NaCl_Private {
                                        [in] PP_Bool enable_ppapi_dev,
                                        [in] PP_Bool enable_dyncode_syscalls,
                                        [in] PP_Bool enable_exception_handling,
+                                       [in] PP_Bool enable_crash_throttling,
                                        [out] mem_t imc_handle,
                                        [out] PP_Var error_message);
 
diff --git a/ppapi/c/private/ppb_nacl_private.h b/ppapi/c/private/ppb_nacl_private.h
index 8be2829..772998b 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 Tue Aug  6 11:51:26 2013. */
+/* From private/ppb_nacl_private.idl modified Mon Aug 19 14:06:38 2013. */
 
 #ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
 #define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
@@ -63,6 +63,9 @@ struct PPB_NaCl_Private_1_0 {
    * will be able to use dynamic code system calls (e.g., mmap with PROT_EXEC).
    * The |enable_exception_handling| flag indicates whether or not the nexe
    * will be able to use hardware exception handling.
+   * The |enable_crash_throttling| flag indicates whether or not crashes of
+   * the nexe contribute to crash throttling statisics and whether nexe starts
+   * are throttled by crash throttling.
    */
   PP_ExternalPluginResult (*LaunchSelLdr)(PP_Instance instance,
                                           const char* alleged_url,
@@ -71,6 +74,7 @@ struct PPB_NaCl_Private_1_0 {
                                           PP_Bool enable_ppapi_dev,
                                           PP_Bool enable_dyncode_syscalls,
                                           PP_Bool enable_exception_handling,
+                                          PP_Bool enable_crash_throttling,
                                           void* imc_handle,
                                           struct PP_Var* error_message);
   /* This function starts the IPC proxy so the nexe can communicate with the
diff --git a/ppapi/native_client/src/trusted/plugin/module_ppapi.cc b/ppapi/native_client/src/trusted/plugin/module_ppapi.cc
index 4204ed4..f78bbf3 100644
--- a/ppapi/native_client/src/trusted/plugin/module_ppapi.cc
+++ b/ppapi/native_client/src/trusted/plugin/module_ppapi.cc
@@ -76,28 +76,6 @@ pp::Instance* ModulePpapi::CreateInstance(PP_Instance pp_instance) {
   return plugin;
 }
 
-const uint64_t kMaxCrashesPerInterval = 3;
-const uint64_t kCrashesIntervalInSeconds = 120;
-
-void ModulePpapi::RegisterPluginCrash() {
-  PLUGIN_PRINTF(("ModulePpapi::RegisterPluginCrash ()\n"));
-  if (crash_times_.size() == kMaxCrashesPerInterval) {
-    crash_times_.pop_front();
-  }
-  int64_t time = NaClGetTimeOfDayMicroseconds();
-  crash_times_.push_back(time);
-}
-
-bool ModulePpapi::IsPluginUnstable() {
-  PLUGIN_PRINTF(("ModulePpapi::IsPluginUnstable ()\n"));
-  if (crash_times_.size() != kMaxCrashesPerInterval) {
-    return false;
-  }
-  int64_t now = NaClGetTimeOfDayMicroseconds();
-  int64_t delta = now - crash_times_.front();
-  return delta / (1000.0 * 1000.0) <= kCrashesIntervalInSeconds;
-}
-
 }  // namespace plugin
 
 
diff --git a/ppapi/native_client/src/trusted/plugin/module_ppapi.h b/ppapi/native_client/src/trusted/plugin/module_ppapi.h
index b2e7564..ee7bd19 100644
--- a/ppapi/native_client/src/trusted/plugin/module_ppapi.h
+++ b/ppapi/native_client/src/trusted/plugin/module_ppapi.h
@@ -4,8 +4,6 @@
  * found in the LICENSE file.
  */
 
-#include <deque>
-
 #include "ppapi/c/private/ppb_nacl_private.h"
 #include "ppapi/cpp/module.h"
 
@@ -21,18 +19,9 @@ class ModulePpapi : public pp::Module {
 
   virtual pp::Instance* CreateInstance(PP_Instance pp_instance);
 
-  // NaCl crash throttling.  If RegisterPluginCrash is called too many times
-  // within a time period, IsPluginUnstable reports true.  As long as
-  // IsPluginUnstable returns true, NaCl modules will fail to load.
-  void RegisterPluginCrash();
-  bool IsPluginUnstable();
-
  private:
   bool init_was_successful_;
   const PPB_NaCl_Private* private_interface_;
-
-  // Crash throttling support.
-  std::deque<int64_t> crash_times_;
 };
 
 }  // namespace plugin
diff --git a/ppapi/native_client/src/trusted/plugin/nacl_entry_points.h b/ppapi/native_client/src/trusted/plugin/nacl_entry_points.h
index 4fd620d..0d18208 100644
--- a/ppapi/native_client/src/trusted/plugin/nacl_entry_points.h
+++ b/ppapi/native_client/src/trusted/plugin/nacl_entry_points.h
@@ -24,6 +24,7 @@ typedef PP_ExternalPluginResult (*LaunchNaClProcessFunc)(
     PP_Bool enable_ppapi_dev,
     PP_Bool enable_dyncode_syscalls,
     PP_Bool enable_exception_handling,
+    PP_Bool enable_crash_throttling,
     NaClHandle* result_socket,
     struct PP_Var* error_message);
 
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index fe27aa2..c3faff2 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -55,7 +55,6 @@
 
 #include "ppapi/native_client/src/trusted/plugin/file_utils.h"
 #include "ppapi/native_client/src/trusted/plugin/json_manifest.h"
-#include "ppapi/native_client/src/trusted/plugin/module_ppapi.h"
 #include "ppapi/native_client/src/trusted/plugin/nacl_entry_points.h"
 #include "ppapi/native_client/src/trusted/plugin/nacl_subprocess.h"
 #include "ppapi/native_client/src/trusted/plugin/nexe_arch.h"
@@ -447,6 +446,7 @@ bool Plugin::LoadNaClModule(nacl::DescWrapper* wrapper,
                             ErrorInfo* error_info,
                             bool enable_dyncode_syscalls,
                             bool enable_exception_handling,
+                            bool enable_crash_throttling,
                             const pp::CompletionCallback& init_done_cb,
                             const pp::CompletionCallback& crash_cb) {
   // Before forking a new sel_ldr process, ensure that we do not leak
@@ -460,7 +460,8 @@ bool Plugin::LoadNaClModule(nacl::DescWrapper* wrapper,
                            true /* uses_ppapi */,
                            enable_dev_interfaces_,
                            enable_dyncode_syscalls,
-                           enable_exception_handling);
+                           enable_exception_handling,
+                           enable_crash_throttling);
   if (!LoadNaClModuleCommon(wrapper, &main_subprocess_, manifest_.get(),
                             true /* should_report_uma */,
                             params, init_done_cb, crash_cb)) {
@@ -532,7 +533,8 @@ NaClSubprocess* Plugin::LoadHelperNaClModule(nacl::DescWrapper* wrapper,
                            false /* uses_ppapi */,
                            enable_dev_interfaces_,
                            false /* enable_dyncode_syscalls */,
-                           false /* enable_exception_handling */);
+                           false /* enable_exception_handling */,
+                           true /* enable_crash_throttling */);
   if (!LoadNaClModuleCommon(wrapper, nacl_subprocess.get(), manifest,
                             false /* should_report_uma */,
                             params,
@@ -907,6 +909,7 @@ void Plugin::NexeFileDidOpen(int32_t pp_error) {
       wrapper.get(), &error_info,
       true, /* enable_dyncode_syscalls */
       true, /* enable_exception_handling */
+      false, /* enable_crash_throttling */
       callback_factory_.NewCallback(&Plugin::NexeFileDidOpenContinuation),
       callback_factory_.NewCallback(&Plugin::NexeDidCrash));
 
@@ -1010,10 +1013,6 @@ void Plugin::NexeDidCrash(int32_t pp_error) {
   // invocation will just be a no-op, since all the crash log will
   // have been received and we'll just get an EOF indication.
   CopyCrashLogToJsConsole();
-
-  // Remember the nexe crash time, which helps determine the need to throttle.
-  ModulePpapi* module_ppapi = static_cast<ModulePpapi*>(pp::Module::Get());
-  module_ppapi->RegisterPluginCrash();
 }
 
 void Plugin::BitcodeDidTranslate(int32_t pp_error) {
@@ -1033,6 +1032,7 @@ void Plugin::BitcodeDidTranslate(int32_t pp_error) {
       wrapper.get(), &error_info,
       false, /* enable_dyncode_syscalls */
       false, /* enable_exception_handling */
+      true, /* enable_crash_throttling */
       callback_factory_.NewCallback(&Plugin::BitcodeDidTranslateContinuation),
       callback_factory_.NewCallback(&Plugin::NexeDidCrash));
 
@@ -1211,23 +1211,15 @@ void Plugin::ProcessNaClManifest(const nacl::string& manifest_json) {
       if (this->nacl_interface()->IsPnaclEnabled()) {
         // Check whether PNaCl has been crashing "frequently".  If so, report
         // a load error.
-        ModulePpapi* module_ppapi =
-            static_cast<ModulePpapi*>(pp::Module::Get());
-        if (module_ppapi->IsPluginUnstable()) {
-          error_info.SetReport(ERROR_PNACL_CRASH_THROTTLED,
-                               "PNaCl has been temporarily disabled because too"
-                               " many crashes have been observed.");
-        } else {
-          pp::CompletionCallback translate_callback =
-              callback_factory_.NewCallback(&Plugin::BitcodeDidTranslate);
-          // Will always call the callback on success or failure.
-          pnacl_coordinator_.reset(
-              PnaclCoordinator::BitcodeToNative(this,
-                                                program_url,
-                                                pnacl_options,
-                                                translate_callback));
-          return;
-        }
+        pp::CompletionCallback translate_callback =
+            callback_factory_.NewCallback(&Plugin::BitcodeDidTranslate);
+        // Will always call the callback on success or failure.
+        pnacl_coordinator_.reset(
+            PnaclCoordinator::BitcodeToNative(this,
+                                              program_url,
+                                              pnacl_options,
+                                              translate_callback));
+        return;
       } else {
         error_info.SetReport(ERROR_PNACL_NOT_ENABLED,
                              "PNaCl has not been enabled (e.g., by setting "
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h
index 1e9e780..416ab5b 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin.h
@@ -106,6 +106,7 @@ class Plugin : public pp::InstancePrivate {
   bool LoadNaClModule(nacl::DescWrapper* wrapper, ErrorInfo* error_info,
                       bool enable_dyncode_syscalls,
                       bool enable_exception_handling,
+                      bool enable_crash_throttling,
                       const pp::CompletionCallback& init_done_cb,
                       const pp::CompletionCallback& crash_cb);
 
diff --git a/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.cc b/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.cc
index 2714eac..486696b 100644
--- a/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.cc
+++ b/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.cc
@@ -24,6 +24,7 @@ bool SelLdrLauncherChrome::Start(PP_Instance instance,
                                  bool enable_ppapi_dev,
                                  bool enable_dyncode_syscalls,
                                  bool enable_exception_handling,
+                                 bool enable_crash_throttling,
                                  nacl::string* error_message) {
   *error_message = "";
   if (!launch_nacl_process)
@@ -37,6 +38,7 @@ bool SelLdrLauncherChrome::Start(PP_Instance instance,
                           PP_FromBool(enable_ppapi_dev),
                           PP_FromBool(enable_dyncode_syscalls),
                           PP_FromBool(enable_exception_handling),
+                          PP_FromBool(enable_crash_throttling),
                           &channel_,
                           &var_error_message) != PP_EXTERNAL_PLUGIN_OK) {
     pp::Var var_error_message_cpp(pp::PASS_REF, var_error_message);
diff --git a/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.h b/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.h
index a64c66b..69c1716 100644
--- a/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.h
+++ b/ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.h
@@ -20,6 +20,7 @@ class SelLdrLauncherChrome : public nacl::SelLdrLauncherBase {
                      bool enable_ppapi_dev,
                      bool enable_dyncode_syscalls,
                      bool enable_exception_handling,
+                     bool enable_crash_throttling,
                      nacl::string* error_message);
 };
 
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index 2b43b2e6..b7af0c1 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -736,6 +736,7 @@ bool ServiceRuntime::StartSelLdr(const SelLdrStartParams& params) {
                                        params.enable_dev_interfaces,
                                        params.enable_dyncode_syscalls,
                                        params.enable_exception_handling,
+                                       params.enable_crash_throttling,
                                        &error_message);
   if (!started) {
     NaClLog(LOG_ERROR, "ServiceRuntime::Start (start failed)\n");
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index ee97710..e7ebbbc 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -56,14 +56,16 @@ struct SelLdrStartParams {
                     bool uses_ppapi,
                     bool enable_dev_interfaces,
                     bool enable_dyncode_syscalls,
-                    bool enable_exception_handling)
+                    bool enable_exception_handling,
+                    bool enable_crash_throttling)
       : url(url),
         error_info(error_info),
         uses_irt(uses_irt),
         uses_ppapi(uses_ppapi),
         enable_dev_interfaces(enable_dev_interfaces),
         enable_dyncode_syscalls(enable_dyncode_syscalls),
-        enable_exception_handling(enable_exception_handling) {
+        enable_exception_handling(enable_exception_handling),
+        enable_crash_throttling(enable_crash_throttling) {
   }
   nacl::string url;
   ErrorInfo* error_info;
@@ -72,6 +74,7 @@ struct SelLdrStartParams {
   bool enable_dev_interfaces;
   bool enable_dyncode_syscalls;
   bool enable_exception_handling;
+  bool enable_crash_throttling;
 };
 
 // Callback resources are essentially our continuation state.
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 8bb17f7..ab66d82 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
@@ -2809,9 +2809,9 @@ static void Pnacl_M13_PPB_Instance_Private_ExecuteScript(struct PP_Var* _struct_
 
 /* Begin wrapper methods for PPB_NaCl_Private_1_0 */
 
-static PP_ExternalPluginResult Pnacl_M13_PPB_NaCl_Private_LaunchSelLdr(PP_Instance instance, const char* alleged_url, PP_Bool uses_irt, PP_Bool uses_ppapi, PP_Bool enable_ppapi_dev, PP_Bool enable_dyncode_syscalls, PP_Bool enable_exception_handling, void* imc_handle, struct PP_Var* error_message) {
+static PP_ExternalPluginResult Pnacl_M13_PPB_NaCl_Private_LaunchSelLdr(PP_Instance instance, const char* alleged_url, PP_Bool uses_irt, PP_Bool uses_ppapi, PP_Bool enable_ppapi_dev, PP_Bool enable_dyncode_syscalls, PP_Bool enable_exception_handling, PP_Bool enable_crash_throttling, void* imc_handle, struct PP_Var* error_message) {
   const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
-  return iface->LaunchSelLdr(instance, alleged_url, uses_irt, uses_ppapi, enable_ppapi_dev, enable_dyncode_syscalls, enable_exception_handling, imc_handle, error_message);
+  return iface->LaunchSelLdr(instance, alleged_url, uses_irt, uses_ppapi, enable_ppapi_dev, enable_dyncode_syscalls, enable_exception_handling, enable_crash_throttling, imc_handle, error_message);
 }
 
 static PP_ExternalPluginResult Pnacl_M13_PPB_NaCl_Private_StartPpapiProxy(PP_Instance instance) {
@@ -4655,7 +4655,7 @@ struct PPB_Instance_Private_0_1 Pnacl_Wrappers_PPB_Instance_Private_0_1 = {
 };
 
 struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = {
-    .LaunchSelLdr = (PP_ExternalPluginResult (*)(PP_Instance instance, const char* alleged_url, PP_Bool uses_irt, PP_Bool uses_ppapi, PP_Bool enable_ppapi_dev, PP_Bool enable_dyncode_syscalls, PP_Bool enable_exception_handling, void* imc_handle, struct PP_Var* error_message))&Pnacl_M13_PPB_NaCl_Private_LaunchSelLdr,
+    .LaunchSelLdr = (PP_ExternalPluginResult (*)(PP_Instance instance, const char* alleged_url, PP_Bool uses_irt, PP_Bool uses_ppapi, PP_Bool enable_ppapi_dev, PP_Bool enable_dyncode_syscalls, PP_Bool enable_exception_handling, PP_Bool enable_crash_throttling, void* imc_handle, struct PP_Var* error_message))&Pnacl_M13_PPB_NaCl_Private_LaunchSelLdr,
     .StartPpapiProxy = (PP_ExternalPluginResult (*)(PP_Instance instance))&Pnacl_M13_PPB_NaCl_Private_StartPpapiProxy,
     .UrandomFD = (int32_t (*)(void))&Pnacl_M13_PPB_NaCl_Private_UrandomFD,
     .Are3DInterfacesDisabled = (PP_Bool (*)(void))&Pnacl_M13_PPB_NaCl_Private_Are3DInterfacesDisabled,