Sampling profiling thread should be joined in a FILE thread

content/browser/tracing/tracing_controller_impl.cc

Index: content/browser/tracing/tracing_controller_impl.cc
diff --git a/content/browser/tracing/tracing_controller_impl.cc b/content/browser/tracing/tracing_controller_impl.cc
index ba9f2e01ab1b88fe1104f123b98191e0b02d9087..44d80816da7b3abf7be5217c7ebb772c6cc2ea50 100644
--- a/content/browser/tracing/tracing_controller_impl.cc
+++ b/content/browser/tracing/tracing_controller_impl.cc
@@ -150,6 +150,24 @@ bool TracingControllerImpl::GetCategories(
   return true;
 }
 
+void TracingControllerImpl::SetEnabled(const std::string& category_filter,
+                                       int trace_options,
+                                       const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));

[dsinclair, 2013/12/12 15:14:04]

Should we add logic in here that, if we aren't on the FILE thread it does a
PostTask to the correct thread?  Or, rename the method to be
SetEnabledOnFileThread to make the threading requirement specific?

[haraken, 2013/12/13 03:27:38]

Renamed the method to SetEnabledOnFileThread and SetDisabledOnFileThread.

+
+  TraceLog::GetInstance()->SetEnabled(

[haraken, 2013/12/12 04:22:14]

We need to dispatch SetEnabled in the FILE thread, because the thread that stops
the sampling thread must be the same as the thread that creates the sampling
thread. Otherwise, we will hit the ASSERT in Platform::Stop() in SetDisabled().

+      base::debug::CategoryFilter(category_filter),
+      static_cast<TraceLog::Options>(trace_options));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
+void TracingControllerImpl::SetDisabled(const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  TraceLog::GetInstance()->SetDisabled();
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
 bool TracingControllerImpl::EnableRecording(
     const std::string& category_filter,
     TracingController::Options options,
@@ -158,33 +176,47 @@ bool TracingControllerImpl::EnableRecording(
 
   if (!can_enable_recording())
     return false;
+  is_recording_ = true;
 
 #if defined(OS_ANDROID)
   if (pending_get_categories_done_callback_.is_null())
     TraceLog::GetInstance()->AddClockSyncMetadataEvent();
 #endif
 
-  TraceLog::Options trace_options = (options & RECORD_CONTINUOUSLY) ?
+  int trace_options = (options & RECORD_CONTINUOUSLY) ?
       TraceLog::RECORD_CONTINUOUSLY : TraceLog::RECORD_UNTIL_FULL;
   if (options & ENABLE_SAMPLING) {
-    trace_options = static_cast<TraceLog::Options>(
-        trace_options | TraceLog::ENABLE_SAMPLING);
+    trace_options |=  TraceLog::ENABLE_SAMPLING;
   }
   // TODO(haraken): How to handle ENABLE_SYSTRACE?
 
-  TraceLog::GetInstance()->SetEnabled(
-      base::debug::CategoryFilter(category_filter), trace_options);
-  is_recording_ = true;
+  base::Closure on_enable_recording_done_callback =
+      base::Bind(&TracingControllerImpl::OnEnableRecordingDone,
+                 base::Unretained(this),
+                 category_filter, trace_options, callback);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&TracingControllerImpl::SetEnabled,
+                 base::Unretained(this),
+                 category_filter, trace_options,
+                 on_enable_recording_done_callback));
+  return true;
+}
+
+void TracingControllerImpl::OnEnableRecordingDone(
+    const std::string& category_filter,
+    int trace_options,
+    const EnableRecordingDoneCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Notify all child processes.
   for (TraceMessageFilterMap::iterator it = trace_message_filters_.begin();
       it != trace_message_filters_.end(); ++it) {
-    it->get()->SendBeginTracing(category_filter, trace_options);
+    it->get()->SendBeginTracing(category_filter,
+                                static_cast<TraceLog::Options>(trace_options));
   }
 
   if (!callback.is_null())
     callback.Run();
-  return true;
 }
 
 bool TracingControllerImpl::DisableRecording(
@@ -195,11 +227,25 @@ bool TracingControllerImpl::DisableRecording(
   if (!can_disable_recording())
     return false;
 
-  pending_disable_recording_done_callback_ = callback;
-
   // Disable local trace early to avoid traces during end-tracing process from
   // interfering with the process.
-  TraceLog::GetInstance()->SetDisabled();
+  base::Closure on_disable_recording_done_callback =
+      base::Bind(&TracingControllerImpl::OnDisableRecordingDone,
+                 base::Unretained(this),
+                 result_file_path, callback);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&TracingControllerImpl::SetDisabled,
+                 base::Unretained(this),
+                 on_disable_recording_done_callback));
+  return true;
+}
+
+void TracingControllerImpl::OnDisableRecordingDone(
+    const base::FilePath& result_file_path,
+    const TracingFileResultCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  pending_disable_recording_done_callback_ = callback;
 
 #if defined(OS_ANDROID)
   if (pending_get_categories_done_callback_.is_null())
@@ -230,7 +276,6 @@ bool TracingControllerImpl::DisableRecording(
       it != trace_message_filters_.end(); ++it) {
     it->get()->SendEndTracing();
   }
-  return true;
 }
 
 bool TracingControllerImpl::EnableMonitoring(
@@ -247,24 +292,37 @@ bool TracingControllerImpl::EnableMonitoring(
   TraceLog::GetInstance()->AddClockSyncMetadataEvent();
 #endif
 
-  int monitoring_tracing_options = 0;
+  int trace_options = 0;
   if (options & ENABLE_SAMPLING)
-    monitoring_tracing_options |= base::debug::TraceLog::MONITOR_SAMPLING;
+    trace_options |= TraceLog::MONITOR_SAMPLING;
 
-  TraceLog::GetInstance()->SetEnabled(
-      base::debug::CategoryFilter(category_filter),
-      static_cast<TraceLog::Options>(monitoring_tracing_options));
+  base::Closure on_enable_monitoring_done_callback =
+      base::Bind(&TracingControllerImpl::OnEnableMonitoringDone,
+                 base::Unretained(this),
+                 category_filter, trace_options, callback);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&TracingControllerImpl::SetEnabled,
+                 base::Unretained(this),
+                 category_filter, trace_options,
+                 on_enable_monitoring_done_callback));
+  return true;
+}
+
+void TracingControllerImpl::OnEnableMonitoringDone(
+    const std::string& category_filter,
+    int trace_options,
+    const EnableMonitoringDoneCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Notify all child processes.
   for (TraceMessageFilterMap::iterator it = trace_message_filters_.begin();
       it != trace_message_filters_.end(); ++it) {
     it->get()->SendEnableMonitoring(category_filter,
-        static_cast<TraceLog::Options>(monitoring_tracing_options));
+        static_cast<TraceLog::Options>(trace_options));
   }
 
   if (!callback.is_null())
     callback.Run();
-  return true;
 }
 
 bool TracingControllerImpl::DisableMonitoring(
@@ -273,9 +331,22 @@ bool TracingControllerImpl::DisableMonitoring(
 
   if (!can_disable_monitoring())
     return false;
-  is_monitoring_ = false;
 
-  TraceLog::GetInstance()->SetDisabled();
+  base::Closure on_disable_monitoring_done_callback =
+      base::Bind(&TracingControllerImpl::OnDisableMonitoringDone,
+                 base::Unretained(this), callback);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&TracingControllerImpl::SetDisabled,
+                 base::Unretained(this),
+                 on_disable_monitoring_done_callback));
+  return true;
+}
+
+void TracingControllerImpl::OnDisableMonitoringDone(
+    const DisableMonitoringDoneCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  is_monitoring_ = false;
 
   // Notify all child processes.
   for (TraceMessageFilterMap::iterator it = trace_message_filters_.begin();
@@ -285,7 +356,6 @@ bool TracingControllerImpl::DisableMonitoring(
 
   if (!callback.is_null())
     callback.Run();
-  return true;
 }
 
 void TracingControllerImpl::GetMonitoringStatus(