Introducing phased profiling framework

chrome/browser/metrics/chrome_metrics_service_client.cc

 // Copyright 2014 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.
 
 #include "chrome/browser/metrics/chrome_metrics_service_client.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 360 matching lines @@
 }
 
 void ChromeMetricsServiceClient::OnInitTaskGotGoogleUpdateData() {
   // Start the next part of the init task: fetching performance data.  This will
   // call into |FinishedReceivingProfilerData()| when the task completes.
   metrics::TrackingSynchronizer::FetchProfilerDataAsynchronously(
       weak_ptr_factory_.GetWeakPtr());
 }
 
 void ChromeMetricsServiceClient::ReceivedProfilerData(
-    const tracked_objects::ProcessDataSnapshot& process_data,
-    int process_type) {
-  profiler_metrics_provider_->RecordProfilerData(process_data, process_type);
+    const tracked_objects::ProcessDataPhaseSnapshot& process_data_phase,
+    base::ProcessId process_id,
+    content::ProcessType process_type,
+    int profiling_phase,
+    const base::TimeDelta& phase_start,
+    const base::TimeDelta& phase_finish,
+    const metrics::ProfilerEvents& past_events) {

[Alexei Svitkine (slow), 2015/03/16 22:00:15]

I still think we should group all of these fields on a struct or class, so that
code like this can just pass that object through, rather than re-writing these
params here and below.

[vadimt, 2015/03/16 23:09:13]

I suggest moving this to the next CL.
isherman@ questions the very set of these params, I object that their set will
become more clear in the next CL.
It will make more sense to consider grouping once we know which params this
function has.

[Alexei Svitkine (slow), 2015/03/17 14:53:41]

Can you elaborate what you're suggesting? i.e. are you suggesting to split this
CL so that this change will not be part of it or are you suggesting to land
these params as-is and discuss them in the next CL?

Obviously, the former is preferable, but I understand if its infeasible. If
you're suggesting the latter, please add a TODO and reference a crbug.

[vadimt, 2015/03/17 15:12:53]

The latter.
Added TODO.

+  profiler_metrics_provider_->RecordProfilerData(
+      process_data_phase, process_id, process_type, profiling_phase,
+      phase_start, phase_finish, past_events);
 }
 
 void ChromeMetricsServiceClient::FinishedReceivingProfilerData() {
   finished_gathering_initial_metrics_callback_.Run();
 }
 
 void ChromeMetricsServiceClient::OnMemoryDetailCollectionDone() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // This function should only be called as the callback from an ansynchronous
@@ skipping 159 matching lines @@
   // Capture the histogram samples.
   if (dumps_with_crash != 0)
     UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithCrash", dumps_with_crash);
   if (dumps_with_no_crash != 0)
     UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithNoCrash", dumps_with_no_crash);
   int total_dumps = dumps_with_crash + dumps_with_no_crash;
   if (total_dumps != 0)
     UMA_HISTOGRAM_COUNTS("Chrome.BrowserCrashDumpAttempts", total_dumps);
 }
 #endif  // defined(OS_WIN)