Tracing: do not pass entire trace content in TraceDataEndpoint::ReceiveTraceFinalContents

content/browser/tracing/tracing_controller_impl_data_sinks.cc

 // Copyright 2015 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 <utility>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/strings/pattern.h"
@@ skipping 11 matching lines @@
 class StringTraceDataEndpoint : public TraceDataEndpoint {
  public:
   typedef base::Callback<void(std::unique_ptr<const base::DictionaryValue>,
                               base::RefCountedString*)>
       CompletionCallback;
 
   explicit StringTraceDataEndpoint(CompletionCallback callback)
       : completion_callback_(callback) {}
 
   void ReceiveTraceFinalContents(
-      std::unique_ptr<const base::DictionaryValue> metadata,
-      const std::string& contents) override {
-    std::string tmp = contents;
+      std::unique_ptr<const base::DictionaryValue> metadata) override {
+    std::string tmp = trace_.str();
+    trace_.str("");

[oystein (OOO til 10th of July), 2016/07/14 22:04:33]

nit: Should technically also do trace_.clear() in addition to this, to clear
ostringstream flags.

     scoped_refptr<base::RefCountedString> str =
         base::RefCountedString::TakeString(&tmp);
 
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(completion_callback_, base::Passed(std::move(metadata)),
                    base::RetainedRef(str)));
   }
 
+  void ReceiveTraceChunk(const std::string& chunk) override { trace_ << chunk; }
+
  private:
   ~StringTraceDataEndpoint() override {}
 
   CompletionCallback completion_callback_;
+  std::ostringstream trace_;
 
   DISALLOW_COPY_AND_ASSIGN(StringTraceDataEndpoint);
 };
 
 class FileTraceDataEndpoint : public TraceDataEndpoint {
  public:
   explicit FileTraceDataEndpoint(const base::FilePath& trace_file_path,
                                  const base::Closure& callback)
       : file_path_(trace_file_path),
         completion_callback_(callback),
         file_(NULL) {}
 
   void ReceiveTraceChunk(const std::string& chunk) override {
     std::string tmp = chunk;
     scoped_refptr<base::RefCountedString> chunk_ptr =
         base::RefCountedString::TakeString(&tmp);
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&FileTraceDataEndpoint::ReceiveTraceChunkOnFileThread, this,
                    chunk_ptr));
   }
 
-  void ReceiveTraceFinalContents(std::unique_ptr<const base::DictionaryValue>,
-                                 const std::string& contents) override {
+  void ReceiveTraceFinalContents(
+      std::unique_ptr<const base::DictionaryValue>) override {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&FileTraceDataEndpoint::CloseOnFileThread, this));
   }
 
  private:
   ~FileTraceDataEndpoint() override { DCHECK(file_ == NULL); }
 
   void ReceiveTraceChunkOnFileThread(
       const scoped_refptr<base::RefCountedString> chunk) {
@@ skipping 49 matching lines @@
   const std::map<std::string, std::string>& GetAgentTrace() const;
   std::unique_ptr<base::DictionaryValue> TakeMetadata();
 
  private:
   std::map<std::string, std::string> additional_tracing_agent_trace_;
   std::unique_ptr<base::DictionaryValue> metadata_;
 
   DISALLOW_COPY_AND_ASSIGN(TraceDataSinkImplBase);
 };
 
