Starting a refactor to allow adding metrics on upload.

components/metrics/metrics_service.cc

Index: components/metrics/metrics_service.cc
diff --git a/components/metrics/metrics_service.cc b/components/metrics/metrics_service.cc
index b6bb8f548f8a2e2ddf5d74eacab5b7f5d0f68d6e..7504a4f0611083b3755bbfc84fe48292df6daacd 100644
--- a/components/metrics/metrics_service.cc
+++ b/components/metrics/metrics_service.cc
@@ -849,6 +849,9 @@ void MetricsService::StartScheduledUpload() {
     log_manager_.StageNextLogForUpload();
     SendStagedLog();
   } else {
+    for (size_t i = 0; i < metrics_providers_.size(); ++i)
+      metrics_providers_[i]->OnCollectFinalMetrics();

[Alexei Svitkine (slow), 2014/10/17 18:30:52]

Can you call this from DisableRecording() too?

Per my previous reply - this will cover cases where a log is closed without
hitting CollectFinalMetrics().

[Maria, 2014/10/17 21:44:50]

It's a little weird I think to call onCollectFinalMetrics from DisableRecording
without calling CollectFinalMetrics. What's the reason we don't call that method
before disabling?

Do you think it makes sense to add a call from OnDisableRecording to
OnCollectFinalMetrics inside the provider?

[Alexei Svitkine (slow), 2014/10/20 15:47:00]

It's not done because CollectFinalMetrics() involves some async work (i.e.
contact subprocesses to get their metrics) and the DisableRecording codepath is
executed on a shutdown when closing the final log. So I think the idea is to not
block shut down due to async work.
I'd prefer we do it in metrics service, so that each provider that wants to
record such a histogram doesn't need to worry about making sure to do this thing
itself. I agree that OnCollectFinalMetrics would be misleading in this case.

Thinking about this more, let's not introduce the new OnCollectFinalMetrics()
method at all and instead use the existing ProvideGeneralMetrics() method.
Current users of that method only provide non-histogram metrics (i.e. set the
protobuf fields themselves), but it looks like we can easily make that support
logging histograms from it.

To do that, the two calls to RecordGeneralMetrics() in metrics_service.cc have
to be moved just above  RecordCurrentHistograms(). I checked all the current
implementors of ProvideGeneralMetrics() and it should be safe to do (none of
their functionality depends on the ordering w.r.t. to histogram recording).

When you do this, please also expand the comment on ProvideGeneralMetrics() in
metrics_provider.h to mention that this method can also be used to record
histograms that will recorded with every metrics log.

+
     client_->CollectFinalMetrics(
         base::Bind(&MetricsService::OnFinalLogInfoCollectionDone,
                    self_ptr_factory_.GetWeakPtr()));