Improve reporting of subprocess crashes....

chrome/browser/metrics/metrics_service.cc

 // Copyright (c) 2009 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.
 
 
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
@@ skipping 313 matching lines @@
   local_state->RegisterStringPref(prefs::kStabilityStatsVersion, L"");
   local_state->RegisterBooleanPref(prefs::kStabilityExitedCleanly, true);
   local_state->RegisterBooleanPref(prefs::kStabilitySessionEndCompleted, true);
   local_state->RegisterIntegerPref(prefs::kMetricsSessionID, -1);
   local_state->RegisterIntegerPref(prefs::kStabilityLaunchCount, 0);
   local_state->RegisterIntegerPref(prefs::kStabilityCrashCount, 0);
   local_state->RegisterIntegerPref(prefs::kStabilityIncompleteSessionEndCount,
                                    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,
                                    0);
   local_state->RegisterIntegerPref(prefs::kStabilityDebuggerPresent, 0);
   local_state->RegisterIntegerPref(prefs::kStabilityDebuggerNotPresent, 0);
 
   local_state->RegisterDictionaryPref(prefs::kProfileMetrics);
   local_state->RegisterIntegerPref(prefs::kNumBookmarksOnBookmarkBar, 0);
   local_state->RegisterIntegerPref(prefs::kNumFoldersOnBookmarkBar, 0);
@@ skipping 206 matching lines @@
 
     case NotificationType::LOAD_STOP:
       LogLoadComplete(type, source, details);
       break;
 
     case NotificationType::LOAD_START:
       LogLoadStarted();
       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:
       LogRendererHang();
       break;
 
     case NotificationType::CHILD_PROCESS_HOST_CONNECTED:
     case NotificationType::CHILD_PROCESS_CRASHED:
     case NotificationType::CHILD_INSTANCE_CREATED:
       LogChildProcessChange(type, source, details);
@@ skipping 1085 matching lines @@
   IncrementPrefValue(prefs::kStabilityPageLoadCount);
   IncrementLongPrefsValue(prefs::kUninstallMetricsPageLoadCount);
   // We need to save the prefs, as page load count is a critical stat, and it
   // might be lost due to a crash :-(.
 }
 
 void MetricsService::LogRendererCrash() {
   IncrementPrefValue(prefs::kStabilityRendererCrashCount);
 }
 
+void MetricsService::LogExtensionRendererCrash() {
+  IncrementPrefValue(prefs::kStabilityExtensionRendererCrashCount);
+}
+
 void MetricsService::LogRendererHang() {
   IncrementPrefValue(prefs::kStabilityRendererHangCount);
 }
 
 void MetricsService::LogChildProcessChange(
     NotificationType type,
     const NotificationSource& source,
     const NotificationDetails& details) {
   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] =
         ChildProcessStats(child_details->type());
   }
 
   ChildProcessStats& stats = child_process_stats_buffer_[child_name];
   switch (type.value) {
     case NotificationType::CHILD_PROCESS_HOST_CONNECTED:
       stats.process_launches++;
       break;
 
     case NotificationType::CHILD_INSTANCE_CREATED:
       stats.instances++;
       break;
 
     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) {

[jar (doing other things), 2009/12/18 16:28:14]

There is a surprising asymmetry in this code.  I'd expect an else clause here to
record the other crashes... or I'd expect this section to only have
stats-counter increments, and then this crash counter to be incremented along
side the other tally.

[asargent_no_longer_on_chrome, 2010/01/05 01:01:19]

The cause of this weirdness is probably that the stats struct is used to count
per-plugin values, whereas I am keeping a global counter of all non-renderer
child process crashes.

I'll prepare a separate CL that tries to make this a little more clear.

+        IncrementPrefValue(prefs::kStabilityChildProcessCrashCount);
+      }
       break;
 
     default:
       NOTREACHED() << "Unexpected notification type " << type.value;
       return;
   }
 }
 
 // Recursively counts the number of bookmarks and folders in node.
 static void CountBookmarks(const BookmarkNode* node,
@@ skipping 192 matching lines @@
     thread_id = PlatformThread::CurrentId();
   return PlatformThread::CurrentId() == thread_id;
 }
 
 #if defined(OS_CHROMEOS)
 void MetricsService::StartExternalMetrics(Profile* profile) {
   external_metrics_ = new chromeos::ExternalMetrics;
   external_metrics_->Start(profile);
 }
 #endif