Reland "Perfetto: Unify TraceEventAgent classes"

This is a reland of aa69df1c

Includes a fix for flakiness of tests:
 SystemDisplayApiTest.SetDisplay (crbug.com/901586) and
- HeadlessBrowserTest.CreateAndDestroyBrowserContext (crbug.com/901637)

Original change's description:
> Perfetto: Unify TraceEventAgent classes
>
> This unifies the Perfetto and non-Perfetto TraceEventAgent classes, so
> that it's possible to use Perfetto directly even when it's not enabled
> as the default TraceLog backend.
>
> Also moved the Mojo bindings of ProducerHost into a StrongBindingSet of PerfettoService
> so they get cleaned up if the service gets shut down.
>
> Already reviewed as part of https://chromium-review.googlesource.com/c/chromium/src/+/1161541
>
> Change-Id: I3070d69e92778de8e3b5e33957ab4f4694b782af
> Reviewed-on: https://chromium-review.googlesource.com/c/1307998
> Commit-Queue: oysteine <oysteine@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Reviewed-by: oysteine <oysteine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605060}

Change-Id: Ic0e57577e5f63ece544e48f9bc01ae597cd0b3be
Reviewed-on: https://chromium-review.googlesource.com/c/1321585
Commit-Queue: oysteine <oysteine@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606134}

services/tracing/coordinator.h

