Move child process observing from MetricsService to stability provider.

Also moves the prefs registration to the stability provider.

BUG=374206

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

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

chrome/browser/metrics/chrome_stability_metrics_provider.cc

@@ -9,17 +9,23 @@
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
+#include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "components/metrics/proto/system_profile.pb.h"
+#include "content/public/browser/child_process_data.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/process_map.h"
 
+#if defined(ENABLE_PLUGINS)
+#include "chrome/browser/metrics/plugin_metrics_provider.h"
+#endif
+
 #if defined(OS_WIN)
 #include <windows.h>  // Needed for STATUS_* codes
 #endif
@@ -57,9 +63,11 @@ int MapCrashExitCodeForHistogram(int exit_code) {
 }  // namespace
 
 ChromeStabilityMetricsProvider::ChromeStabilityMetricsProvider() {
+  BrowserChildProcessObserver::Add(this);
 }
 
 ChromeStabilityMetricsProvider::~ChromeStabilityMetricsProvider() {
+  BrowserChildProcessObserver::Remove(this);
 }
 
 void ChromeStabilityMetricsProvider::OnRecordingEnabled() {
@@ -109,6 +117,18 @@ void ChromeStabilityMetricsProvider::ProvideStabilityMetrics(
   }
 }
 
+// static
+void ChromeStabilityMetricsProvider::RegisterPrefs(
+    PrefRegistrySimple* registry) {
+  registry->RegisterIntegerPref(prefs::kStabilityPageLoadCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityRendererCrashCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityExtensionRendererCrashCount,
+                                0);
+  registry->RegisterIntegerPref(prefs::kStabilityRendererHangCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityChildProcessCrashCount, 0);
+  registry->RegisterInt64Pref(prefs::kUninstallMetricsPageLoadCount, 0);
+}
+
 void ChromeStabilityMetricsProvider::Observe(
     int type,
     const content::NotificationSource& source,
@@ -143,6 +163,18 @@ void ChromeStabilityMetricsProvider::Observe(
   }
 }
 