-class StringTraceDataSink : public TraceDataSinkImplBase {
+class JSONTraceDataSink : public TraceDataSinkImplBase {
  public:
-  explicit StringTraceDataSink(scoped_refptr<TraceDataEndpoint> endpoint)
-      : endpoint_(endpoint) {}
+  explicit JSONTraceDataSink(scoped_refptr<TraceDataEndpoint> endpoint)
+      : endpoint_(endpoint), first_chunk_(true) {}
 
   void AddTraceChunk(const std::string& chunk) override {
     std::string trace_string;
-    if (trace_.empty())
+    if (first_chunk_)
       trace_string = "{\"" + std::string(kChromeTraceLabel) + "\":[";
     else
       trace_string = ",";
     trace_string += chunk;
+    first_chunk_ = false;
 
-    AddTraceChunkAndPassToEndpoint(trace_string);
-  }
-
-  void AddTraceChunkAndPassToEndpoint(const std::string& chunk) {
-    trace_ += chunk;
-
-    endpoint_->ReceiveTraceChunk(chunk);
+    endpoint_->ReceiveTraceChunk(trace_string);
   }
 
   void Close() override {
-    AddTraceChunkAndPassToEndpoint("]");
+    endpoint_->ReceiveTraceChunk("]");
 
     for (auto const &it : GetAgentTrace())
-      AddTraceChunkAndPassToEndpoint(",\"" + it.first + "\": " + it.second);
+      endpoint_->ReceiveTraceChunk(",\"" + it.first + "\": " + it.second);
 
     std::unique_ptr<base::DictionaryValue> metadata(TakeMetadata());
     std::string metadataJSON;
 
     if (base::JSONWriter::Write(*metadata, &metadataJSON) &&
         !metadataJSON.empty()) {
-      AddTraceChunkAndPassToEndpoint(
-          ",\"" + std::string(kMetadataTraceLabel) + "\": " + metadataJSON);
+      endpoint_->ReceiveTraceChunk(",\"" + std::string(kMetadataTraceLabel) +
+                                   "\": " + metadataJSON);
     }
 
-    AddTraceChunkAndPassToEndpoint("}");
-
-    endpoint_->ReceiveTraceFinalContents(std::move(metadata), trace_);
+    endpoint_->ReceiveTraceChunk("}");
+    endpoint_->ReceiveTraceFinalContents(std::move(metadata));
   }
 
  private:
-  ~StringTraceDataSink() override {}
+  ~JSONTraceDataSink() override {}
 
   scoped_refptr<TraceDataEndpoint> endpoint_;
-  std::string trace_;
-
-  DISALLOW_COPY_AND_ASSIGN(StringTraceDataSink);
+  bool first_chunk_;

[oystein (OOO til 10th of July), 2016/07/14 22:04:33]

nit: has_received_first_chunk_ (and inverted checks) may be clearer.

+  DISALLOW_COPY_AND_ASSIGN(JSONTraceDataSink);
 };
 
 class CompressedStringTraceDataSink : public TraceDataSinkImplBase {
  public:
   explicit CompressedStringTraceDataSink(
       scoped_refptr<TraceDataEndpoint> endpoint)
-      : endpoint_(endpoint), already_tried_open_(false) {}
+      : endpoint_(endpoint), already_tried_open_(false), first_chunk_(true) {}
 
   void AddTraceChunk(const std::string& chunk) override {
     std::string tmp = chunk;
     scoped_refptr<base::RefCountedString> chunk_ptr =
         base::RefCountedString::TakeString(&tmp);
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&CompressedStringTraceDataSink::AddTraceChunkOnFileThread,
                    this, chunk_ptr));
   }
