From b77226e27cab2828e1638a978864442dd336a73e Mon Sep 17 00:00:00 2001
From: "husky@chromium.org"
 <husky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 24 Feb 2012 04:17:06 +0000
Subject: Use SequencedWorkerPool for disk operations in TraceSubscriberStdio.

This class was hitting a ThreadRestrictions assert because it called
OpenFile on the UI thread. To reduce unnecessary copying, I've changed
the OnTraceDataCollected argument from std::string to RefCountedString.

Second attempt (originally committed in 123140, reverted in 123146).
Added DISALLOW_COPY_AND_ASSIGN to fix Windows compilation error.

BUG=None
TEST=content_unittests


Review URL: http://codereview.chromium.org/9443020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@123433 0039d316-1c4b-4281-b951-d872f2087c98
---
 content/browser/trace_subscriber_stdio.cc | 116 +++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 41 deletions(-)

(limited to 'content/browser/trace_subscriber_stdio.cc')

diff --git a/content/browser/trace_subscriber_stdio.cc b/content/browser/trace_subscriber_stdio.cc
index 0ecd51e..028404c 100644
--- a/content/browser/trace_subscriber_stdio.cc
+++ b/content/browser/trace_subscriber_stdio.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -6,60 +6,94 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "content/public/browser/browser_thread.h"
 
-TraceSubscriberStdio::TraceSubscriberStdio() : file_(0) {
-}
+namespace content {
 
-TraceSubscriberStdio::TraceSubscriberStdio(const FilePath& path) : file_(0) {
-  OpenFile(path);
-}
+// All method calls on this class are done on a SequencedWorkerPool thread.
+class TraceSubscriberStdioImpl
+    : public base::RefCountedThreadSafe<TraceSubscriberStdioImpl> {
+ public:
+  explicit TraceSubscriberStdioImpl(const FilePath& path)
+      : path_(path),
+        file_(0) {}
 
-TraceSubscriberStdio::~TraceSubscriberStdio() {
-  CloseFile();
-}
+  ~TraceSubscriberStdioImpl() {
+    CloseFile();
+  }
+
+  void OnStart() {
+    DCHECK(!file_);
+    file_ = file_util::OpenFile(path_, "w+");
+    if (IsValid()) {
+      LOG(INFO) << "Logging performance trace to file: " << path_.value();
+      trace_buffer_.SetOutputCallback(
+          base::Bind(&TraceSubscriberStdioImpl::Write, this));
+      trace_buffer_.Start();
+    } else {
+      LOG(ERROR) << "Failed to open performance trace file: " << path_.value();
+    }
+  }
 
-bool TraceSubscriberStdio::OpenFile(const FilePath& path) {
-  LOG(INFO) << "Logging performance trace to file: " << path.value();
-  CloseFile();
-  file_ = file_util::OpenFile(path, "w+");
-  if (IsValid()) {
-    trace_buffer_.SetOutputCallback(base::Bind(&TraceSubscriberStdio::Write,
-                                               base::Unretained(this)));
-    trace_buffer_.Start();
-    return true;
-  } else {
-    LOG(ERROR) << "Failed to open performance trace file: " << path.value();
-    return false;
+  void OnData(const scoped_refptr<base::RefCountedString>& data_ptr) {
+    trace_buffer_.AddFragment(data_ptr->data());
+  }
+
+  void OnEnd() {
+    trace_buffer_.Finish();
+    CloseFile();
+  }
+
+ private:
+  bool IsValid() {
+    return file_ && (0 == ferror(file_));
+  }
+
+  void CloseFile() {
+    if (file_) {
+      fclose(file_);
+      file_ = 0;
+    }
   }
-}
 
-void TraceSubscriberStdio::CloseFile() {
-  if (file_) {
-    fclose(file_);
-    file_ = 0;
+  void Write(const std::string& output_str) {
+    if (IsValid()) {
+      size_t written = fwrite(output_str.data(), 1, output_str.size(), file_);
+      if (written != output_str.size()) {
+        LOG(ERROR) << "Error " << ferror(file_) << " in fwrite() to trace file";
+        CloseFile();
+      }
+    }
   }
+
+  FilePath path_;
+  FILE* file_;
+  base::debug::TraceResultBuffer trace_buffer_;
+};
+
+TraceSubscriberStdio::TraceSubscriberStdio(const FilePath& path)
+    : impl_(new TraceSubscriberStdioImpl(path)) {
+  BrowserThread::PostBlockingPoolSequencedTask(
+      __FILE__, FROM_HERE,
+      base::Bind(&TraceSubscriberStdioImpl::OnStart, impl_.get()));
 }
 
-bool TraceSubscriberStdio::IsValid() {
-  return file_ && (0 == ferror(file_));
+TraceSubscriberStdio::~TraceSubscriberStdio() {
 }
 
 void TraceSubscriberStdio::OnEndTracingComplete() {
-  trace_buffer_.Finish();
-  CloseFile();
+  BrowserThread::PostBlockingPoolSequencedTask(
+      __FILE__, FROM_HERE,
+      base::Bind(&TraceSubscriberStdioImpl::OnEnd, impl_.get()));
 }
 
 void TraceSubscriberStdio::OnTraceDataCollected(
-    const std::string& trace_fragment) {
-  trace_buffer_.AddFragment(trace_fragment);
+    const scoped_refptr<base::RefCountedString>& data_ptr) {
+  BrowserThread::PostBlockingPoolSequencedTask(
+      __FILE__, FROM_HERE,
+      base::Bind(&TraceSubscriberStdioImpl::OnData, impl_.get(), data_ptr));
 }
 
-void TraceSubscriberStdio::Write(const std::string& output_str) {
-  if (IsValid()) {
-    size_t written = fwrite(output_str.data(), 1, output_str.size(), file_);
-    if (written != output_str.size()) {
-      LOG(ERROR) << "Error " << ferror(file_) << " when writing to trace file";
-      CloseFile();
-    }
-  }
-}
+}  // namespace content
+
-- 
cgit v1.1