+void ChromeStabilityMetricsProvider::BrowserChildProcessCrashed(
+    const content::ChildProcessData& data) {
+#if defined(ENABLE_PLUGINS)
+  // Exclude plugin crashes from the count below because we report them via
+  // a separate UMA metric.
+  if (PluginMetricsProvider::IsPluginProcess(data.process_type))
+    return;
+#endif
+
+  IncrementPrefValue(prefs::kStabilityChildProcessCrashCount);
+}
+
 void ChromeStabilityMetricsProvider::LogLoadStarted(
     content::WebContents* web_contents) {
   content::RecordAction(base::UserMetricsAction("PageLoad"));

chrome/browser/metrics/chrome_stability_metrics_provider.h

@@ -9,9 +9,12 @@
 #include "base/metrics/user_metrics.h"
 #include "base/process/kill.h"
 #include "components/metrics/metrics_provider.h"
+#include "content/public/browser/browser_child_process_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
+class PrefRegistrySimple;
+
 namespace content {
 class RenderProcessHost;
 class WebContents;
@@ -19,8 +22,10 @@ class WebContents;
 
 // ChromeStabilityMetricsProvider gathers and logs Chrome-specific stability-
 // related metrics.
-class ChromeStabilityMetricsProvider : public metrics::MetricsProvider,
-                                       public content::NotificationObserver {
+class ChromeStabilityMetricsProvider
+    : public metrics::MetricsProvider,
+      public content::BrowserChildProcessObserver,
+      public content::NotificationObserver {
  public:
   ChromeStabilityMetricsProvider();
   virtual ~ChromeStabilityMetricsProvider();
@@ -31,12 +36,19 @@ class ChromeStabilityMetricsProvider : public metrics::MetricsProvider,
   virtual void ProvideStabilityMetrics(
       metrics::SystemProfileProto* system_profile_proto) OVERRIDE;
 
+  // Registers local state prefs used by this class.
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
  private:
   // content::NotificationObserver:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // content::BrowserChildProcessObserver:
+  virtual void BrowserChildProcessCrashed(
+      const content::ChildProcessData& data) OVERRIDE;
+
   // Logs the initiation of a page load and uses |web_contents| to do
   // additional logging of the type of page loaded.
   void LogLoadStarted(content::WebContents* web_contents);

chrome/browser/metrics/metrics_service.cc

@@ -200,9 +200,6 @@
 #include "components/metrics/metrics_reporting_scheduler.h"
 #include "components/metrics/metrics_service_client.h"
 #include "components/variations/entropy_provider.h"
-#include "content/public/browser/child_process_data.h"
-#include "content/public/browser/render_process_host.h"
-#include "content/public/browser/user_metrics.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 
@@ -334,12 +331,6 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(prefs::kStabilityLaunchCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityCrashCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityIncompleteSessionEndCount, 0);
-  registry->RegisterIntegerPref(prefs::kStabilityPageLoadCount, 0);
-  registry->RegisterIntegerPref(prefs::kStabilityRendererCrashCount, 0);
-  registry->RegisterIntegerPref(prefs::kStabilityExtensionRendererCrashCount,
-                                0);
-  registry->RegisterIntegerPref(prefs::kStabilityRendererHangCount, 0);
-  registry->RegisterIntegerPref(prefs::kStabilityChildProcessCrashCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityBreakpadRegistrationFail, 0);
   registry->RegisterIntegerPref(prefs::kStabilityBreakpadRegistrationSuccess,
                                 0);
@@ -360,12 +351,14 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(metrics::prefs::kMetricsOngoingLogs);
 
   registry->RegisterInt64Pref(prefs::kInstallDate, 0);
-  registry->RegisterInt64Pref(prefs::kUninstallMetricsPageLoadCount, 0);
   registry->RegisterInt64Pref(prefs::kUninstallLaunchCount, 0);
   registry->RegisterInt64Pref(prefs::kUninstallMetricsUptimeSec, 0);
   registry->RegisterInt64Pref(prefs::kUninstallLastLaunchTimeSec, 0);
   registry->RegisterInt64Pref(prefs::kUninstallLastObservedRunTimeSec, 0);
 
+  // TODO(asvitkine): Move this out of here.
+  ChromeStabilityMetricsProvider::RegisterPrefs(registry);
+
 #if defined(OS_ANDROID)
   // TODO(asvitkine): Move this out of here.
   AndroidMetricsProvider::RegisterPrefs(registry);
@@ -428,14 +421,10 @@ MetricsService::MetricsService(metrics::MetricsStateManager* state_manager,
   RegisterMetricsProvider(scoped_ptr<metrics::MetricsProvider>(
       plugin_metrics_provider_));
 #endif
-
-  BrowserChildProcessObserver::Add(this);
 }
 
 MetricsService::~MetricsService() {
   DisableRecording();
-
-  BrowserChildProcessObserver::Remove(this);
 }
 
 void MetricsService::InitializeMetricsRecordingState() {
@@ -563,19 +552,6 @@ void MetricsService::InconsistencyDetectedInLoggedCount(int amount) {
                        std::abs(amount));
 }
 
-void MetricsService::BrowserChildProcessCrashed(
-    const content::ChildProcessData& data) {
-  // TODO(asvitkine): Move this into ChromeStabilityStatsProvider.
-#if defined(ENABLE_PLUGINS)
-  // Exclude plugin crashes from the count below because we report them via
-  // a separate UMA metric.
-  if (PluginMetricsProvider::IsPluginProcess(data.process_type))
-    return;
-#endif
-
-  IncrementPrefValue(prefs::kStabilityChildProcessCrashCount);
-}
-
 void MetricsService::HandleIdleSinceLastTransmission(bool in_idle) {
   // If there wasn't a lot of action, maybe the computer was asleep, in which
   // case, the log transmissions should have stopped.  Here we start them up
@@ -1473,4 +1449,3 @@ void MetricsService::RecordCurrentState(PrefService* pref) {
   plugin_metrics_provider_->RecordPluginChanges();
 #endif
 }
-

chrome/browser/metrics/metrics_service.h

@@ -32,8 +32,6 @@
 #include "components/metrics/metrics_provider.h"
 #include "components/metrics/metrics_service_observer.h"
 #include "components/variations/active_field_trials.h"
-#include "content/public/browser/browser_child_process_observer.h"
-#include "content/public/browser/user_metrics.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
 class GoogleUpdateMetricsProviderWin;
@@ -91,7 +89,6 @@ struct SyntheticTrialGroup {
 class MetricsService
     : public base::HistogramFlattener,
       public chrome_browser_metrics::TrackingSynchronizerObserver,
-      public content::BrowserChildProcessObserver,
       public net::URLFetcherDelegate {
  public:
   // The execution phase of the browser.
@@ -171,10 +168,6 @@ class MetricsService
       base::HistogramBase::Inconsistency problem) OVERRIDE;
   virtual void InconsistencyDetectedInLoggedCount(int amount) OVERRIDE;
 
-  // Implementation of content::BrowserChildProcessObserver
-  virtual void BrowserChildProcessCrashed(
-      const content::ChildProcessData& data) OVERRIDE;
-
   // This should be called when the application is not idle, i.e. the user seems
   // to be interacting with the application.
   void OnApplicationNotIdle();