Allow TracingController to output trace data to specified file

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 d2e8a31854617e550deb33c5c749404cc6a951a8..babb829f2c19685e9c6d01e42fc6d7887d5932d1 100644
--- a/content/browser/tracing/tracing_controller_impl.cc
+++ b/content/browser/tracing/tracing_controller_impl.cc
@@ -8,6 +8,7 @@
 #include "base/file_util.h"
 #include "base/json/string_escape.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/threading/thread_restrictions.h"
 #include "content/browser/tracing/trace_message_filter.h"
 #include "content/common/child_process_messages.h"
 #include "content/public/browser/browser_message_filter.h"
@@ -28,15 +29,94 @@ TracingController* TracingController::GetInstance() {
   return TracingControllerImpl::GetInstance();
 }
 
+class TracingControllerImpl::ResultFile {
+ public:
+  explicit ResultFile(const base::FilePath& path);
+  void Write(const scoped_refptr<base::RefCountedString>& events_str_ptr) {
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+        base::Bind(&TracingControllerImpl::ResultFile::WriteTask,
+                   base::Unretained(this), events_str_ptr));
+  }
+  void Close(const base::Closure& callback) {
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+        base::Bind(&TracingControllerImpl::ResultFile::CloseTask,
+                   base::Unretained(this), callback));
+  }
+  const base::FilePath& path() const { return path_; }
+
+ private:
+  void OpenTask();
+  void WriteTask(const scoped_refptr<base::RefCountedString>& events_str_ptr);
+  void CloseTask(const base::Closure& callback);
+
+  FILE* file_;
+  base::FilePath path_;
+  bool has_at_least_one_result_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResultFile);
+};
+
+TracingControllerImpl::ResultFile::ResultFile(const base::FilePath& path)
+    : file_(NULL),
+      path_(path),
+      has_at_least_one_result_(false) {
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&TracingControllerImpl::ResultFile::OpenTask,
+                 base::Unretained(this)));
+}
+
+void TracingControllerImpl::ResultFile::OpenTask() {
+  if (path_.empty())
+    file_util::CreateTemporaryFile(&path_);
+  file_ = file_util::OpenFile(path_, "w");
+  if (!file_) {
+    LOG(ERROR) << "Failed to open " << path_.value();
+    return;
+  }
+  const char* preamble = "{\"traceEvents\": [";
+  size_t written = fwrite(preamble, strlen(preamble), 1, file_);
+  DCHECK(written == 1);
+}
+
+void TracingControllerImpl::ResultFile::WriteTask(
+    const scoped_refptr<base::RefCountedString>& events_str_ptr) {
+  if (!file_)
+    return;
+
+  // If there is already a result in the file, then put a commma
+  // before the next batch of results.
+  if (has_at_least_one_result_)
+    fwrite(",", 1, 1, file_);
+  has_at_least_one_result_ = true;
+  size_t written = fwrite(events_str_ptr->data().c_str(),
+                          events_str_ptr->data().size(), 1,
+                          file_);
+  DCHECK(written == 1);
+}
+
+void TracingControllerImpl::ResultFile::CloseTask(
+    const base::Closure& callback) {
+  if (!file_)
+    return;
+
+  const char* trailout = "]}";
+  size_t written = fwrite(trailout, strlen(trailout), 1, file_);
+  DCHECK(written == 1);
+  file_util::CloseFile(file_);
+  file_ = NULL;
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
+
 TracingControllerImpl::TracingControllerImpl() :
     pending_disable_recording_ack_count_(0),
     pending_capture_monitoring_snapshot_ack_count_(0),
     is_recording_(false),
     is_monitoring_(false),
+    trace_options_(TraceLog::RECORD_UNTIL_FULL),
     category_filter_(
-        base::debug::CategoryFilter::kDefaultCategoryFilterString),
-    result_file_(0),
-    result_file_has_at_least_one_result_(false) {
+        base::debug::CategoryFilter::kDefaultCategoryFilterString) {
 }
 
 TracingControllerImpl::~TracingControllerImpl() {
@@ -59,7 +139,7 @@ void TracingControllerImpl::GetCategories(
   EnableRecording(base::debug::CategoryFilter("*"),
                   TracingController::Options(),
                   EnableRecordingDoneCallback());
-  DisableRecording(TracingFileResultCallback());
+  DisableRecording(base::FilePath(), TracingFileResultCallback());
 }
 
 bool TracingControllerImpl::EnableRecording(
@@ -94,6 +174,7 @@ bool TracingControllerImpl::EnableRecording(
 }
 
 bool TracingControllerImpl::DisableRecording(
+    const base::FilePath& result_file_path,
     const TracingFileResultCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -111,17 +192,8 @@ bool TracingControllerImpl::DisableRecording(
     TraceLog::GetInstance()->AddClockSyncMetadataEvent();
 #endif
 
-  // We don't need to create a temporary file when getting categories.
-  if (pending_get_categories_done_callback_.is_null()) {
-    base::FilePath temporary_file;
-    file_util::CreateTemporaryFile(&temporary_file);
-    result_file_path_.reset(new base::FilePath(temporary_file));
-    result_file_ = file_util::OpenFile(*result_file_path_, "w");
-    result_file_has_at_least_one_result_ = false;
-    const char* preamble = "{\"traceEvents\": [";
-    size_t written = fwrite(preamble, strlen(preamble), 1, result_file_);
-    DCHECK(written == 1);
-  }
+  if (!callback.is_null() || !result_file_path.empty())
+    result_file_.reset(new ResultFile(result_file_path));

[Jói, 2013/11/19 12:42:22]

Do you not need to close the previous result_file_ before setting it?

[Xianzhu, 2013/11/19 16:48:34]

The !can_disable_recording() condition above ensures that an open ResultFile
won't be replaced here. result_file_ should be either initially NULL or has been
reset in the last OnResultFileClosed.

Modified can_disable_recording() to make the logic more explicit.

 
   // There could be a case where there are no child processes and filters_
   // is empty. In that case we can immediately tell the subscriber that tracing
@@ -203,6 +275,7 @@ void TracingControllerImpl::GetMonitoringStatus(
 }
 
 void TracingControllerImpl::CaptureMonitoringSnapshot(
+    const base::FilePath& result_file_path,
     const TracingFileResultCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -211,14 +284,8 @@ void TracingControllerImpl::CaptureMonitoringSnapshot(
 
   pending_capture_monitoring_snapshot_done_callback_ = callback;
 
-  base::FilePath temporary_file;
-  file_util::CreateTemporaryFile(&temporary_file);
-  result_file_path_.reset(new base::FilePath(temporary_file));
-  result_file_ = file_util::OpenFile(*result_file_path_, "w");
-  result_file_has_at_least_one_result_ = false;
-  const char* preamble = "{\"traceEvents\": [";
-  size_t written = fwrite(preamble, strlen(preamble), 1, result_file_);
-  DCHECK(written == 1);
+  if (!callback.is_null() || !result_file_path.empty())
+    monitoring_snapshot_file_.reset(new ResultFile(result_file_path));
 
   // There could be a case where there are no child processes and filters_
   // is empty. In that case we can immediately tell the subscriber that tracing
@@ -308,15 +375,24 @@ void TracingControllerImpl::OnDisableRecordingAcked(
   if (!pending_get_categories_done_callback_.is_null()) {
     pending_get_categories_done_callback_.Run(known_category_groups_);
     pending_get_categories_done_callback_.Reset();
-  } else if (!pending_disable_recording_done_callback_.is_null()) {
-    const char* trailout = "]}";
-    size_t written = fwrite(trailout, strlen(trailout), 1, result_file_);
-    DCHECK(written == 1);
-    file_util::CloseFile(result_file_);
-    result_file_ = 0;
-    pending_disable_recording_done_callback_.Run(result_file_path_.Pass());
+  } else if (result_file_) {
+    result_file_->Close(
+        base::Bind(&TracingControllerImpl::OnResultFileClosed,
+                   base::Unretained(this)));
+  }
+}
+
+void TracingControllerImpl::OnResultFileClosed() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (!result_file_)
+    return;
+
+  if (!pending_disable_recording_done_callback_.is_null()) {
+    pending_disable_recording_done_callback_.Run(result_file_->path());
     pending_disable_recording_done_callback_.Reset();
   }
+  result_file_.reset();
 }
 
 void TracingControllerImpl::OnCaptureMonitoringSnapshotAcked() {
@@ -342,16 +418,25 @@ void TracingControllerImpl::OnCaptureMonitoringSnapshotAcked() {
   if (pending_capture_monitoring_snapshot_ack_count_ != 0)
     return;
 
+  if (monitoring_snapshot_file_) {
+    monitoring_snapshot_file_->Close(
+        base::Bind(&TracingControllerImpl::OnMonitoringSnapshotFileClosed,
+                   base::Unretained(this)));
+  }
+}
+
+void TracingControllerImpl::OnMonitoringSnapshotFileClosed() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (!monitoring_snapshot_file_)
+    return;
+
   if (!pending_capture_monitoring_snapshot_done_callback_.is_null()) {
-    const char* trailout = "]}";
-    size_t written = fwrite(trailout, strlen(trailout), 1, result_file_);
-    DCHECK(written == 1);
-    file_util::CloseFile(result_file_);
-    result_file_ = 0;
     pending_capture_monitoring_snapshot_done_callback_.Run(
-        result_file_path_.Pass());
+        monitoring_snapshot_file_->path());
     pending_capture_monitoring_snapshot_done_callback_.Reset();
   }
+  monitoring_snapshot_file_.reset();
 }
 
 void TracingControllerImpl::OnTraceDataCollected(
@@ -365,22 +450,21 @@ void TracingControllerImpl::OnTraceDataCollected(
     return;
   }
 
-  // Drop trace events if we are just getting categories.
-  if (!pending_get_categories_done_callback_.is_null())
-    return;
+  if (result_file_)
+    result_file_->Write(events_str_ptr);
+}
 
-  // If there is already a result in the file, then put a commma
-  // before the next batch of results.
-  if (result_file_has_at_least_one_result_) {
-    size_t written = fwrite(",", 1, 1, result_file_);
-    DCHECK(written == 1);
-  } else {
-    result_file_has_at_least_one_result_ = true;
+void TracingControllerImpl::OnMonitoringTraceDataCollected(
+    const scoped_refptr<base::RefCountedString>& events_str_ptr) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(&TracingControllerImpl::OnMonitoringTraceDataCollected,
+                   base::Unretained(this), events_str_ptr));
+    return;
   }
-  size_t written = fwrite(events_str_ptr->data().c_str(),
-                          events_str_ptr->data().size(), 1,
-                          result_file_);
-  DCHECK(written == 1);
+
+  if (!monitoring_snapshot_file_)
+    monitoring_snapshot_file_->Write(events_str_ptr);
 }
 
 void TracingControllerImpl::OnLocalTraceDataCollected(
@@ -402,7 +486,7 @@ void TracingControllerImpl::OnLocalMonitoringTraceDataCollected(
     const scoped_refptr<base::RefCountedString>& events_str_ptr,
     bool has_more_events) {
   if (events_str_ptr->data().size())
-    OnTraceDataCollected(events_str_ptr);
+    OnMonitoringTraceDataCollected(events_str_ptr);
 
   if (has_more_events)
     return;