Introduce a MetricsDataProvider interface.

chrome/browser/metrics/metrics_service.cc

Index: chrome/browser/metrics/metrics_service.cc
===================================================================
--- chrome/browser/metrics/metrics_service.cc	(revision 271296)
+++ chrome/browser/metrics/metrics_service.cc	(working copy)
@@ -1168,15 +1168,17 @@
   DCHECK(current_log);
   std::vector<chrome_variations::ActiveGroupId> synthetic_trials;
   GetCurrentSyntheticFieldTrials(&synthetic_trials);
-  current_log->RecordEnvironment(plugins_, google_update_metrics_,
-                                 synthetic_trials);
+  current_log->RecordEnvironment(data_providers_, plugins_,
+                                 google_update_metrics_, synthetic_trials);
   PrefService* pref = g_browser_process->local_state();
   base::TimeDelta incremental_uptime;
   base::TimeDelta uptime;
   GetUptimes(pref, &incremental_uptime, &uptime);
-  current_log->RecordStabilityMetrics(incremental_uptime, uptime);
+  current_log->RecordStabilityMetrics(data_providers_, incremental_uptime,
+                                      uptime);
 
   RecordCurrentHistograms();
+  current_log->RecordGeneralMetrics(data_providers_);

[Ilya Sherman, 2014/05/19 14:45:00]

Does it make sense to fold this into RecordEnvironment?

[Alexei Svitkine (slow), 2014/05/19 15:23:02]

I don't think it does. RecordEnvironment() records the system profile proto.
Whereas general metrics would be things like omnibox events, which shouldn't
necessarily be recorded every time we record the system profile.

 
   log_manager_.FinishCurrentLog();
 }
@@ -1428,12 +1430,19 @@
 
   if (!initial_stability_log->LoadSavedEnvironmentFromPrefs())
     return;
-  initial_stability_log->RecordStabilityMetrics(base::TimeDelta(),
-                                                base::TimeDelta());
+
   log_manager_.LoadPersistedUnsentLogs();
 
   log_manager_.PauseCurrentLog();
+  MetricsLog* initial_stability_log_ptr = initial_stability_log.get();

[Ilya Sherman, 2014/05/19 14:45:00]

Optiona nit: Perhaps it would be cleaner to access the log as
log_manager_.current_log()?  (Imagine it without the static_cast, since
eventually that will go away.)

[Alexei Svitkine (slow), 2014/05/19 15:23:02]

Fair enough, the static cast is why I did this and you're right that it will go
away. Done.

   log_manager_.BeginLoggingWithLog(initial_stability_log.release());
+
+  // Note: Some stability providers may record stability stats via histograms,
+  //       so this call has to be after BeginLoggingWithLog().
+  initial_stability_log_ptr->RecordStabilityMetrics(data_providers_,
+                                                    base::TimeDelta(),
+                                                    base::TimeDelta());
+
 #if defined(OS_ANDROID)
   ConvertAndroidStabilityPrefsToHistograms(pref);
   RecordCurrentStabilityHistograms();
@@ -1454,13 +1463,15 @@
 
   std::vector<chrome_variations::ActiveGroupId> synthetic_trials;
   GetCurrentSyntheticFieldTrials(&synthetic_trials);
-  initial_metrics_log_->RecordEnvironment(plugins_, google_update_metrics_,
+  initial_metrics_log_->RecordEnvironment(data_providers_, plugins_,
+                                          google_update_metrics_,
                                           synthetic_trials);
   PrefService* pref = g_browser_process->local_state();
   base::TimeDelta incremental_uptime;
   base::TimeDelta uptime;
   GetUptimes(pref, &incremental_uptime, &uptime);
-  initial_metrics_log_->RecordStabilityMetrics(incremental_uptime, uptime);
+  initial_metrics_log_->RecordStabilityMetrics(data_providers_,
+                                               incremental_uptime, uptime);

[Ilya Sherman, 2014/05/19 14:45:00]

Are you intentionally not recording general metrics as part of this log?

[Alexei Svitkine (slow), 2014/05/19 15:23:02]

Oops, it was indeed not intentional. There was also an additional issue with
RecordStabilityMetrics not being moved below BeginLogginWithLog(), so I fixed
this now here too. (And yes, we should consolidate these methods more... but not
today.)

 
   // Histograms only get written to the current log, so make the new log current
   // before writing them.
@@ -1722,6 +1733,13 @@
   synthetic_trial_groups_.push_back(trial_group);
 }
 
+void MetricsService::RegisterDataProvider(
+    scoped_ptr<metrics::MetricsDataProvider> provider) {
+  DCHECK_EQ(INITIALIZED, state_);
+

[Ilya Sherman, 2014/05/19 14:45:00]

Optional nit: I'd omit this blank line.

[Alexei Svitkine (slow), 2014/05/19 15:23:02]

Done.

+  data_providers_.push_back(provider.release());
+}
+
 void MetricsService::CheckForClonedInstall() {
   state_manager_->CheckForClonedInstall();
 }