Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

components/metrics/profiler/tracking_synchronizer_observer.h

Index: components/metrics/profiler/tracking_synchronizer_observer.h
diff --git a/components/metrics/profiler/tracking_synchronizer_observer.h b/components/metrics/profiler/tracking_synchronizer_observer.h
index 0e943508079bd814169fae18b759ff563f74c21f..65557db3916a44d7554381378a29517a4426cd3c 100644
--- a/components/metrics/profiler/tracking_synchronizer_observer.h
+++ b/components/metrics/profiler/tracking_synchronizer_observer.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/process/process_handle.h"
+#include "base/time/time.h"
 #include "components/metrics/proto/chrome_user_metrics_extension.pb.h"
 #include "content/public/common/process_type.h"
 
@@ -29,28 +30,21 @@ typedef std::vector<ProfilerEventProto::ProfilerEvent> ProfilerEvents;
 // Observer for notifications from the TrackingSynchronizer class.
 class TrackingSynchronizerObserver {
  public:
-  // TODO(vadimt): Consider isherman@ idea: I'd change the phase_start and
-  // phase_finish from TimeDeltas to TimeTicks.  And I'd omit the |past_events|
-  // list -- either in favor of a single ProfilerEvent that corresponds to the
-  // phase, or a method on the TrackingSynchronizer that can translate a
-  // profiling_phase to a ProfilerEvent.
-
   // Received |process_data_phase| for profiling phase |profiling_phase| from a
-  // single process of |process_type|. The phase start and finish times,
-  // relative to the start time are |phase_start| and
-  // |phase_finish|. All profiling phases prior to the reported one have already
-  // completed, and each completion was associated with an instance of
-  // ProfilerEventProto::ProfilerEvent. |past_events| contains events associated
-  // with completions of phases prior to the reported one.
-  // The observer should assume there might be more data coming until
+  // single process of |process_type|. The phase start and finish times are
+  // |phase_start| and |phase_finish|. All profiling phases prior to the
+  // reported one have already completed, and each completion was associated
+  // with an instance of ProfilerEventProto::ProfilerEvent. |past_events|
+  // contains events associated with completions of phases prior to the reported
+  // one. The observer should assume there might be more data coming until
   // FinishedReceivingData() is called.

[Ilya Sherman, 2015/04/08 23:36:15]

I'm still not thrilled with passing the set of |past_events|, because I think
it's simpler to write the chrome://profiler UI page without using this data
structure, and I think the data structure is relatively non-obvious in terms of
how it's used.  But, I think it's not really useful to spend more time
bikeshedding over it, so let's go with your preference and keep it here.  If we
do so, let's make the documentation very clear about why such a structure is
important and useful.  Here's my attempt at refining the comment; feel free to
refine further:

"Each completed phase is associated with an event that triggered the completion
of the phase.  |past_events| contains the set of events that completed prior to
the reported phase. This data structure is useful for quickly computing the full
set of profiled traces that occurred before or after a given event."

[vadimt, 2015/04/09 00:15:03]

Done. Thanks for the suggested comment!

   virtual void ReceivedProfilerData(
       const tracked_objects::ProcessDataPhaseSnapshot& process_data_phase,
       base::ProcessId process_id,
       content::ProcessType process_type,
       int profiling_phase,
-      base::TimeDelta phase_start,
-      base::TimeDelta phase_finish,
+      base::TimeTicks phase_start,
+      base::TimeTicks phase_finish,
       const ProfilerEvents& past_events) = 0;
 
   // The observer should not expect any more calls to |ReceivedProfilerData()|