Pile of nits for tracked object enablement

[Re-land of Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107921
with supressions for induced (and planned) leaks) ]

Be extra carful about handling races in access to status_.
This will avoid generating a delta between a null time
and a real time, when status is changing in/around
the run of a task. This won't help with the benign
race for checking status_, but it may help with unit
test tsan complaints.

Leak data aggressively, rather than cleaning up, to
prevent any chance of a data access race between
tracked object testing (which need a near-virgin
global state, and hence must start by cleaning it up), and
other tests, which may have lingering threaded actions, 
that still access some previously created task tracking
data.

Provide more options for flags to enable/disable
tracking. These options might become useful
if we changed the default to not do tracking.

Allow for HTML generation even if the tracking has
changed to being disabled. This is especially useful
for looking at the tracked instances that were
monitored after turning tracking on by default, but
before the command line deactiated tracking.

tbr=rtenneti
bug=102327
Review URL: http://codereview.chromium.org/8414053

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107928 0039d316-1c4b-4281-b951-d872f2087c98

base/tracked_objects.cc

@@ -220,7 +220,7 @@ void ThreadData::OnThreadTerminationCleanup() const {
 
 // static
 void ThreadData::WriteHTML(const std::string& query, std::string* output) {
-  if (!ThreadData::tracking_status())
+  if (status_ == UNINITIALIZED)
     return;  // Not yet initialized.
 
   DataCollector collected_data;  // Gather data.
@@ -406,8 +406,18 @@ void ThreadData::TallyRunOnNamedThreadIfTracking(
       ? tracked_objects::TrackedTime(completed_task.time_posted)
       : tracked_objects::TrackedTime(completed_task.delayed_run_time);
 
-  Duration queue_duration = start_of_run - effective_post_time;
-  Duration run_duration = end_of_run - start_of_run;
+  // Watch out for a race where status_ is changing, and hence one or both
+  // of start_of_run or end_of_run is zero.  IN that case, we didn't bother to
+  // get a time value since we "weren't tracking" and we were trying to be
+  // efficient by not calling for a genuine time value. For simplicity, we'll
+  // use a default zero duration when we can't calculate a true value.
+  Duration queue_duration;
+  Duration run_duration;
+  if (!start_of_run.is_null()) {
+    queue_duration = start_of_run - effective_post_time;
+    if (!end_of_run.is_null())
+      run_duration = end_of_run - start_of_run;
+  }
   current_thread_data->TallyADeath(*birth, queue_duration, run_duration);
 }
 
@@ -439,8 +449,13 @@ void ThreadData::TallyRunOnWorkerThreadIfTracking(
   if (!current_thread_data)
     return;
 
-  Duration queue_duration = start_of_run - time_posted;
-  Duration run_duration = end_of_run - start_of_run;
+  Duration queue_duration;
+  Duration run_duration;
+  if (!start_of_run.is_null()) {
+    queue_duration = start_of_run - time_posted;
+    if (!end_of_run.is_null())
+      run_duration = end_of_run - start_of_run;
+  }
   current_thread_data->TallyADeath(*birth, queue_duration, run_duration);
 }
 
@@ -524,9 +539,9 @@ bool ThreadData::tracking_status() {
 
 // static
 TrackedTime ThreadData::Now() {
-  if (!kTrackAllTaskObjects || status_ != ACTIVE)
-    return TrackedTime();  // Super fast when disabled, or not compiled.
-  return TrackedTime::Now();
+  if (kTrackAllTaskObjects && tracking_status())
+    return TrackedTime::Now();
+  return TrackedTime();  // Super fast when disabled, or not compiled.
 }
 
 // static
@@ -546,6 +561,19 @@ void ThreadData::ShutdownSingleThreadedCleanup() {
     unregistered_thread_data_pool_ = NULL;
   }
 
+  // Put most global static back in pristine shape.
+  thread_number_counter_ = 0;
+  tls_index_.Set(NULL);
+  status_ = UNINITIALIZED;
+
+  // To avoid any chance of racing in unit tests, which is the only place we
+  // call this function, we will leak all the data structures we recovered.
+  // These structures could plausibly be used by other threads in earlier tests
+  // that are still running.
+  return;
+
+  // If we wanted to cleanup (on a single thread), here is what we would do.
+
   if (final_pool) {
     // The thread_data_list contains *all* the instances, and we'll use it to
     // delete them.  This pool has pointers to some instances, and we just
@@ -567,10 +595,6 @@ void ThreadData::ShutdownSingleThreadedCleanup() {
     next_thread_data->death_map_.clear();
     delete next_thread_data;  // Includes all Death Records.
   }
-  // Put most global static back in pristine shape.
-  thread_number_counter_ = 0;
-  tls_index_.Set(NULL);
-  status_ = UNINITIALIZED;
 }
 
 //------------------------------------------------------------------------------

chrome/browser/chrome_browser_main.cc

@@ -1218,10 +1218,11 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   }
 
   if (parsed_command_line().HasSwitch(switches::kEnableTracking)) {
+    // User wants to override default tracking status.
     std::string flag =
       parsed_command_line().GetSwitchValueASCII(switches::kEnableTracking);
-    if (flag.compare("0") == 0)
-       tracked_objects::ThreadData::InitializeAndSetTrackingStatus(false);
+    bool enabled = flag.compare("0") != 0;
+    tracked_objects::ThreadData::InitializeAndSetTrackingStatus(enabled);
   }
 
   // This forces the TabCloseableStateWatcher to be created and, on chromeos,

tools/heapcheck/suppressions.txt

@@ -193,6 +193,25 @@
    fun:base::LeakyLazyInstanceTraits::New
    fun:base::LazyInstance::Pointer
 }
+{
+   Intentional leak in object tracking statics to avoid shutdown race
+   Heapcheck:Leak
+   ...
+   fun:tracked_objects::ThreadData::Initialize
+}
+{
+   Intentional leak in object tracking of thread context to avoid shutdown race
+   Heapcheck:Leak
+   fun:tracked_objects::ThreadData::Get
+}
+{
+   Intentional leak of task birth and death data to avoid shutdown race
+   Heapcheck:Leak
+   ...
+   fun:tracked_objects::ThreadData::TallyA*
+}
+
+
 #-----------------------------------------------------------------------
 # 3. Suppressions for real chromium bugs that are not yet fixed.
 # These should all be in chromium's bug tracking system (but a few aren't yet).