Introduce a MetricsDataProvider interface.

chrome/browser/metrics/metrics_service.cc

 // Copyright (c) 2012 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
 //
 // A MetricsService instance is typically created at application startup.  It is
@@ skipping 1150 matching lines @@
 
   // Put incremental data (histogram deltas, and realtime stats deltas) at the
   // end of all log transmissions (initial log handles this separately).
   // RecordIncrementalStabilityElements only exists on the derived
   // MetricsLog class.
   MetricsLog* current_log =
       static_cast<MetricsLog*>(log_manager_.current_log());
   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();
 }
 
 void MetricsService::PushPendingLogsToPersistentStorage() {
   if (state_ < SENDING_INITIAL_STABILITY_LOG)
     return;  // We didn't and still don't have time to get plugin list etc.
 
   if (log_manager_.has_staged_log()) {
     // We may race here, and send second copy of the log later.
@@ skipping 231 matching lines @@
 
   scoped_ptr<MetricsLog> initial_stability_log(
       new MetricsLog(state_manager_->client_id(), session_id_,
                      MetricsLog::INITIAL_STABILITY_LOG));
 
   // Do not call NotifyOnDidCreateMetricsLog here because the stability
   // log describes stats from the _previous_ session.
 
   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();
 #endif  // defined(OS_ANDROID)
   log_manager_.FinishCurrentLog();
   log_manager_.ResumePausedLog();
 
   // Store unsent logs, including the stability log that was just saved, so
   // that they're not lost in case of a crash before upload time.
   log_manager_.PersistUnsentLogs();
 
   has_initial_stability_log_ = true;
 }
 
 void MetricsService::PrepareInitialMetricsLog() {
   DCHECK(state_ == INIT_TASK_DONE || state_ == SENDING_INITIAL_STABILITY_LOG);
   initial_metrics_log_->set_hardware_class(hardware_class_);
 
   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.
   log_manager_.PauseCurrentLog();
   log_manager_.BeginLoggingWithLog(initial_metrics_log_.release());
 #if defined(OS_ANDROID)
   ConvertAndroidStabilityPrefsToHistograms(pref);
 #endif  // defined(OS_ANDROID)
   RecordCurrentHistograms();
   log_manager_.FinishCurrentLog();
@@ skipping 241 matching lines @@
       }
       return;
     }
   }
 
   SyntheticTrialGroup trial_group = trial;
   trial_group.start_time = base::TimeTicks::Now();
   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();
 }
 
 void MetricsService::GetCurrentSyntheticFieldTrials(
     std::vector<chrome_variations::ActiveGroupId>* synthetic_trials) {
   DCHECK(synthetic_trials);
   synthetic_trials->clear();
   const MetricsLog* current_log =
       static_cast<const MetricsLog*>(log_manager_.current_log());
@@ skipping 227 matching lines @@
   if (metrics_service)
     metrics_service->AddObserver(observer);
 }
 
 void MetricsServiceHelper::RemoveMetricsServiceObserver(
     MetricsServiceObserver* observer) {
   MetricsService* metrics_service = g_browser_process->metrics_service();
   if (metrics_service)
     metrics_service->RemoveObserver(observer);
 }