Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

base/tracked_objects.h

Index: base/tracked_objects.h
diff --git a/base/tracked_objects.h b/base/tracked_objects.h
index 964326511b23355f0bca71b1671919d8cb3e8548..5167fc5e32e562b29bffdcf185829d1a85425882 100644
--- a/base/tracked_objects.h
+++ b/base/tracked_objects.h
@@ -246,6 +246,9 @@ class BASE_EXPORT Births: public BirthOnThread {
   // When we have a birth we update the count for this birthplace.
   void RecordBirth();
 
+  // Subtracts a value from the birth count.
+  void SubtractBirths(int count);
+
  private:
   // The number of births on this thread for our location_.
   int birth_count_;
@@ -283,7 +286,7 @@ class BASE_EXPORT DeathData {
   int32 queue_duration_max() const;
   int32 queue_duration_sample() const;
 
-  // Reset all tallies to zero. This is used as a hack on realtime data.
+  // Reset all tallies to zero.
   void Clear();
 
  private:
@@ -356,6 +359,11 @@ class BASE_EXPORT TaskStopwatch;
 // phase.
 typedef std::map<int, ProcessDataPhaseSnapshot> PhasedProcessDataSnapshotMap;
 
+// Results of resetting death counts to 0. This is a map from the Births pointer
+// to the count of deaths (for tasks with this Births) that were set to 0 by
+// resetting.
+typedef std::map<const Births*, int> DeathResetResults;
+
 class BASE_EXPORT ThreadData {
  public:
   // Current allowable states of the tracking system.  The states can vary
@@ -388,8 +396,21 @@ class BASE_EXPORT ThreadData {
   static ThreadData* Get();
 
   // Fills |process_data_snapshot| with phased snapshots of all profiling
-  // phases, including the current one.
-  static void Snapshot(ProcessDataSnapshot* process_data_snapshot);
+  // phases, including the current one, identified by |current_profiling_phase|.
+  // |current_profiling_phase| is necessary because a child process can start
+  // after several phase-changing events, so it needs to receive the current
+  // phase number from the browser process to fill the correct entry for the
+  // current phase in the |process_data_snapshot| map.
+  static void Snapshot(int current_profiling_phase,
+                       ProcessDataSnapshot* process_data_snapshot);
+
+  // Called when the current profiling phase, identified by |profiling_phase|,
+  // ends.
+  // |profiling_phase| is necessary because a child process can start after
+  // several phase-changing events, so it needs to receive the phase number from
+  // the browser process to fill the correct entry in the
+  // completed_phases_snapshots_ map.
+  static void OnProfilingPhaseCompletion(int profiling_phase);
 
   // Finds (or creates) a place to count births from the given location in this
   // thread, and increment that tally.
@@ -415,13 +436,13 @@ class BASE_EXPORT ThreadData {
   // the task.
   // The |end_of_run| was just obtained by a call to Now() (just after the task
   // finished).
-  static void TallyRunOnWorkerThreadIfTracking(const Births* birth,
+  static void TallyRunOnWorkerThreadIfTracking(const Births* births,
                                                const TrackedTime& time_posted,
                                                const TaskStopwatch& stopwatch);
 
   // Record the end of execution in region, generally corresponding to a scope
   // being exited.
-  static void TallyRunInAScopedRegionIfTracking(const Births* birth,
+  static void TallyRunInAScopedRegionIfTracking(const Births* births,
                                                 const TaskStopwatch& stopwatch);
 
   const std::string& thread_name() const { return thread_name_; }
@@ -517,19 +538,23 @@ class BASE_EXPORT ThreadData {
   Births* TallyABirth(const Location& location);
 
   // Find a place to record a death on this thread.
-  void TallyADeath(const Births& birth,
+  void TallyADeath(const Births& births,
                    int32 queue_duration,
                    const TaskStopwatch& stopwatch);
 
   // Snapshot (under a lock) the profiled data for the tasks in each ThreadData
   // instance.  Also updates the |birth_counts| tally for each task to keep
-  // track of the number of living instances of the task.
+  // track of the number of living instances of the task. If |reset| is true,
+  // remove all recorded deaths.
   static void SnapshotAllExecutedTasks(
+      bool reset,

[jar (doing other things), 2015/04/01 17:10:25]

This is totally unsafe, and will instigate errors as large as the tally of
births, due to races.

[vadimt, 2015/04/06 23:25:12]

Done.

       ProcessDataPhaseSnapshot* process_data_phase,
       BirthCountMap* birth_counts);
 
   // Fills |process_data_phase| with all the recursive results in our process.
+  // If |reset| is true, remove all recorded deaths.
   static void SnapshotCurrentPhase(
+      bool reset,
       ProcessDataPhaseSnapshot* process_data_phase);
 
   // Snapshots (under a lock) the profiled data for the tasks for this thread
@@ -537,17 +562,32 @@ class BASE_EXPORT ThreadData {
   // with with entries in the death_map_ -- into |process_data_phase|.  Also
   // updates the |birth_counts| tally for each task to keep track of the number
   // of living instances of the task -- that is, each task maps to the number of
-  // births for the task that have not yet been balanced by a death.
+  // births for the task that have not yet been balanced by a death. If
+  // |death_reset_results| is not null, store current death counts in it and

[jar (doing other things), 2015/04/01 17:10:25]

You can't get away with doing atomic operation so casually, as a race (without a
lock) will result in garbage values (delta as large as the size of the current
tally generally... and perhaps it will actually be worse).

[vadimt, 2015/04/06 23:25:12]

Done.

+  // remove all recorded deaths.
   void SnapshotExecutedTasks(ProcessDataPhaseSnapshot* process_data_phase,
+                             DeathResetResults* death_reset_results,
                              BirthCountMap* birth_counts);
 
   // Using our lock, make a copy of the specified maps.  This call may be made
   // on  non-local threads, which necessitate the use of the lock to prevent
-  // the map(s) from being reallocated while they are copied.
+  // the map(s) from being reallocated while they are copied. If
+  // |death_reset_results| is not null, store current death counts in it and
+  // remove all recorded deaths.
   void SnapshotMaps(BirthMap* birth_map,
                     DeathMap* death_map,
+                    DeathResetResults* death_reset_results,
                     ParentChildSet* parent_child_set);
 
+  // Using our lock, decrease births counts owned by this thread after a death
+  // data's reset.
+  // |death_reset_results| is the result of the cross-threads death count reset
+  // operation that took place before calling this method. For each Birth owned
+  // by this ThreadData, the method will subtract from its birth count the total
+  // number of deaths that were 'forgotten' by resetting death counts to 0.
+  void SubtractDeathResultsFromBirths(
+      const DeathResetResults& death_reset_results);
+
   // This method is called by the TLS system when a thread terminates.
   // The argument may be NULL if this thread has never tracked a birth or death.
   static void OnThreadTermination(void* thread_data);
@@ -612,6 +652,10 @@ class BASE_EXPORT ThreadData {
   // We set status_ to SHUTDOWN when we shut down the tracking service.
   static Status status_;
 
+  // Process data snapshots for completed profiling phases.
+  static base::LazyInstance<PhasedProcessDataSnapshotMap>
+      completed_phases_snapshots_;
+
   // Link to next instance (null terminated list). Used to globally track all
   // registered instances (corresponds to all registered threads where we keep
   // data).