Merge 33794 - Improve reporting of subprocess crashes.

Split extension renderer crashes out of the existing UMA renderer crash metric 
into its own metric.

Add a new metric for the sum of all ChildProcessHost crashes.

Add histograms for each crash type.


BUG=28022
TEST=We should start getting more crash reports in UMA and histograms.

Review URL: http://codereview.chromium.org/468005

TBR=asargent@chromium.org
Review URL: http://codereview.chromium.org/501064

git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@34768 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 75 insertions, 9 deletions

diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index 025fea8..18441d4 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -447,11 +447,23 @@ void MetricsLog::WriteRealtimeStabilityAttributes(PrefService* pref) {
     pref->SetInteger(prefs::kStabilityRendererCrashCount, 0);
   }
 
+  count = pref->GetInteger(prefs::kStabilityExtensionRendererCrashCount);
+  if (count) {
+    WriteIntAttribute("extensionrenderercrashcount", count);
+    pref->SetInteger(prefs::kStabilityExtensionRendererCrashCount, 0);
+  }
+
   count = pref->GetInteger(prefs::kStabilityRendererHangCount);
   if (count) {
     WriteIntAttribute("rendererhangcount", count);
     pref->SetInteger(prefs::kStabilityRendererHangCount, 0);
   }
+
+  count = pref->GetInteger(prefs::kStabilityChildProcessCrashCount);
+  if (count) {
+    WriteIntAttribute("childprocesscrashcount", count);
+    pref->SetInteger(prefs::kStabilityChildProcessCrashCount, 0);
+  }
 }
 
 void MetricsLog::WritePluginList(
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index c49886a..12bc07d 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -331,7 +331,10 @@ void MetricsService::RegisterPrefs(PrefService* local_state) {
                                    0);
   local_state->RegisterIntegerPref(prefs::kStabilityPageLoadCount, 0);
   local_state->RegisterIntegerPref(prefs::kStabilityRendererCrashCount, 0);
+  local_state->RegisterIntegerPref(prefs::kStabilityExtensionRendererCrashCount,
+                                   0);
   local_state->RegisterIntegerPref(prefs::kStabilityRendererHangCount, 0);
+  local_state->RegisterIntegerPref(prefs::kStabilityChildProcessCrashCount, 0);
   local_state->RegisterIntegerPref(prefs::kStabilityBreakpadRegistrationFail,
                                    0);
   local_state->RegisterIntegerPref(prefs::kStabilityBreakpadRegistrationSuccess,
@@ -558,8 +561,17 @@ void MetricsService::Observe(NotificationType type,
       break;
 
     case NotificationType::RENDERER_PROCESS_CLOSED:
-      if (*Details<bool>(details).ptr())
-        LogRendererCrash();
+      {
+        RenderProcessHost::RendererClosedDetails* process_details =
+            Details<RenderProcessHost::RendererClosedDetails>(details).ptr();
+        if (process_details->did_crash) {
+          if (process_details->was_extension_renderer) {
+            LogExtensionRendererCrash();
+          } else {
+            LogRendererCrash();
+          }
+        }
+      }
       break;
 
     case NotificationType::RENDERER_PROCESS_HANG:
@@ -1665,6 +1677,10 @@ void MetricsService::LogRendererCrash() {
   IncrementPrefValue(prefs::kStabilityRendererCrashCount);
 }
 
+void MetricsService::LogExtensionRendererCrash() {
+  IncrementPrefValue(prefs::kStabilityExtensionRendererCrashCount);
+}
+
 void MetricsService::LogRendererHang() {
   IncrementPrefValue(prefs::kStabilityRendererHangCount);
 }
@@ -1676,7 +1692,6 @@ void MetricsService::LogChildProcessChange(
   Details<ChildProcessInfo> child_details(details);
   const std::wstring& child_name = child_details->name();
 
-
   if (child_process_stats_buffer_.find(child_name) ==
       child_process_stats_buffer_.end()) {
     child_process_stats_buffer_[child_name] =
@@ -1695,6 +1710,11 @@ void MetricsService::LogChildProcessChange(
 
     case NotificationType::CHILD_PROCESS_CRASHED:
       stats.process_crashes++;
+      // Exclude plugin crashes from the count below because we report them via
+      // a separate UMA metric.
+      if (child_details->type() != ChildProcessInfo::PLUGIN_PROCESS) {
+        IncrementPrefValue(prefs::kStabilityChildProcessCrashCount);
+      }
       break;
 
     default:
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 9fa21fc..7c41fd8 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -338,6 +338,9 @@ class MetricsService : public NotificationObserver,
   // Records a renderer process crash.
   void LogRendererCrash();
 
+  // Records an extension renderer process crash.
+  void LogExtensionRendererCrash();
+
   // Records a renderer process hang.
   void LogRendererHang();
 
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 534991a..c231d69 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -801,10 +801,16 @@ void BrowserRenderProcessHost::OnChannelError() {
   bool did_crash =
       child_process_.get() ? child_process_->DidProcessCrash() : false;
 
+  if (did_crash) {
+    UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.ChildCrashes",
+                             extension_process_ ? 2 : 1);
+  }
+
+  RendererClosedDetails details(did_crash, extension_process_);
   NotificationService::current()->Notify(
       NotificationType::RENDERER_PROCESS_CLOSED,
       Source<RenderProcessHost>(this),
-      Details<bool>(&did_crash));
+      Details<RendererClosedDetails>(&details));
 
   WebCacheManager::GetInstance()->Remove(id());
   child_process_.reset();
diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h
index e91e3c0..ab47f57 100644
--- a/chrome/browser/renderer_host/render_process_host.h
+++ b/chrome/browser/renderer_host/render_process_host.h
@@ -46,6 +46,16 @@ class RenderProcessHost : public IPC::Channel::Sender,
     TYPE_EXTENSION,  // Renderer with extension privileges.
   };
 
+  // Details for RENDERER_PROCESS_CLOSED notifications.
+  struct RendererClosedDetails {
+    RendererClosedDetails(bool did_crash, bool was_extension_renderer) {
+      this->did_crash = did_crash;
+      this->was_extension_renderer = was_extension_renderer;
+    }
+    bool did_crash;
+    bool was_extension_renderer;
+  };
+
   explicit RenderProcessHost(Profile* profile);
   virtual ~RenderProcessHost();
 
diff --git a/chrome/common/child_process_host.cc b/chrome/common/child_process_host.cc
index a34621b..50d7307 100644
--- a/chrome/common/child_process_host.cc
+++ b/chrome/common/child_process_host.cc
@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
+#include "base/histogram.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
@@ -179,6 +180,7 @@ void ChildProcessHost::OnChildDied() {
       OnProcessCrashed();
       // Report that this child process crashed.
       Notify(NotificationType::CHILD_PROCESS_CRASHED);
+      UMA_HISTOGRAM_COUNTS("ChildProcess.Crashes", this->type());
     }
     // Notify in the main loop of the disconnection.
     Notify(NotificationType::CHILD_PROCESS_HOST_DISCONNECTED);
diff --git a/chrome/common/child_process_info.h b/chrome/common/child_process_info.h
index 31b1cb84..50d1d0d 100644
--- a/chrome/common/child_process_info.h
+++ b/chrome/common/child_process_info.h
@@ -12,7 +12,10 @@
 // Holds information about a child process.
 class ChildProcessInfo {
  public:
+  // NOTE: Do not remove or reorder the elements in this enum, and only add new
+  // items at the end. We depend on these specific values in a histogram.
   enum ProcessType {
+    UNKNOWN_PROCESS = 1,
     BROWSER_PROCESS,
     RENDER_PROCESS,
     PLUGIN_PROCESS,
@@ -21,8 +24,7 @@ class ChildProcessInfo {
     UTILITY_PROCESS,
     PROFILE_IMPORT_PROCESS,
     ZYGOTE_PROCESS,
-    SANDBOX_HELPER_PROCESS,
-    UNKNOWN_PROCESS,
+    SANDBOX_HELPER_PROCESS
   };
 
   ChildProcessInfo(const ChildProcessInfo& original);
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index 6f69d3e..97dd7de 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -357,9 +357,8 @@ class NotificationType {
 
     // Indicates that a render process was closed (meaning it exited, but the
     // RenderProcessHost might be reused).  The source will be the corresponding
-    // RenderProcessHost.  The details will be a bool which is true if the
-    // process crashed.  This may get sent along with
-    // RENDERER_PROCESS_TERMINATED.
+    // RenderProcessHost.  The details will be a RendererClosedDetails struct.
+    // This may get sent along with RENDERER_PROCESS_TERMINATED.
     RENDERER_PROCESS_CLOSED,
 
     // Indicates that a render process has become unresponsive for a period of
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 8245531..4f78b06 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -376,6 +376,10 @@ const wchar_t kStabilityPageLoadCount[] =
 const wchar_t kStabilityRendererCrashCount[] =
     L"user_experience_metrics.stability.renderer_crash_count";
 
+// Number of times an extension renderer process crashed since the last report.
+const wchar_t kStabilityExtensionRendererCrashCount[] =
+    L"user_experience_metrics.stability.extension_renderer_crash_count";
+
 // Time when the app was last launched, in seconds since the epoch.
 const wchar_t kStabilityLaunchTimeSec[] =
     L"user_experience_metrics.stability.launch_time_sec";
@@ -399,6 +403,12 @@ const wchar_t kStabilityPluginStats[] =
 const wchar_t kStabilityRendererHangCount[] =
     L"user_experience_metrics.stability.renderer_hang_count";
 
+// Total number of child process crashes (other than renderer / extension
+// renderer ones, and plugin children, which are counted separately) since the
+// last report.
+const wchar_t kStabilityChildProcessCrashCount[] =
+    L"user_experience_metrics.stability.child_process_crash_count";
+
 // Number of times the browser has been able to register crash reporting.
 const wchar_t kStabilityBreakpadRegistrationSuccess[] =
     L"user_experience_metrics.stability.breakpad_registration_ok";
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 9a97244..1e3175c 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -140,10 +140,12 @@ extern const wchar_t kStabilityCrashCount[];
 extern const wchar_t kStabilityIncompleteSessionEndCount[];
 extern const wchar_t kStabilityPageLoadCount[];
 extern const wchar_t kStabilityRendererCrashCount[];
+extern const wchar_t kStabilityExtensionRendererCrashCount[];
 extern const wchar_t kStabilityLaunchTimeSec[];
 extern const wchar_t kStabilityLastTimestampSec[];
 extern const wchar_t kStabilityUptimeSec[];
 extern const wchar_t kStabilityRendererHangCount[];
+extern const wchar_t kStabilityChildProcessCrashCount[];
 
 extern const wchar_t kStabilityBreakpadRegistrationSuccess[];
 extern const wchar_t kStabilityBreakpadRegistrationFail[];