@@ -39,6 +39,10 @@ namespace tracing {
 //
 // If we see that the above-mentioned assumption does not hold in some cases, we
 // should guard against it using timeouts.
+//
+// Note that this class is only used when TraceLog is used as the tracing
+// backend; when Perfetto is used, PerfettoTracingCoordinator is used instead to
+// implement the same interface.
 class Coordinator : public mojom::Coordinator {
  public:
   explicit Coordinator(AgentRegistry* agent_registry);

services/tracing/perfetto/perfetto_integration_unittest.cc

@@ -33,7 +33,6 @@ class PerfettoIntegrationTest : public testing::Test {
   void TearDown() override { perfetto_service_.reset(); }
 
   PerfettoService* perfetto_service() const { return perfetto_service_.get(); }
-
   void RunUntilIdle() { scoped_task_environment_.RunUntilIdle(); }
 
  private:

services/tracing/perfetto/perfetto_service.cc

@@ -6,11 +6,11 @@
 
 #include <utility>
 
+#include "base/no_destructor.h"
 #include "base/task/post_task.h"
 #include "services/service_manager/public/cpp/bind_source_info.h"
 #include "services/tracing/perfetto/producer_host.h"
 #include "services/tracing/public/cpp/perfetto/shared_memory.h"
-
 #include "third_party/perfetto/include/perfetto/tracing/core/tracing_service.h"
 
 namespace tracing {
@@ -19,12 +19,6 @@ namespace {
 
 const char kPerfettoProducerName[] = "org.chromium.perfetto_producer";
 
-PerfettoService* g_perfetto_service;
-
-// Just used to destroy disconnected clients.
-template <typename T>
-void OnClientDisconnect(std::unique_ptr<T>) {}
-
 }  // namespace
 
 /*
@@ -46,17 +40,23 @@ The above should be resolved before we move the Perfetto usage out from the
 flag so we can run this on non-thread-bound sequence.
 */
 
+// static
+PerfettoService* PerfettoService::GetInstance() {
+  static base::NoDestructor<PerfettoService> perfetto_service;
+  return perfetto_service.get();
+}
+
 PerfettoService::PerfettoService(
     scoped_refptr<base::SequencedTaskRunner> task_runner_for_testing)
     : perfetto_task_runner_(
           task_runner_for_testing
               ? task_runner_for_testing
               : base::CreateSingleThreadTaskRunnerWithTraits(
-                    {base::MayBlock(), base::WithBaseSyncPrimitives(),
+                    {base::MayBlock(),
+                     base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN,
+                     base::WithBaseSyncPrimitives(),
                      base::TaskPriority::BEST_EFFORT},
                     base::SingleThreadTaskRunnerThreadMode::DEDICATED)) {
-  DCHECK(!g_perfetto_service);
-  g_perfetto_service = this;
   DETACH_FROM_SEQUENCE(sequence_checker_);
   perfetto_task_runner_.task_runner()->PostTask(
       FROM_HERE, base::BindOnce(&PerfettoService::CreateServiceOnSequence,
@@ -64,13 +64,7 @@ PerfettoService::PerfettoService(
 }
 
 PerfettoService::~PerfettoService() {
-  DCHECK_EQ(g_perfetto_service, this);
-  g_perfetto_service = nullptr;
-}
-
-// static
-PerfettoService* PerfettoService::GetInstance() {
-  return g_perfetto_service;
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
 void PerfettoService::CreateServiceOnSequence() {
@@ -103,13 +97,13 @@ void PerfettoService::BindOnSequence(
 
 void PerfettoService::ConnectToProducerHost(
     mojom::ProducerClientPtr producer_client,
-    mojom::ProducerHostRequest producer_host) {
+    mojom::ProducerHostRequest producer_host_request) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto new_producer = std::make_unique<ProducerHost>();
-  new_producer->Initialize(std::move(producer_client), std::move(producer_host),
-                           service_.get(), kPerfettoProducerName);
-  new_producer->set_connection_error_handler(base::BindOnce(
-      &OnClientDisconnect<ProducerHost>, std::move(new_producer)));
+  new_producer->Initialize(std::move(producer_client), service_.get(),
+                           kPerfettoProducerName);
+  producer_bindings_.AddBinding(std::move(new_producer),
+                                std::move(producer_host_request));
 }
 
 }  // namespace tracing

services/tracing/perfetto/perfetto_service.h

@@ -12,6 +12,7 @@
 #include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/strong_binding_set.h"
 #include "services/service_manager/public/cpp/identity.h"
 #include "services/tracing/public/cpp/perfetto/task_runner.h"
 #include "services/tracing/public/mojom/perfetto_service.mojom.h"
@@ -58,6 +59,7 @@ class PerfettoService : public mojom::PerfettoService {
   PerfettoTaskRunner perfetto_task_runner_;
   std::unique_ptr<perfetto::TracingService> service_;
   mojo::BindingSet<mojom::PerfettoService, service_manager::Identity> bindings_;
+  mojo::StrongBindingSet<mojom::ProducerHost> producer_bindings_;
   SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(PerfettoService);

services/tracing/perfetto/producer_host.cc

@@ -17,19 +17,12 @@ ProducerHost::ProducerHost() = default;
 ProducerHost::~ProducerHost() = default;
 
 void ProducerHost::Initialize(mojom::ProducerClientPtr producer_client,
-                              mojom::ProducerHostRequest producer_host,
                               perfetto::TracingService* service,
                               const std::string& name) {
   DCHECK(service);
   DCHECK(!producer_endpoint_);
-  producer_client_ = std::move(producer_client);
-  producer_client_.set_connection_error_handler(
-      base::BindOnce(&ProducerHost::OnConnectionError, base::Unretained(this)));
 
-  binding_ = std::make_unique<mojo::Binding<mojom::ProducerHost>>(
-      this, std::move(producer_host));
-  binding_->set_connection_error_handler(
-      base::BindOnce(&ProducerHost::OnConnectionError, base::Unretained(this)));
+  producer_client_ = std::move(producer_client);
 
   // TODO(oysteine): Figure out an uid once we need it.
   // TODO(oysteine): Figure out a good buffer size.
@@ -37,19 +30,15 @@ void ProducerHost::Initialize(mojom::ProducerClientPtr producer_client,
       this, 0 /* uid */, name,
       4 * 1024 * 1024 /* shared_memory_size_hint_bytes */);
   DCHECK(producer_endpoint_);
+
+  producer_client_.set_connection_error_handler(
+      base::BindOnce(&ProducerHost::OnConnectionError, base::Unretained(this)));
 }
 
 void ProducerHost::OnConnectionError() {
   // Manually reset to prevent any callbacks from the ProducerEndpoint
   // when we're in a half-destructed state.
   producer_endpoint_.reset();
-  // If the ProducerHost is owned by the PerfettoService, let it know
-  // we're disconnected to let this be cleaned up. Tests manage lifespan
-  // themselves.
-  if (connection_error_handler_) {
-    std::move(connection_error_handler_).Run();
-  }
-  // This object *may* be destroyed at this point.
 }
 
 void ProducerHost::OnConnect() {

services/tracing/perfetto/producer_host.h

@@ -11,12 +11,8 @@
 #include <vector>
 
 #include "base/macros.h"
-
 #include "services/tracing/perfetto/producer_host.h"
 #include "services/tracing/public/mojom/perfetto_service.mojom.h"
-
-#include "mojo/public/cpp/bindings/binding.h"
-
 #include "third_party/perfetto/include/perfetto/tracing/core/producer.h"
 #include "third_party/perfetto/include/perfetto/tracing/core/tracing_service.h"
 
@@ -41,16 +37,10 @@ class ProducerHost : public tracing::mojom::ProducerHost,
   ProducerHost();
   ~ProducerHost() override;
 
-  void set_connection_error_handler(
-      base::OnceClosure connection_error_handler) {
-    connection_error_handler_ = std::move(connection_error_handler);
-  }
-
   // Called by the ProducerService to register the
   // Producer with Perfetto and connect to the
   // corresponding remote ProducerClient.
   void Initialize(mojom::ProducerClientPtr producer_client,
-                  mojom::ProducerHostRequest producer_host,
                   perfetto::TracingService* service,
                   const std::string& name);
 
@@ -95,8 +85,6 @@ class ProducerHost : public tracing::mojom::ProducerHost,
 
  private:
   mojom::ProducerClientPtr producer_client_;
-  std::unique_ptr<mojo::Binding<mojom::ProducerHost>> binding_;
-  base::OnceClosure connection_error_handler_;
 
  protected:
   // Perfetto guarantees that no OnXX callbacks are invoked on |this|

services/tracing/perfetto/test_utils.cc

@@ -233,8 +233,10 @@ void MockProducerHost::OnMessagepipesReadyCallback(
     perfetto::TracingService* perfetto_service,
     mojom::ProducerClientPtr producer_client_pipe,
     mojom::ProducerHostRequest producer_host_pipe) {
-  Initialize(std::move(producer_client_pipe), std::move(producer_host_pipe),
-             perfetto_service, kPerfettoProducerName);
+  Initialize(std::move(producer_client_pipe), perfetto_service,
+             kPerfettoProducerName);
+  binding_ = std::make_unique<mojo::Binding<mojom::ProducerHost>>(
+      this, std::move(producer_host_pipe));
 }
 
 MockProducer::MockProducer(const std::string& data_source_name,

services/tracing/perfetto/test_utils.h

@@ -140,6 +140,7 @@ class MockProducerHost : public ProducerHost {
   base::OnceClosure datasource_registered_callback_;
   const std::string data_source_name_;
   std::string all_host_commit_data_requests_;
+  std::unique_ptr<mojo::Binding<mojom::ProducerHost>> binding_;
 };
 
 class MockProducer {

services/tracing/public/cpp/perfetto/producer_client.cc

@@ -100,6 +100,10 @@ void ProducerClient::CreateMojoMessagepipesOnSequence(
   origin_task_runner->PostTask(
       FROM_HERE, base::BindOnce(std::move(callback), std::move(producer_client),
                                 mojo::MakeRequest(&producer_host_)));
+
+  for (auto* data_source : data_sources_) {
+    RegisterDataSourceWithHost(data_source);
+  }
 }
 
 void ProducerClient::AddDataSource(DataSourceBase* data_source) {
@@ -110,7 +114,14 @@ void ProducerClient::AddDataSource(DataSourceBase* data_source) {
 
 void ProducerClient::AddDataSourceOnSequence(DataSourceBase* data_source) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   data_sources_.insert(data_source);
+  if (producer_host_) {
+    RegisterDataSourceWithHost(data_source);
+  }
+}
+
+void ProducerClient::RegisterDataSourceWithHost(DataSourceBase* data_source) {
   auto new_registration = mojom::DataSourceRegistration::New();
   new_registration->name = data_source->name();
   new_registration->will_notify_on_stop = true;
@@ -164,7 +175,7 @@ void ProducerClient::StopDataSource(uint64_t id,
     }
   }
 
-  LOG(FATAL) << "Invalid data source ID.";
+  LOG(DFATAL) << "Invalid data source ID.";
 }
 
 void ProducerClient::Flush(uint64_t flush_request_id,

services/tracing/public/cpp/perfetto/producer_client.h

@@ -128,6 +128,7 @@ class COMPONENT_EXPORT(TRACING_CPP) ProducerClient
  private:
   void CommitDataOnSequence(mojom::CommitDataRequestPtr request);
   void AddDataSourceOnSequence(DataSourceBase*);
+  void RegisterDataSourceWithHost(DataSourceBase* data_source);
 
   // The callback will be run on the |origin_task_runner|, meaning
   // the same sequence as CreateMojoMessagePipes() got called on.

services/tracing/public/cpp/perfetto/trace_event_data_source.cc

@@ -379,7 +379,6 @@ TraceEventDataSource* TraceEventDataSource::GetInstance() {
 
 TraceEventDataSource::TraceEventDataSource()
     : DataSourceBase(mojom::kTraceEventDataSourceName) {
-  RegisterTracedValueProtoWriter();
 }
 
 TraceEventDataSource::~TraceEventDataSource() = default;
@@ -395,6 +394,8 @@ void TraceEventDataSource::StartTracing(
     target_buffer_ = data_source_config.target_buffer;
   }
 
+  RegisterTracedValueProtoWriter(true);
+
   TraceLog::GetInstance()->SetAddTraceEventOverride(
       &TraceEventDataSource::OnAddTraceEvent,
       &TraceEventDataSource::FlushCurrentThread);
@@ -414,6 +415,7 @@ void TraceEventDataSource::StopTracing(
           return;
         }
 
+        RegisterTracedValueProtoWriter(false);
         TraceLog::GetInstance()->SetAddTraceEventOverride(nullptr, nullptr);
 
         // TraceLog::CancelTracing will cause metadata events to be written;

services/tracing/public/cpp/perfetto/traced_value_proto_writer.cc

@@ -258,8 +258,8 @@ std::unique_ptr<TracedValue::Writer> CreateProtoWriter(size_t capacity) {
 
 }  // namespace
 
-void RegisterTracedValueProtoWriter() {
-  TracedValue::SetWriterFactoryCallback(&CreateProtoWriter);
+void RegisterTracedValueProtoWriter(bool enable) {
+  TracedValue::SetWriterFactoryCallback(enable ? &CreateProtoWriter : nullptr);
 }
 
 }  // namespace tracing

services/tracing/public/cpp/perfetto/traced_value_proto_writer.h

@@ -35,7 +35,7 @@ class COMPONENT_EXPORT(TRACING_CPP) PerfettoProtoAppender
   perfetto::protos::pbzero::ChromeTraceEvent_Arg* proto_;
 };
 
-void COMPONENT_EXPORT(TRACING_CPP) RegisterTracedValueProtoWriter();
+void COMPONENT_EXPORT(TRACING_CPP) RegisterTracedValueProtoWriter(bool enable);
 }  // namespace tracing
 
 #endif  // SERVICES_TRACING_PUBLIC_CPP_PERFETTO_TRACED_VALUE_PROTO_WRITER_H_

services/tracing/public/cpp/perfetto/traced_value_proto_writer_unittest.cc

@@ -68,9 +68,9 @@ class ProtoInputStream : public google::protobuf::io::ZeroCopyInputStream {
 
 class TracedValueProtoWriterTest : public testing::Test {
  public:
-  void SetUp() override { RegisterTracedValueProtoWriter(); }
+  void SetUp() override { RegisterTracedValueProtoWriter(true); }
 
-  void TearDown() override {}
+  void TearDown() override { RegisterTracedValueProtoWriter(false); }
 };
 
 const perfetto::protos::ChromeTracedValue* FindDictEntry(

services/tracing/public/cpp/trace_event_agent.cc

@@ -51,67 +51,50 @@ ProducerClient* GetProducerClient() {
   return producer_client.get();
 }
 
-}  // namespace
+void InitializeProducerClient(service_manager::Connector* connector) {
+  mojom::PerfettoServicePtr perfetto_service;
+  connector->BindInterface(mojom::kServiceName, &perfetto_service);
+
+  GetProducerClient()->CreateMojoMessagepipes(base::BindOnce(
+      [](mojom::PerfettoServicePtr perfetto_service,
+         mojom::ProducerClientPtr producer_client_pipe,
+         mojom::ProducerHostRequest producer_host_pipe) {
+        perfetto_service->ConnectToProducerHost(std::move(producer_client_pipe),
+                                                std::move(producer_host_pipe));
+      },
+      std::move(perfetto_service)));
+
+  GetProducerClient()->AddDataSource(TraceEventDataSource::GetInstance());
+}
 
-class PerfettoTraceEventAgent : public TraceEventAgent {
- public:
-  PerfettoTraceEventAgent(service_manager::Connector* connector,
-                          bool request_clock_sync_marker_on_android)
-      : TraceEventAgent(connector, request_clock_sync_marker_on_android) {
-    mojom::PerfettoServicePtr perfetto_service;
-    connector->BindInterface(mojom::kServiceName, &perfetto_service);
-
-    GetProducerClient()->CreateMojoMessagepipes(base::BindOnce(
-        [](mojom::PerfettoServicePtr perfetto_service,
-           mojom::ProducerClientPtr producer_client_pipe,
-           mojom::ProducerHostRequest producer_host_pipe) {
-          perfetto_service->ConnectToProducerHost(
-              std::move(producer_client_pipe), std::move(producer_host_pipe));
-        },
-        std::move(perfetto_service)));
-
-    GetProducerClient()->AddDataSource(TraceEventDataSource::GetInstance());
-  }
+void AddPerfettoMetadataGeneratorFunction(
+    TraceEventAgent::MetadataGeneratorFunction generator) {
+  // Instantiate and register the metadata data source on the first
+  // call.
+  static TraceEventMetadataSource* metadata_source = []() {
+    static base::NoDestructor<TraceEventMetadataSource> instance;
+    GetProducerClient()->AddDataSource(instance.get());
+    return instance.get();
+  }();
+
+  metadata_source->AddGeneratorFunction(generator);
+}
+
+}  // namespace
 
-  void AddMetadataGeneratorFunction(
-      MetadataGeneratorFunction generator) override {
-    // Instantiate and register the metadata data source on the first
-    // call.
-    static TraceEventMetadataSource* metadata_source = []() {
-      static base::NoDestructor<TraceEventMetadataSource> instance;
-      GetProducerClient()->AddDataSource(instance.get());
-      return instance.get();
-    }();
-
-    metadata_source->AddGeneratorFunction(generator);
-  }
-};
 #endif
 
 // static
 std::unique_ptr<TraceEventAgent> TraceEventAgent::Create(
     service_manager::Connector* connector,
     bool request_clock_sync_marker_on_android) {
-  std::unique_ptr<TraceEventAgent> new_agent;
 
-  if (TracingUsesPerfettoBackend()) {
 #if defined(PERFETTO_AVAILABLE)
-    new_agent = std::make_unique<PerfettoTraceEventAgent>(
-        connector, request_clock_sync_marker_on_android);
-#else
-    LOG(ERROR) << "Perfetto is not yet available for this platform; falling "
-                  "back to using legacy TraceLog";
+  InitializeProducerClient(connector);
 #endif
-  }
 
-  // Use legacy tracing if we're on an unsupported platform or the feature flag
-  // is disabled.
-  if (!new_agent) {
-    new_agent = std::make_unique<LegacyTraceEventAgent>(
-        connector, request_clock_sync_marker_on_android);
-  }
-
-  return new_agent;
+  return std::make_unique<TraceEventAgent>(
+      connector, request_clock_sync_marker_on_android);
 }
 
 TraceEventAgent::TraceEventAgent(service_manager::Connector* connector,
@@ -124,13 +107,16 @@ TraceEventAgent::TraceEventAgent(service_manager::Connector* connector,
 #else
                 false,
 #endif
-                base::trace_event::TraceLog::GetInstance()->process_id()) {
+                base::trace_event::TraceLog::GetInstance()->process_id()),
+      enabled_tracing_modes_(0) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(!g_trace_event_agent);
   g_trace_event_agent = this;
 }
 
 TraceEventAgent::~TraceEventAgent() {
   g_trace_event_agent = nullptr;
+  DCHECK(!trace_log_needs_me_);
 }
 
 void TraceEventAgent::RequestClockSyncMarker(
@@ -151,27 +137,18 @@ void TraceEventAgent::GetCategories(GetCategoriesCallback callback) {
   std::move(callback).Run(base::JoinString(category_vector, ","));
 }
 
-LegacyTraceEventAgent::LegacyTraceEventAgent(
-    service_manager::Connector* connector,
-    bool request_clock_sync_marker_on_android)
-    : TraceEventAgent(connector, request_clock_sync_marker_on_android),
-      enabled_tracing_modes_(0) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-}
-
-LegacyTraceEventAgent::~LegacyTraceEventAgent() {
-  DCHECK(!trace_log_needs_me_);
-}
-
-void LegacyTraceEventAgent::AddMetadataGeneratorFunction(
+void TraceEventAgent::AddMetadataGeneratorFunction(
     MetadataGeneratorFunction generator) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   metadata_generator_functions_.push_back(generator);
+#if defined(PERFETTO_AVAILABLE)
+  AddPerfettoMetadataGeneratorFunction(generator);
+#endif
 }
 
-void LegacyTraceEventAgent::StartTracing(const std::string& config,
-                                         base::TimeTicks coordinator_time,
-                                         StartTracingCallback callback) {
+void TraceEventAgent::StartTracing(const std::string& config,
+                                   base::TimeTicks coordinator_time,
+                                   StartTracingCallback callback) {
   DCHECK(!recorder_);
 #if defined(__native_client__)
   // NaCl and system times are offset by a bit, so subtract some time from
@@ -189,7 +166,7 @@ void LegacyTraceEventAgent::StartTracing(const std::string& config,
   std::move(callback).Run(true);
 }
 
-void LegacyTraceEventAgent::StopAndFlush(mojom::RecorderPtr recorder) {
+void TraceEventAgent::StopAndFlush(mojom::RecorderPtr recorder) {
   DCHECK(!recorder_);
   recorder_ = std::move(recorder);
   base::trace_event::TraceLog::GetInstance()->SetDisabled(
@@ -201,18 +178,18 @@ void LegacyTraceEventAgent::StopAndFlush(mojom::RecorderPtr recorder) {
       recorder_->AddMetadata(std::move(*metadata));
   }
   trace_log_needs_me_ = true;
-  base::trace_event::TraceLog::GetInstance()->Flush(base::Bind(
-      &LegacyTraceEventAgent::OnTraceLogFlush, base::Unretained(this)));
+  base::trace_event::TraceLog::GetInstance()->Flush(
+      base::Bind(&TraceEventAgent::OnTraceLogFlush, base::Unretained(this)));
 }
 
-void LegacyTraceEventAgent::RequestBufferStatus(
+void TraceEventAgent::RequestBufferStatus(
     RequestBufferStatusCallback callback) {
   base::trace_event::TraceLogStatus status =
       base::trace_event::TraceLog::GetInstance()->GetStatus();
   std::move(callback).Run(status.event_capacity, status.event_count);
 }
 
-void LegacyTraceEventAgent::OnTraceLogFlush(
+void TraceEventAgent::OnTraceLogFlush(
     const scoped_refptr<base::RefCountedString>& events_str,
     bool has_more_events) {
   if (!events_str->data().empty())

services/tracing/public/cpp/trace_event_agent.h

@@ -27,6 +27,11 @@ class Connector;
 
 namespace tracing {
 
+// Agent used to interface with the legacy tracing system.
+// When Perfetto is used for the backend instead of TraceLog,
+// most of the mojom::Agent functions will never be used
+// as the control signals will go through the Perfetto
+// interface instead.
 class COMPONENT_EXPORT(TRACING_CPP) TraceEventAgent : public BaseAgent {
  public:
   using MetadataGeneratorFunction =
@@ -36,40 +41,16 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventAgent : public BaseAgent {
       service_manager::Connector* connector,
       bool request_clock_sync_marker_on_android);
 
-  ~TraceEventAgent() override;
-
   TraceEventAgent(service_manager::Connector* connector,
-                  bool request_clock_sync_marker_on_android);
-
-  // mojom::Agent
-  void RequestClockSyncMarker(
-      const std::string& sync_id,
-      Agent::RequestClockSyncMarkerCallback callback) override;
-  void GetCategories(GetCategoriesCallback callback) override;
-
-  virtual void AddMetadataGeneratorFunction(
-      MetadataGeneratorFunction generator) = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TraceEventAgent);
-};
-
-// Agent used to interface with the legacy tracing system,
-// i.e. without Perfetto.
-class COMPONENT_EXPORT(TRACING_CPP) LegacyTraceEventAgent
-    : public TraceEventAgent {
- public:
-  LegacyTraceEventAgent(service_manager::Connector* connector,
                         bool request_clock_sync_marker_on_android);
 
-  void AddMetadataGeneratorFunction(
-      MetadataGeneratorFunction generator) override;
+  void AddMetadataGeneratorFunction(MetadataGeneratorFunction generator);
 
  private:
-  friend std::default_delete<LegacyTraceEventAgent>;  // For Testing
+  friend std::default_delete<TraceEventAgent>;      // For Testing
   friend class TraceEventAgentTest;                 // For Testing
 
-  ~LegacyTraceEventAgent() override;
+  ~TraceEventAgent() override;
 
   // mojom::Agent
   void StartTracing(const std::string& config,
@@ -77,6 +58,12 @@ class COMPONENT_EXPORT(TRACING_CPP) LegacyTraceEventAgent
                     StartTracingCallback callback) override;
   void StopAndFlush(mojom::RecorderPtr recorder) override;
 
+  void RequestClockSyncMarker(
+      const std::string& sync_id,
+      Agent::RequestClockSyncMarkerCallback callback) override;
+
+  void GetCategories(GetCategoriesCallback callback) override;
+
   void RequestBufferStatus(RequestBufferStatusCallback callback) override;
 
   void OnTraceLogFlush(const scoped_refptr<base::RefCountedString>& events_str,
@@ -89,7 +76,7 @@ class COMPONENT_EXPORT(TRACING_CPP) LegacyTraceEventAgent
 
   THREAD_CHECKER(thread_checker_);
 
-  DISALLOW_COPY_AND_ASSIGN(LegacyTraceEventAgent);
+  DISALLOW_COPY_AND_ASSIGN(TraceEventAgent);
 };
 
 }  // namespace tracing

services/tracing/public/cpp/trace_event_agent_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/callback_forward.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "base/test/trace_event_analyzer.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_config.h"
@@ -81,16 +82,12 @@ class MockRecorder : public mojom::Recorder {
 
 class TraceEventAgentTest : public testing::Test {
  public:
-  void SetUp() override {
-    message_loop_.reset(new base::MessageLoop());
-    agent_.reset(new LegacyTraceEventAgent(nullptr, false));
-  }
+  void SetUp() override { agent_.reset(new TraceEventAgent(nullptr, false)); }
 
   void TearDown() override {
     base::trace_event::TraceLog::GetInstance()->SetDisabled();
     recorder_.reset();
     agent_.reset();
-    message_loop_.reset();
   }
 
   void StartTracing(const std::string& categories) {
@@ -129,8 +126,8 @@ class TraceEventAgentTest : public testing::Test {
   MockRecorder* recorder() const { return recorder_.get(); }
 
  private:
-  std::unique_ptr<base::MessageLoop> message_loop_;
-  std::unique_ptr<LegacyTraceEventAgent> agent_;
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  std::unique_ptr<TraceEventAgent> agent_;
   std::unique_ptr<MockRecorder> recorder_;
 };
 

services/tracing/tracing_service.cc

@@ -35,10 +35,6 @@ TracingService::~TracingService() {
     task_runner_->DeleteSoon(FROM_HERE,
                              std::move(perfetto_tracing_coordinator_));
   }
-
-  if (perfetto_service_) {
-    task_runner_->DeleteSoon(FROM_HERE, std::move(perfetto_service_));
-  }
 #endif
 }
 
@@ -47,13 +43,13 @@ void TracingService::OnStart() {
 
   bool enable_legacy_tracing = true;
 
-  if (TracingUsesPerfettoBackend()) {
 #if defined(PERFETTO_SERVICE_AVAILABLE)
-    perfetto_service_ = std::make_unique<tracing::PerfettoService>();
-    task_runner_ = perfetto_service_->task_runner();
-    registry_.AddInterface(
-        base::BindRepeating(&tracing::PerfettoService::BindRequest,
-                            base::Unretained(perfetto_service_.get())));
+  registry_.AddInterface(base::BindRepeating(
+      &tracing::PerfettoService::BindRequest,
+      base::Unretained(tracing::PerfettoService::GetInstance())));
+
+  if (TracingUsesPerfettoBackend()) {
+    task_runner_ = tracing::PerfettoService::GetInstance()->task_runner();
 
     auto perfetto_coordinator = std::make_unique<PerfettoTracingCoordinator>(
         tracing_agent_registry_.get());
@@ -61,9 +57,15 @@ void TracingService::OnStart() {
         base::BindRepeating(&PerfettoTracingCoordinator::BindCoordinatorRequest,
                             base::Unretained(perfetto_coordinator.get())));
     perfetto_tracing_coordinator_ = std::move(perfetto_coordinator);
+
     enable_legacy_tracing = false;
-#endif
   }
+#else
+  if (TracingUsesPerfettoBackend()) {
+    LOG(ERROR) << "Perfetto is not yet available for this platform; falling "
+                  "back to using legacy TraceLog";
+  }
+#endif
 
   // Use legacy tracing if we're on an unsupported platform or the feature flag
   // is disabled.

services/tracing/tracing_service.h

@@ -25,7 +25,6 @@
 namespace tracing {
 
 class PerfettoTracingCoordinator;
-class PerfettoService;
 
 class TracingService : public service_manager::Service {
  public:
@@ -51,7 +50,6 @@ class TracingService : public service_manager::Service {
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
 #if defined(PERFETTO_SERVICE_AVAILABLE)
-  std::unique_ptr<tracing::PerfettoService> perfetto_service_;
   std::unique_ptr<PerfettoTracingCoordinator> perfetto_tracing_coordinator_;
 #endif