Remove TraceController

content/browser/devtools/devtools_tracing_handler.cc

Index: content/browser/devtools/devtools_tracing_handler.cc
diff --git a/content/browser/devtools/devtools_tracing_handler.cc b/content/browser/devtools/devtools_tracing_handler.cc
index 0657b8e48d2a9c83cc2d171630eddcefc77913b4..1cecf1a1c4f8c0260e318c16589633074e9b83f1 100644
--- a/content/browser/devtools/devtools_tracing_handler.cc
+++ b/content/browser/devtools/devtools_tracing_handler.cc
@@ -6,25 +6,17 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/file_util.h"
 #include "base/location.h"
-#include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "content/browser/devtools/devtools_http_handler_impl.h"
 #include "content/browser/devtools/devtools_protocol_constants.h"
-#include "content/public/browser/trace_controller.h"
-#include "content/public/browser/trace_subscriber.h"
+#include "content/browser/tracing/tracing_controller_impl.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace content {
 
-namespace {
-
-const char kRecordUntilFull[]   = "record-until-full";
-const char kRecordContinuously[] = "record-continuously";
-const char kEnableSampling[] = "enable-sampling";
-
-}  // namespace
-
 DevToolsTracingHandler::DevToolsTracingHandler()
     : is_running_(false) {
   RegisterCommandHandler(devtools::Tracing::start::kName,
@@ -38,75 +30,76 @@ DevToolsTracingHandler::DevToolsTracingHandler()
 DevToolsTracingHandler::~DevToolsTracingHandler() {
 }
 
-void DevToolsTracingHandler::OnEndTracingComplete() {
+void DevToolsTracingHandler::ReadRecordingResult(const base::FilePath& path) {
+  if (!is_running_)
+    return;
+
   is_running_ = false;
-  SendNotification(devtools::Tracing::tracingComplete::kName, NULL);
-}
 
-void DevToolsTracingHandler::OnTraceDataCollected(
-    const scoped_refptr<base::RefCountedString>& trace_fragment) {
-  if (is_running_) {
-    // Hand-craft protocol notification message so we can substitute JSON
-    // that we already got as string as a bare object, not a quoted string.
-    std::string message = base::StringPrintf(
-        "{ \"method\": \"%s\", \"params\": { \"%s\": [ %s ] } }",
-        devtools::Tracing::dataCollected::kName,
-        devtools::Tracing::dataCollected::kValue,
-        trace_fragment->data().c_str());
-    SendRawMessage(message);
-  }
+  // Hand-craft protocol notification message so we can substitute JSON
+  // that we already got as string as a bare object, not a quoted string.
+  std::string message = base::StringPrintf(
+      "{ \"method\": \"%s\", \"params\": { \"%s\": ",
+      devtools::Tracing::dataCollected::kName,
+      devtools::Tracing::dataCollected::kValue);
+  if (!base::ReadFileToString(path, &message))
+    LOG(ERROR) << "Failed to read file: " << path.value();
+  base::DeleteFile(path, false);
+  message.append("} }");
+  SendRawMessage(message);
 }
 
-// Note, if you add more options here you also need to update:
-// base/debug/trace_event_impl:TraceOptionsFromString
-base::debug::TraceLog::Options DevToolsTracingHandler::TraceOptionsFromString(
-    const std::string& options) {
-  std::vector<std::string> split;
-  std::vector<std::string>::iterator iter;
-  int ret = 0;
-
-  base::SplitString(options, ',', &split);
-  for (iter = split.begin(); iter != split.end(); ++iter) {
-    if (*iter == kRecordUntilFull) {
-      ret |= base::debug::TraceLog::RECORD_UNTIL_FULL;
-    } else if (*iter == kRecordContinuously) {
-      ret |= base::debug::TraceLog::RECORD_CONTINUOUSLY;
-    } else if (*iter == kEnableSampling) {
-      ret |= base::debug::TraceLog::ENABLE_SAMPLING;
-    }
-  }
-  if (!(ret & base::debug::TraceLog::RECORD_UNTIL_FULL) &&
-      !(ret & base::debug::TraceLog::RECORD_CONTINUOUSLY))
-    ret |= base::debug::TraceLog::RECORD_UNTIL_FULL;
+void DevToolsTracingHandler::BeginReadingRecordingResult(
+    const base::FilePath& path) {
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DevToolsTracingHandler::ReadRecordingResult,
+                 base::Unretained(this), path));

[piman, 2013/11/15 23:42:52]

What makes Unretained here safe?

[Xianzhu, 2013/11/16 01:16:14]

Used this because I saw existing Unretained(this) and thought it were safe.
Actually it's not. The existing Unretaineds in the constructor are safe because
the callbacks won't outlive this.

Changed to use weak_ptr.

+}
 
-  return static_cast<base::debug::TraceLog::Options>(ret);
+void DevToolsTracingHandler::OnTraceBufferPercentFullResult(
+    float percent_full) {
+  base::DictionaryValue* params = new base::DictionaryValue();
+  params->SetDouble(devtools::Tracing::traceBufferPercentFull::kValue,
+                    percent_full);
+  SendNotification(devtools::Tracing::traceBufferPercentFull::kName, params);
 }
 
 scoped_refptr<DevToolsProtocol::Response>
 DevToolsTracingHandler::OnStart(
     scoped_refptr<DevToolsProtocol::Command> command) {
-  std::string categories;
-  base::DictionaryValue* params = command->params();
-  if (params)
-    params->GetString(devtools::Tracing::start::kCategories, &categories);
+  std::string category_filter;
+  TracingController::Options options;
+  if (!TracingControllerImpl::ParseTracingParams(command->params(),
+                                                 &category_filter, &options)) {
+    return command->InternalErrorResponse("Malformed params");
+  }
 
-  base::debug::TraceLog::Options options =
-      base::debug::TraceLog::RECORD_UNTIL_FULL;
-  if (params && params->HasKey(devtools::Tracing::start::kTraceOptions)) {
-    std::string options_param;
-    params->GetString(devtools::Tracing::start::kTraceOptions, &options_param);
-    options = TraceOptionsFromString(options_param);
+  if (TracingController::GetInstance()->EnableRecording(
+      category_filter, options,
+      TracingController::EnableRecordingDoneCallback())) {
+    is_running_ = true;
+    return command->SuccessResponse(NULL);
   }
+  return command->InternalErrorResponse("Failed to start tracing");
+}
 
-  TraceController::GetInstance()->BeginTracing(this, categories, options);
-  is_running_ = true;
+scoped_refptr<DevToolsProtocol::Response>
+DevToolsTracingHandler::OnGetTraceBufferPercentFull(
+    scoped_refptr<DevToolsProtocol::Command> command) {
+  TracingController::GetInstance()->GetTraceBufferPercentFull(
+      base::Bind(&DevToolsTracingHandler::OnTraceBufferPercentFullResult,
+                 base::Unretained(this)));

[piman, 2013/11/15 23:42:52]

Same here, what makes Unretained safe?

[Xianzhu, 2013/11/16 01:16:14]

Done.

   return command->SuccessResponse(NULL);
 }
 
 scoped_refptr<DevToolsProtocol::Response>
 DevToolsTracingHandler::OnEnd(
     scoped_refptr<DevToolsProtocol::Command> command) {
-  TraceController::GetInstance()->EndTracingAsync(this);
+  TracingController::GetInstance()->DisableRecording(
+      base::FilePath(),
+      base::Bind(&DevToolsTracingHandler::BeginReadingRecordingResult,
+                 base::Unretained(this)));

[piman, 2013/11/15 23:42:52]

And here.

[Xianzhu, 2013/11/16 01:16:14]

Done.

   return command->SuccessResponse(NULL);
 }