@@ skipping 27 matching lines @@
                               MAX_WBITS + 16,
                               8,  // memLevel = 8 is default.
                               Z_DEFAULT_STRATEGY);
     return result == 0;
   }
 
   void AddTraceChunkOnFileThread(
       const scoped_refptr<base::RefCountedString> chunk_ptr) {
     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
     std::string trace;
-    if (compressed_trace_data_.empty())
+    if (first_chunk_)
       trace = "{\"" + std::string(kChromeTraceLabel) + "\":[";
     else
       trace = ",";
+    first_chunk_ = false;
     trace += chunk_ptr->data();
     AddTraceChunkAndCompressOnFileThread(trace, false);
   }
 
   void AddTraceChunkAndCompressOnFileThread(const std::string& chunk,
                                             bool finished) {
     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
     if (!OpenZStreamOnFileThread())
       return;
 
     const int kChunkSize = 0x4000;
 
     char buffer[kChunkSize];
     int err;
     stream_->avail_in = chunk.size();
     stream_->next_in = (unsigned char*)chunk.data();
     do {
       stream_->avail_out = kChunkSize;
       stream_->next_out = (unsigned char*)buffer;
       err = deflate(stream_.get(), finished ? Z_FINISH : Z_NO_FLUSH);
       if (err != Z_OK && (err != Z_STREAM_END && finished)) {
         stream_.reset();
         return;
       }
 
       int bytes = kChunkSize - stream_->avail_out;
       if (bytes) {
         std::string compressed_chunk = std::string(buffer, bytes);
-        compressed_trace_data_ += compressed_chunk;
         endpoint_->ReceiveTraceChunk(compressed_chunk);
       }
     } while (stream_->avail_out == 0);
   }
 
   void CloseOnFileThread() {
     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
     if (!OpenZStreamOnFileThread())
       return;
 
-    if (compressed_trace_data_.empty()) {
+    if (first_chunk_) {
       AddTraceChunkAndCompressOnFileThread(
           "{\"" + std::string(kChromeTraceLabel) + "\":[", false);
     }
     AddTraceChunkAndCompressOnFileThread("]", false);
 
     for (auto const &it : GetAgentTrace()) {
       AddTraceChunkAndCompressOnFileThread(
           ",\"" + it.first + "\": " + it.second, false);
     }
 
     std::unique_ptr<base::DictionaryValue> metadata(TakeMetadata());
     std::string metadataJSON;
     if (base::JSONWriter::Write(*metadata, &metadataJSON) &&
         !metadataJSON.empty()) {
       AddTraceChunkAndCompressOnFileThread(
           ",\"" + std::string(kMetadataTraceLabel) + "\": " + metadataJSON,
           false);
     }
     AddTraceChunkAndCompressOnFileThread("}", true);
 
     deflateEnd(stream_.get());
     stream_.reset();
 
-    endpoint_->ReceiveTraceFinalContents(std::move(metadata),
-                                         compressed_trace_data_);
+    endpoint_->ReceiveTraceFinalContents(std::move(metadata));
   }
 
   scoped_refptr<TraceDataEndpoint> endpoint_;
   std::unique_ptr<z_stream> stream_;
   bool already_tried_open_;
-  std::string compressed_trace_data_;
+  bool first_chunk_;
 
   DISALLOW_COPY_AND_ASSIGN(CompressedStringTraceDataSink);
 };
 
 }  // namespace
 
 TracingController::TraceDataSink::TraceDataSink() {}
 TracingController::TraceDataSink::~TraceDataSink() {}
 
 void TraceDataSinkImplBase::AddAgentTrace(const std::string& trace_label,
@@ skipping 14 matching lines @@
 }
 
 std::unique_ptr<base::DictionaryValue> TraceDataSinkImplBase::TakeMetadata() {
   return std::move(metadata_);
 }
 
 scoped_refptr<TracingController::TraceDataSink>
 TracingController::CreateStringSink(
     const base::Callback<void(std::unique_ptr<const base::DictionaryValue>,
                               base::RefCountedString*)>& callback) {
-  return new StringTraceDataSink(new StringTraceDataEndpoint(callback));
+  return new JSONTraceDataSink(new StringTraceDataEndpoint(callback));
 }
 
 scoped_refptr<TracingController::TraceDataSink>
 TracingController::CreateFileSink(const base::FilePath& file_path,
                                   const base::Closure& callback) {
-  return new StringTraceDataSink(
-      new FileTraceDataEndpoint(file_path, callback));
+  return new JSONTraceDataSink(new FileTraceDataEndpoint(file_path, callback));
 }
 
 scoped_refptr<TracingController::TraceDataSink>
 TracingControllerImpl::CreateCompressedStringSink(
     scoped_refptr<TraceDataEndpoint> endpoint) {
   return new CompressedStringTraceDataSink(endpoint);
 }
 
 scoped_refptr<TraceDataEndpoint> TracingControllerImpl::CreateCallbackEndpoint(
     const base::Callback<void(std::unique_ptr<const base::DictionaryValue>,
                               base::RefCountedString*)>& callback) {
   return new StringTraceDataEndpoint(callback);
 }
 
 }  // namespace content