Tracing: Migrated away from BrowserThread::FILE to TaskScheduler

Bug: 667892
Change-Id: I578c3b3386ee018b8a4ac2a097e56555a34c9f51
Reviewed-on: https://chromium-review.googlesource.com/611188Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501498}

chrome/browser/tracing/crash_service_uploader.cc

@@ -14,6 +14,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/task_scheduler/post_task.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
@@ -104,6 +105,7 @@ void TraceCrashServiceUploader::OnURLFetchUploadProgress(
     int64_t current,
     int64_t total) {
   DCHECK(url_fetcher_.get());
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   LOG(WARNING) << "Upload progress: " << current << " of " << total;
 
@@ -121,27 +123,24 @@ void TraceCrashServiceUploader::DoUpload(
     const UploadProgressCallback& progress_callback,
     const UploadDoneCallback& done_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  content::BrowserThread::PostTask(
-      content::BrowserThread::FILE, FROM_HERE,
-      base::Bind(&TraceCrashServiceUploader::DoUploadOnFileThread,
+
+  progress_callback_ = progress_callback;
+  done_callback_ = done_callback;
+
+  base::PostTaskWithTraits(
+      FROM_HERE, {base::TaskPriority::BACKGROUND},
+      base::Bind(&TraceCrashServiceUploader::DoCompressOnBackgroundThread,
                  base::Unretained(this), file_contents, upload_mode,
-                 upload_url_, base::Passed(std::move(metadata)),
-                 progress_callback, done_callback));
+                 upload_url_, base::Passed(std::move(metadata))));
 }
 
-void TraceCrashServiceUploader::DoUploadOnFileThread(
+void TraceCrashServiceUploader::DoCompressOnBackgroundThread(
     const std::string& file_contents,
     UploadMode upload_mode,
     const std::string& upload_url,
-    std::unique_ptr<const base::DictionaryValue> metadata,
-    const UploadProgressCallback& progress_callback,
-    const UploadDoneCallback& done_callback) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
+    std::unique_ptr<const base::DictionaryValue> metadata) {
   DCHECK(!url_fetcher_.get());
 
-  progress_callback_ = progress_callback;
-  done_callback_ = done_callback;
-
   if (upload_url.empty()) {
     OnUploadError("Upload URL empty or invalid");
     return;

chrome/browser/tracing/crash_service_uploader.h

@@ -49,13 +49,12 @@ class TraceCrashServiceUploader : public content::TraceUploader,
                 const UploadDoneCallback& done_callback) override;
 
  private:
-  void DoUploadOnFileThread(
+  void DoCompressOnBackgroundThread(
       const std::string& file_contents,
       UploadMode upload_mode,
       const std::string& upload_url,
-      std::unique_ptr<const base::DictionaryValue> metadata,
-      const UploadProgressCallback& progress_callback,
-      const UploadDoneCallback& done_callback);
+      std::unique_ptr<const base::DictionaryValue> metadata);
+
   // Sets up a multipart body to be uploaded. The body is produced according
   // to RFC 2046.
   void SetupMultipart(const std::string& product,

content/browser/tracing/tracing_controller_impl.cc

@@ -18,6 +18,7 @@
 #include "base/memory/ref_counted_memory.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/sys_info.h"
+#include "base/task_scheduler/post_task.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ -134,7 +135,9 @@ TracingControllerImpl::TracingControllerImpl()
       maximum_trace_buffer_usage_(0),
       approximate_event_count_(0),
       pending_clock_sync_ack_count_(0),
-      enabled_tracing_modes_(0) {
+      enabled_tracing_modes_(0),
+      background_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
+          {base::TaskPriority::BACKGROUND})) {
   // Deliberately leaked, like this class.
   base::FileTracing::SetProvider(new FileTracingProviderImpl);
 }
@@ -166,19 +169,15 @@ bool TracingControllerImpl::GetCategories(
   return true;
 }
 
-void TracingControllerImpl::SetEnabledOnFileThread(
+void TracingControllerImpl::SetEnabledOnBackgroundThread(
     const TraceConfig& trace_config,
     const base::Closure& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-
   TraceLog::GetInstance()->SetEnabled(trace_config, enabled_tracing_modes_);
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
 }
 
-void TracingControllerImpl::SetDisabledOnFileThread(
+void TracingControllerImpl::SetDisabledOnBackgroundThread(
     const base::Closure& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-
   DCHECK(enabled_tracing_modes_);
   TraceLog::GetInstance()->SetDisabled(enabled_tracing_modes_);
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
@@ -338,9 +337,9 @@ void TracingControllerImpl::StopTracingAfterClockSync() {
   // interfering with the process.
   base::Closure on_stop_tracing_done_callback = base::Bind(
       &TracingControllerImpl::OnStopTracingDone, base::Unretained(this));
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::BindOnce(&TracingControllerImpl::SetDisabledOnFileThread,
+  background_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&TracingControllerImpl::SetDisabledOnBackgroundThread,
                      base::Unretained(this), on_stop_tracing_done_callback));
 }
 
@@ -418,7 +417,7 @@ void TracingControllerImpl::AddTraceMessageFilter(
 
 void TracingControllerImpl::RemoveTraceMessageFilter(
     TraceMessageFilter* trace_message_filter) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // If a filter is removed while a response from that filter is pending then
   // simulate the response. Otherwise the response count will be wrong and the
@@ -457,7 +456,8 @@ void TracingControllerImpl::AddTracingAgent(const std::string& agent_name) {
   if (agent_name == debug_daemon->GetTracingAgentName()) {
     additional_tracing_agents_.push_back(debug_daemon);
     debug_daemon->SetStopAgentTracingTaskRunner(
-        BrowserThread::GetBlockingPool());
+        base::CreateSequencedTaskRunnerWithTraits(
+            {base::MayBlock(), base::TaskPriority::BACKGROUND}));
     return;
   }
 
@@ -678,16 +678,10 @@ void TracingControllerImpl::StartAgentTracing(
 
   base::Closure on_agent_started =
       base::Bind(callback, kChromeTracingAgentName, true);
-  if (!BrowserThread::PostTask(
-          BrowserThread::FILE, FROM_HERE,
-          base::BindOnce(&TracingControllerImpl::SetEnabledOnFileThread,
-                         base::Unretained(this), trace_config,
-                         on_agent_started))) {
-    // BrowserThread::PostTask fails if the threads haven't been created yet,
-    // so it should be safe to just use TraceLog::SetEnabled directly.
-    TraceLog::GetInstance()->SetEnabled(trace_config, enabled_tracing_modes_);
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, on_agent_started);
-  }
+  background_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&TracingControllerImpl::SetEnabledOnBackgroundThread,
+                 base::Unretained(this), trace_config, on_agent_started));
 }
 
 void TracingControllerImpl::StopAgentTracing(

content/browser/tracing/tracing_controller_impl.h

@@ -143,10 +143,10 @@ class TracingControllerImpl
   void OnTraceLogStatusReply(TraceMessageFilter* trace_message_filter,
                              const base::trace_event::TraceLogStatus& status);
 
-  void SetEnabledOnFileThread(
+  void SetEnabledOnBackgroundThread(
       const base::trace_event::TraceConfig& trace_config,
       const base::Closure& callback);
-  void SetDisabledOnFileThread(const base::Closure& callback);
+  void SetDisabledOnBackgroundThread(const base::Closure& callback);
   void OnAllTracingAgentsStarted();
   void StopTracingAfterClockSync();
   void OnStopTracingDone();
@@ -198,6 +198,7 @@ class TracingControllerImpl
   scoped_refptr<TraceDataSink> trace_data_sink_;
   scoped_refptr<TraceDataSink> monitoring_data_sink_;
   std::unique_ptr<base::DictionaryValue> metadata_;
+  const scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(TracingControllerImpl);
 };

content/browser/tracing/tracing_controller_impl_data_sinks.cc

@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/pattern.h"
+#include "base/task_scheduler/post_task.h"
 #include "content/browser/tracing/tracing_controller_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/zlib/zlib.h"
@@ -65,29 +66,30 @@ class FileTraceDataEndpoint : public TraceDataEndpoint {
         file_(NULL) {}
 
   void ReceiveTraceChunk(std::unique_ptr<std::string> chunk) override {
-    BrowserThread::PostTask(
-        BrowserThread::FILE, FROM_HERE,
-        base::BindOnce(&FileTraceDataEndpoint::ReceiveTraceChunkOnFileThread,
+    background_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&FileTraceDataEndpoint::ReceiveTraceChunkOnBlockingThread,
                    this, base::Passed(std::move(chunk))));
   }
 
   void ReceiveTraceFinalContents(
       std::unique_ptr<const base::DictionaryValue>) override {
-    BrowserThread::PostTask(
-        BrowserThread::FILE, FROM_HERE,
-        base::BindOnce(&FileTraceDataEndpoint::CloseOnFileThread, this));
+    background_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&FileTraceDataEndpoint::CloseOnBlockingThread, this));
   }
 
  private:
   ~FileTraceDataEndpoint() override { DCHECK(file_ == NULL); }
 
-  void ReceiveTraceChunkOnFileThread(std::unique_ptr<std::string> chunk) {
-    if (!OpenFileIfNeededOnFileThread())
+  void ReceiveTraceChunkOnBlockingThread(std::unique_ptr<std::string> chunk) {
+    if (!OpenFileIfNeededOnBlockingThread())
       return;
     ignore_result(fwrite(chunk->c_str(), chunk->size(), 1, file_));
   }
 
-  bool OpenFileIfNeededOnFileThread() {
+  bool OpenFileIfNeededOnBlockingThread() {
+    base::ThreadRestrictions::AssertIOAllowed();
     if (file_ != NULL)
       return true;
     file_ = base::OpenFile(file_path_, "w");
@@ -98,11 +100,12 @@ class FileTraceDataEndpoint : public TraceDataEndpoint {
     return true;
   }
 
-  void CloseOnFileThread() {
-    if (OpenFileIfNeededOnFileThread()) {
+  void CloseOnBlockingThread() {
+    if (OpenFileIfNeededOnBlockingThread()) {
       base::CloseFile(file_);
       file_ = NULL;
     }
+
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::BindOnce(&FileTraceDataEndpoint::FinalizeOnUIThread, this));
@@ -113,6 +116,9 @@ class FileTraceDataEndpoint : public TraceDataEndpoint {
   base::FilePath file_path_;
   base::Closure completion_callback_;
   FILE* file_;
+  const scoped_refptr<base::SequencedTaskRunner> background_task_runner_ =
+      base::CreateSequencedTaskRunnerWithTraits(
+          {base::MayBlock(), base::TaskPriority::BACKGROUND});
 
   DISALLOW_COPY_AND_ASSIGN(FileTraceDataEndpoint);
 };