Introducing phased profiling framework

components/metrics/profiler/tracking_synchronizer.cc

Index: components/metrics/profiler/tracking_synchronizer.cc
diff --git a/components/metrics/profiler/tracking_synchronizer.cc b/components/metrics/profiler/tracking_synchronizer.cc
index 8d53bd2a7ec19f42df2336cd968f5dcf49eb89b2..36a868c1d4e71f4992a9ceaaa41544e688360d02 100644
--- a/components/metrics/profiler/tracking_synchronizer.cc
+++ b/components/metrics/profiler/tracking_synchronizer.cc
@@ -9,6 +9,7 @@
 #include "base/threading/thread.h"
 #include "base/tracked_objects.h"
 #include "components/metrics/profiler/tracking_synchronizer_observer.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/profiler_controller.h"
 #include "content/public/common/process_type.h"
@@ -179,10 +180,12 @@ base::LazyInstance
 
 // TrackingSynchronizer methods and members.
 
-TrackingSynchronizer::TrackingSynchronizer()
-    : last_used_sequence_number_(kNeverUsableSequenceNumber) {
+TrackingSynchronizer::TrackingSynchronizer(const base::TimeTicks& current_time)

[Ilya Sherman, 2015/03/14 00:20:34]

nit: s/current_time/now

[Ilya Sherman, 2015/03/14 00:20:34]

Though, rather than passing the time, it might be more convenient to pass a
TickClock.

[vadimt, 2015/03/14 01:39:15]

This would create more complexity in unit tests, and with caring about TickClock
lifetime, then create simplicity...

[vadimt, 2015/03/14 01:39:15]

Done.

+    : last_used_sequence_number_(kNeverUsableSequenceNumber),
+      start_time_(current_time) {
   DCHECK(!g_tracking_synchronizer);
   g_tracking_synchronizer = this;
+  phase_start_times_.push_back(current_time);
   content::ProfilerController::GetInstance()->Register(this);
 }
 
@@ -232,7 +235,7 @@ void TrackingSynchronizer::OnPendingProcesses(int sequence_number,
 void TrackingSynchronizer::OnProfilerDataCollected(
     int sequence_number,
     const tracked_objects::ProcessDataSnapshot& profiler_data,
-    int process_type) {
+    content::ProcessType process_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DecrementPendingProcessesAndSendData(sequence_number, profiler_data,
                                        process_type);
@@ -253,29 +256,67 @@ int TrackingSynchronizer::RegisterAndNotifyAllProcesses(
   // Get profiler data from renderer and browser child processes.
   content::ProfilerController::GetInstance()->GetProfilerData(sequence_number);
 
-  // Send profiler_data from browser process.
-  tracked_objects::ProcessDataSnapshot process_data;
-  tracked_objects::ThreadData::Snapshot(&process_data);
-  DecrementPendingProcessesAndSendData(sequence_number, process_data,
+  // Send process data snapshot from browser process.
+  tracked_objects::ProcessDataSnapshot process_data_snapshot;
+  tracked_objects::ThreadData::Snapshot(&process_data_snapshot);
+  DecrementPendingProcessesAndSendData(sequence_number, process_data_snapshot,
                                        content::PROCESS_TYPE_BROWSER);
 
   return sequence_number;
 }
 
+void TrackingSynchronizer::SendData(
+    const tracked_objects::ProcessDataSnapshot& profiler_data,
+    content::ProcessType process_type,
+    const base::TimeTicks& current_time,
+    TrackingSynchronizerObserver* observer) const {
+  // We are going to loop though past profiling phases and notify the request
+  // about each phase that is contained in profiler_data. past_events
+  // will track the set of past profiling events as we go.
+  ProfilerEvents past_events;
+
+  // Go through all completed phases, and through the current one. The current
+  // one is not in phase_completion_events_sequence_, but note the <=
+  // comparison.
+  for (size_t phase = 0; phase <= phase_completion_events_sequence_.size();
+       ++phase) {
+    tracked_objects::PhasedProcessDataSnapshotMap::const_iterator it =
+        profiler_data.phased_process_data_snapshots.find(phase);
+
+    if (it != profiler_data.phased_process_data_snapshots.end()) {
+      // If the phase is contained in the received snapshot, notify the
+      // request.
+      const base::TimeDelta phase_start =
+          phase_start_times_[phase] - start_time_;
+      const base::TimeDelta phase_finish =
+          (phase + 1 < phase_start_times_.size() ? phase_start_times_[phase + 1]
+                                                 : current_time) -
+          start_time_;
+      observer->ReceivedProfilerData(it->second, profiler_data.process_id,
+                                     process_type, phase, phase_start,
+                                     phase_finish, past_events);
+    }
+
+    if (phase != phase_completion_events_sequence_.size()) {
+      past_events.push_back(phase_completion_events_sequence_[phase]);
+    }
+  }
+}
+
 void TrackingSynchronizer::DecrementPendingProcessesAndSendData(
     int sequence_number,
     const tracked_objects::ProcessDataSnapshot& profiler_data,
-    int process_type) {
+    content::ProcessType process_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   RequestContext* request = RequestContext::GetRequestContext(sequence_number);
   if (!request)
     return;
 
-  if (request->callback_object_.get()) {
-    request->callback_object_
-        ->ReceivedProfilerData(profiler_data, process_type);
-  }
+  TrackingSynchronizerObserver* const observer =

[Ilya Sherman, 2015/03/14 00:20:34]

nit: Is the const really needed here?  The pointer value is pretty clearly never
changed, so it just feels like visual noise -- especially since it might be
misread as synonymous with "const TrackingSynchronizerObserver*".

[vadimt, 2015/03/14 01:39:15]

Done. Unfortunately my style is not a dominating one...
I'd squeezed const in all places where it's only possible...

+      request->callback_object_.get();
+  if (observer)
+    SendData(profiler_data, process_type, base::TimeTicks::Now(), observer);
 
   // Delete request if we have heard back from all child processes.
   request->DecrementProcessesPending();