Remove TraceController

content/browser/devtools/devtools_tracing_handler.cc

 // 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.
 
 #include "content/browser/devtools/devtools_tracing_handler.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/file_util.h"
+#include "base/json/json_reader.h"
+#include "base/json/json_writer.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) {
+    : is_running_(false),
+      weak_factory_(this) {
   RegisterCommandHandler(devtools::Tracing::start::kName,
                          base::Bind(&DevToolsTracingHandler::OnStart,
                                     base::Unretained(this)));
   RegisterCommandHandler(devtools::Tracing::end::kName,
                          base::Bind(&DevToolsTracingHandler::OnEnd,
                                     base::Unretained(this)));
 }
 
 DevToolsTracingHandler::~DevToolsTracingHandler() {
 }
 
-void DevToolsTracingHandler::OnEndTracingComplete() {
+void DevToolsTracingHandler::ReadRecordingResult(const base::FilePath& path) {

[pfeldman, 2013/11/20 00:10:10]

Please make sure order of methods in cc matches the one in the header.

[Xianzhu, 2013/11/20 00:37:22]

Done.

+  if (!is_running_)
+    return;
+
+  std::string trace_data;
+  if (!base::ReadFileToString(path, &trace_data)) {

[pfeldman, 2013/11/20 00:10:10]

Why do we need to deal with files in the handler?

[Xianzhu, 2013/11/20 00:37:22]

This is how the new TracingController interface is defined. I didn't contribute
discussion when defining the interface, but I think the purpose is to simplify
most of the clients (single callback, avoid passing big data through IPC, etc.)

Here the code becomes more complex because we need to conform to the original
devtools protocol. My next step work for
https://code.google.com/p/trace-viewer/issues/detail?id=414) will use and change
the protocol and simplify code here.

BTW, is there already existing client of the protocol about tracing?

+    LOG(ERROR) << "Failed to read file: " << path.value();
+    OnTraceDataCollected("");
+  } else {
+    scoped_ptr<base::Value> trace_value(base::JSONReader::Read(trace_data));
+    DictionaryValue* dictionary = NULL;
+    bool ok = trace_value->GetAsDictionary(&dictionary);
+    DCHECK(ok);
+    ListValue* list = NULL;
+    ok = dictionary->GetList("traceEvents", &list);
+    DCHECK(ok);
+    std::string buffer;
+    for (size_t i = 0; i < list->GetSize(); ++i) {
+      std::string item;
+      base::Value* item_value;
+      list->Get(i, &item_value);
+      base::JSONWriter::Write(item_value, &item);
+      if (buffer.size())
+        buffer.append(",");
+      buffer.append(item);
+      if (i % 1000 == 0) {
+        OnTraceDataCollected(buffer);
+        buffer.clear();
+      }
+    }
+    if (buffer.size())
+      OnTraceDataCollected(buffer);
+  }
+  base::DeleteFile(path, false);
+
   is_running_ = false;

[pfeldman, 2013/11/20 00:10:10]

Do we still need is_running_?

[Xianzhu, 2013/11/20 00:37:22]

Seems useless (even in the original code). Removed.

   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);
-  }
+    const std::string& trace_fragment) {
+  // 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.c_str());
+  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(
+void DevToolsTracingHandler::BeginReadingRecordingResult(
+    const base::FilePath& path) {
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DevToolsTracingHandler::ReadRecordingResult,
+                 weak_factory_.GetWeakPtr(), path));
+}
+
+TracingController::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;
+      ret &= ~TracingController::RECORD_CONTINUOUSLY;

[pfeldman, 2013/11/20 00:10:10]

Is this change intentional?

[Xianzhu, 2013/11/20 00:37:22]

Yes. TracingController::Options doesn't have RECORD_UNTIL_FULL, but imply it by
default. Here just ensures the RECORD_CONTINUOUSLY bit is cleared if
kRecordUntilFull is specified (in case kRecordContinuously is specified
eariler). Can also be a no-op.

Will change the code here when changing the devtools tracing protocol.

     } else if (*iter == kRecordContinuously) {
-      ret |= base::debug::TraceLog::RECORD_CONTINUOUSLY;
+      ret |= TracingController::RECORD_CONTINUOUSLY;
     } else if (*iter == kEnableSampling) {
-      ret |= base::debug::TraceLog::ENABLE_SAMPLING;
+      ret |= TracingController::ENABLE_SAMPLING;
     }
   }
-  if (!(ret & base::debug::TraceLog::RECORD_UNTIL_FULL) &&
-      !(ret & base::debug::TraceLog::RECORD_CONTINUOUSLY))
-    ret |= base::debug::TraceLog::RECORD_UNTIL_FULL;
-
-  return static_cast<base::debug::TraceLog::Options>(ret);
+  return static_cast<TracingController::Options>(ret);
 }
 
 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);
 
-  base::debug::TraceLog::Options options =
-      base::debug::TraceLog::RECORD_UNTIL_FULL;
+  TracingController::Options options = TracingController::DEFAULT_OPTIONS;
   if (params && params->HasKey(devtools::Tracing::start::kTraceOptions)) {
     std::string options_param;
     params->GetString(devtools::Tracing::start::kTraceOptions, &options_param);
     options = TraceOptionsFromString(options_param);
   }
 
-  TraceController::GetInstance()->BeginTracing(this, categories, options);
+  TracingController::GetInstance()->EnableRecording(
+      categories, options,
+      TracingController::EnableRecordingDoneCallback());
   is_running_ = true;
   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,
+                 weak_factory_.GetWeakPtr()));
   return command->SuccessResponse(NULL);
 }
 
 }  // namespace content