tracing: the client lib of the tracing service

services/resource_coordinator/public/cpp/tracing/trace_event_agent.h

+// Copyright 2017 The Chromium Authors. All rights reserved.

[Primiano Tucci (use gerrit), 2017/05/11 15:31:07]

is this "agent" going also to subsume the other base::trace_event::TracingAgent?
If not might be better to pick a different name to avoid confusion (dunno,
TracingClient?)

[chiniforooshan, 2017/05/15 20:44:12]

It's going to be a special base::trace_event::TracingAgent that sends back
chrome trace events. I renamed it to ChromeAgent as we discussed in the design
doc.

[Primiano Tucci (use gerrit), 2017/05/16 04:45:11]

tracing::ChromeAgent SGTM (or if you want even just tracing::Agent)

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_TRACING_TRACE_EVENT_AGENT_H_
+#define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_TRACING_TRACE_EVENT_AGENT_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/ref_counted_memory.h"
+#include "base/values.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "services/resource_coordinator/public/cpp/resource_coordinator_export.h"
+#include "services/resource_coordinator/public/interfaces/tracing/tracing.mojom.h"
+#include "services/service_manager/public/cpp/connector.h"
+
+namespace resource_coordinator {

[Primiano Tucci (use gerrit), 2017/05/11 15:31:07]

i wonder if we should just keep this simple and just use the "tracing"
namespace, like you did for memory_instrumentation . Seems it keeps the rest of
the code more concise.

[chiniforooshan, 2017/05/15 20:44:12]

Done.

+namespace tracing {
+
+class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT TraceEventAgent
+    : public mojom::Agent {
+ public:
+  typedef base::RepeatingCallback<std::unique_ptr<base::DictionaryValue>()>

[Primiano Tucci (use gerrit), 2017/05/11 15:31:07]

small nit: using MetadataGenerator = base...
seems to be the preferred and more readable c++ way

also can you call it MetadataGeneratorFunction or MetadataGeneratorCallback?
otherwise, wihtout context, this looks like a class and makes the code harder to
read.

[chiniforooshan, 2017/05/15 20:44:12]

Done.

+      MetadataGenerator;
+
+  static TraceEventAgent* GetOrCreateInstance(mojom::AgentSetPtr agent_set);

[Primiano Tucci (use gerrit), 2017/05/11 15:31:07]

can you add a comment on what the argument is for? I can't figure out the
agent_set straight away (at least until having seen the rest of the cl)

[chiniforooshan, 2017/05/15 20:44:12]

Done.

+
+  void AddMetadataGenerator(MetadataGenerator generator);
+
+ private:
+  friend std::default_delete<TraceEventAgent>;  // For Testing
+  friend class TraceEventAgentTest;             // For Testing
+
+  explicit TraceEventAgent(mojom::AgentSetPtr agent_set);
+  ~TraceEventAgent() override;
+
+  // mojom::Agent
+  void StartTracing(const std::string& config,
+                    mojom::RecorderPtr recorder,
+                    bool report_categories_only,
+                    const StartTracingCallback& callback) override;
+  void StopAndFlush() override;
+  void RequestClockSyncMarker(
+      const std::string& sync_id,
+      const RequestClockSyncMarkerCallback& callback) override;
+  void RequestBufferStatus(
+      const RequestBufferStatusCallback& callback) override;
+
+  void SendChunk(const scoped_refptr<base::RefCountedString>& events_str,
+                 bool has_more_events);
+
+  mojo::Binding<mojom::Agent> binding_;
+  mojom::RecorderPtr recorder_;
+  bool report_categories_only_;
+  std::vector<MetadataGenerator> metadata_generators_;
+
+  DISALLOW_COPY_AND_ASSIGN(TraceEventAgent);
+};
+
+}  // namespace tracing
+}  // namespace resource_coordinator
+#endif  // SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_TRACING_TRACE_EVENT_AGENT_H_