tracing v2: Introduce TraceBufferReader to read-back the trace buffer

components/tracing/core/trace_buffer_reader.h

Index: components/tracing/core/trace_buffer_reader.h
diff --git a/components/tracing/core/trace_buffer_reader.h b/components/tracing/core/trace_buffer_reader.h
new file mode 100644
index 0000000000000000000000000000000000000000..d37173a8f746806dc3a0d4037b7ec6376b08a35c
--- /dev/null
+++ b/components/tracing/core/trace_buffer_reader.h
@@ -0,0 +1,66 @@
+// Copyright 2016 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.
+
+#ifndef COMPONENTS_TRACING_CORE_TRACE_BUFFER_READER_H_
+#define COMPONENTS_TRACING_CORE_TRACE_BUFFER_READER_H_
+
+#include <memory>
+#include <unordered_map>
+
+#include "base/values.h"
+#include "components/tracing/core/trace_ring_buffer.h"
+#include "components/tracing/tracing_export.h"
+
+namespace tracing {
+namespace v2 {
+
+// Reads back chunks from the proto-encoded tracing buffer and:
+// - Extracts a table of interned strings, when kBuildInternedStringsTable is
+//   enabled.
+// - Reconstructs a JSON-serializable ListValue which is backwards compatible
+//   with Tracing v1, when kBuildLegacyJSONEvents.
+class TRACING_EXPORT TraceBufferReader {
+ public:
+  using InternedStringsMap = std::unordered_map<int32_t, const char*>;
+  static const uint32_t kBuildInternedStringsTable = 1 << 0;

[kraynov, 2016/09/08 16:10:01]

I don't see any point to use bit options. Is it expected to get many? It smells
like WinAPI otherwise :)

[oystein (OOO til 10th of July), 2016/09/09 00:27:35]

Ditto; are there actually any cases where both would be set at the same time?
Wouldn't it always be one or the other?

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

This is intended to be or-able (see the test).
In the transition period I think we'll need both the JSON and the string table
for the proto metadata (at very least for tests)

+  static const uint32_t kBuildLegacyJSONEvents = 1 << 1;
+
+  explicit TraceBufferReader(uint32_t options);
+  ~TraceBufferReader();
+
+  void ParseChunk(const TraceRingBuffer::Chunk*);
+  void ParseChunk(const uint8_t* start, size_t length);

[kraynov, 2016/09/08 16:10:01]

This pair of overloaded methods looks confusing. I suggest to use different
names, e.g. ParseChunk(Chunk*) ParseDataRange(...).

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

ok. Lemme just leave the start/length one for the moment and use that also in
prod code.

+
+  const InternedStringsMap& interned_strings() const {
+    return interned_strings_;
+  }
+
+  std::unique_ptr<base::ListValue>& legacy_events() { return legacy_events_; }
+
+ private:
+  struct WriterState {
+    WriterState();
+    ~WriterState();
+
+    void Reset();
+
+    bool last_fragment_is_valid;
+    uint32_t last_seq_id;
+    std::vector<uint8_t> partial_event_data;

[kraynov, 2016/09/08 16:10:01]

We took an effort to use dynamic allocation as least as we can. I think fixed
size buffer is more appropriate, but vector is also good as a temporay solution.

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

This happens when the trace is finalized, at this point we are out of the
fastpath.
This buffer is temporary. It can't be easily fixed because in theory one can
create a jumbo event as big as the trace which spans across all chunks. It would
be a waste creating such a big array, because in 99% of cases this will be
barely used.

+  };
+
+  void ParseEvent(const uint8_t* start, const uint8_t* end);
+
+  const uint32_t options_;
+  InternedStringsMap interned_strings_;
+  std::unique_ptr<base::ListValue> legacy_events_;
+  std::unordered_map<uint32_t, WriterState> writer_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(TraceBufferReader);
+};
+
+}  // namespace v2
+}  // namespace tracing
+
+#endif  // COMPONENTS_TRACING_CORE_TRACE_BUFFER_READER_H_