Implement per-browser crash throttling for NaCl processes.

This CL replicates the crash logic used to throttle plugins.

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

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

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

8 files changed, 61 insertions, 56 deletions

diff --git a/chrome/browser/nacl_host/nacl_browser.cc b/chrome/browser/nacl_host/nacl_browser.cc
index 8460124..180de94 100644
--- a/chrome/browser/nacl_host/nacl_browser.cc
+++ b/chrome/browser/nacl_host/nacl_browser.cc
@@ -12,6 +12,7 @@
 #include "base/pickle.h"
 #include "base/rand_util.h"
 #include "base/strings/string_split.h"
+#include "base/time/time.h"
 #include "base/win/windows_version.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_thread.h"
@@ -112,6 +113,10 @@ void LogCacheSet(ValidationCacheStatus status) {
   UMA_HISTOGRAM_ENUMERATION("NaCl.ValidationCache.Set", status, CACHE_MAX);
 }
 
+// Crash throttling parameters.
+const size_t kMaxCrashesPerInterval = 3;
+const int64 kCrashesIntervalInSeconds = 120;
+
 }  // namespace
 
 namespace nacl {
@@ -576,3 +581,21 @@ void NaClBrowser::PersistValidationCache() {
   }
   validation_cache_is_modified_ = false;
 }
+
+void NaClBrowser::NotifyOfCrash() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  if (crash_times_.size() == kMaxCrashesPerInterval) {
+    crash_times_.pop_front();
+  }
+  base::Time time = base::Time::Now();
+  crash_times_.push_back(time);
+}
+
+bool NaClBrowser::TooManyCrashes() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  if (crash_times_.size() != kMaxCrashesPerInterval) {
+    return false;
+  }
+  base::TimeDelta delta = base::Time::Now() - crash_times_.front();
+  return delta.InSeconds() <= kCrashesIntervalInSeconds;
+}
diff --git a/chrome/browser/nacl_host/nacl_browser.h b/chrome/browser/nacl_host/nacl_browser.h
index f9c6d9c..80e7968 100644
--- a/chrome/browser/nacl_host/nacl_browser.h
+++ b/chrome/browser/nacl_host/nacl_browser.h
@@ -5,12 +5,15 @@
 #ifndef CHROME_BROWSER_NACL_HOST_NACL_BROWSER_H_
 #define CHROME_BROWSER_NACL_HOST_NACL_BROWSER_H_
 
+#include <deque>
+
 #include "base/bind.h"
 #include "base/containers/mru_cache.h"
 #include "base/files/file_util_proxy.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/platform_file.h"
+#include "base/time/time.h"
 #include "chrome/browser/nacl_host/nacl_validation_cache.h"
 #include "components/nacl/common/nacl_browser_delegate.h"
 
@@ -119,6 +122,14 @@ class NaClBrowser {
   void EarlyStartup();
   static void SetDelegate(NaClBrowserDelegate* delegate);
   static NaClBrowserDelegate* GetDelegate();
+
+  // Support for NaCl crash throttling.
+  // Each time a NaCl module crashes, the browser is notified.
+  void NotifyOfCrash();
+  // If "too many" crashes occur within a given time period, NaCl is throttled
+  // until the rate again drops below the threshold.
+  bool TooManyCrashes();
+
  private:
   friend struct DefaultSingletonTraits<NaClBrowser>;
 
@@ -177,6 +188,9 @@ class NaClBrowser {
   std::vector<base::Closure> waiting_;
 
   scoped_ptr<NaClBrowserDelegate> browser_delegate_;
+
+  std::deque<base::Time> crash_times_;
+
   DISALLOW_COPY_AND_ASSIGN(NaClBrowser);
 };
 
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index c915a5f..cc97d9e 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -252,6 +252,10 @@ NaClProcessHost::~NaClProcessHost() {
 #endif
 }
 
+void NaClProcessHost::OnProcessCrashed(int exit_status) {
+  NaClBrowser::GetInstance()->NotifyOfCrash();
+}
+
 // This is called at browser startup.
 // static
 void NaClProcessHost::EarlyStartup(NaClBrowserDelegate* delegate) {
@@ -284,6 +288,15 @@ void NaClProcessHost::Launch(
   reply_msg_ = reply_msg;
   manifest_path_ = manifest_path;
 
+  // Do not launch the requested NaCl module if NaCl is marked "unstable" due
+  // to too many crashes within a given time period.
+  if (NaClBrowser::GetInstance()->TooManyCrashes()) {
+    SendErrorToRenderer("Process creation was throttled due to excessive"
+                        " crashes");
+    delete this;
+    return;
+  }
+
   const CommandLine* cmd = CommandLine::ForCurrentProcess();
 #if defined(OS_WIN)
   if (cmd->HasSwitch(switches::kEnableNaClDebug) &&
diff --git a/chrome/browser/nacl_host/nacl_process_host.h b/chrome/browser/nacl_host/nacl_process_host.h
index f2133a1..f32d92b 100644
--- a/chrome/browser/nacl_host/nacl_process_host.h
+++ b/chrome/browser/nacl_host/nacl_process_host.h
@@ -59,6 +59,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
                   const base::FilePath& profile_directory);
   virtual ~NaClProcessHost();
 
+  virtual void OnProcessCrashed(int exit_status) OVERRIDE;
+
   // Do any minimal work that must be done at browser startup.
   static void EarlyStartup(NaClBrowserDelegate* delegate);
 
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/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index fe27aa2..57bb648 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"
@@ -1010,10 +1009,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) {
@@ -1211,23 +1206,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_error.h b/ppapi/native_client/src/trusted/plugin/plugin_error.h
index cba1701..9da77ca 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin_error.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin_error.h
@@ -98,7 +98,6 @@ enum PluginErrorCode {
   ERROR_PNACL_NOT_ENABLED = 68,
   ERROR_MANIFEST_NOACCESS_URL = 69,
   ERROR_NEXE_NOACCESS_URL = 70,
-  ERROR_PNACL_CRASH_THROTTLED = 71,
   // If you add a code, read the enum comment above on how to update histograms.
   ERROR_MAX
 };