Revert "Perfetto: Unify TraceEventAgent classes"

This reverts commit aa69df1c.

Reason for revert: culprit for flakes of
- 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}

TBR=oysteine@chromium.org,skyostil@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 901586,901637

Change-Id: I45f18d58e6355722c124ee753de5021e1d965da5
Reviewed-on: https://chromium-review.googlesource.com/c/1317298Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605268}

services/tracing/coordinator.h

@@ -39,10 +39,6 @@ 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_service.cc

@@ -21,6 +21,10 @@ 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
 
 /*
@@ -99,13 +103,13 @@ void PerfettoService::BindOnSequence(
 
 void PerfettoService::ConnectToProducerHost(
     mojom::ProducerClientPtr producer_client,
-    mojom::ProducerHostRequest producer_host_request) {
+    mojom::ProducerHostRequest producer_host) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto new_producer = std::make_unique<ProducerHost>();
-  new_producer->Initialize(std::move(producer_client), service_.get(),
-                           kPerfettoProducerName);
-  producer_bindings_.AddBinding(std::move(new_producer),
-                                std::move(producer_host_request));
+  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)));
 }
 
 }  // namespace tracing

services/tracing/perfetto/perfetto_service.h

@@ -12,7 +12,6 @@
 #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"
@@ -59,7 +58,6 @@ 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,12 +17,19 @@ 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)));
 
   // TODO(oysteine): Figure out an uid once we need it.
   // TODO(oysteine): Figure out a good buffer size.
@@ -30,15 +37,19 @@ 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,8 +11,12 @@
 #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"
 
@@ -37,10 +41,16 @@ 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);
 
@@ -85,6 +95,8 @@ 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,10 +233,8 @@ void MockProducerHost::OnMessagepipesReadyCallback(
     perfetto::TracingService* perfetto_service,
     mojom::ProducerClientPtr producer_client_pipe,
     mojom::ProducerHostRequest producer_host_pipe) {
-  Initialize(std::move(producer_client_pipe), perfetto_service,
-             kPerfettoProducerName);
-  binding_ = std::make_unique<mojo::Binding<mojom::ProducerHost>>(
-      this, std::move(producer_host_pipe));
+  Initialize(std::move(producer_client_pipe), std::move(producer_host_pipe),
+             perfetto_service, kPerfettoProducerName);
 }
 
 MockProducer::MockProducer(const std::string& data_source_name,

services/tracing/perfetto/test_utils.h

@@ -140,7 +140,6 @@ 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,10 +100,6 @@ 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) {
@@ -114,14 +110,7 @@ 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;
@@ -175,7 +164,7 @@ void ProducerClient::StopDataSource(uint64_t id,
     }
   }
 
-  LOG(DFATAL) << "Invalid data source ID.";
+  LOG(FATAL) << "Invalid data source ID.";
 }
 
 void ProducerClient::Flush(uint64_t flush_request_id,

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

@@ -128,7 +128,6 @@ 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,6 +379,7 @@ TraceEventDataSource* TraceEventDataSource::GetInstance() {
 
 TraceEventDataSource::TraceEventDataSource()
     : DataSourceBase(mojom::kTraceEventDataSourceName) {
+  RegisterTracedValueProtoWriter();
 }
 
 TraceEventDataSource::~TraceEventDataSource() = default;
@@ -394,8 +395,6 @@ void TraceEventDataSource::StartTracing(
     target_buffer_ = data_source_config.target_buffer;
   }
 
-  RegisterTracedValueProtoWriter(true);
-
   TraceLog::GetInstance()->SetAddTraceEventOverride(
       &TraceEventDataSource::OnAddTraceEvent,
       &TraceEventDataSource::FlushCurrentThread);
@@ -415,7 +414,6 @@ 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(bool enable) {
-  TracedValue::SetWriterFactoryCallback(enable ? &CreateProtoWriter : nullptr);
+void RegisterTracedValueProtoWriter() {
+  TracedValue::SetWriterFactoryCallback(&CreateProtoWriter);
 }
 
 }  // 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(bool enable);
+void COMPONENT_EXPORT(TRACING_CPP) RegisterTracedValueProtoWriter();
 }  // 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(true); }
+  void SetUp() override { RegisterTracedValueProtoWriter(); }
 
-  void TearDown() override { RegisterTracedValueProtoWriter(false); }
+  void TearDown() override {}
 };
 
 const perfetto::protos::ChromeTracedValue* FindDictEntry(

services/tracing/public/cpp/trace_event_agent.cc

@@ -51,50 +51,67 @@ ProducerClient* GetProducerClient() {
   return producer_client.get();
 }
 
-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());
-}
-
-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
 
+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 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)
-  InitializeProducerClient(connector);
+    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";
 #endif
+  }
 
-  return std::make_unique<TraceEventAgent>(
-      connector, request_clock_sync_marker_on_android);
+  // 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;
 }
 
 TraceEventAgent::TraceEventAgent(service_manager::Connector* connector,
@@ -107,16 +124,13 @@ TraceEventAgent::TraceEventAgent(service_manager::Connector* connector,
 #else
                 false,
 #endif
-                base::trace_event::TraceLog::GetInstance()->process_id()),
-      enabled_tracing_modes_(0) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+                base::trace_event::TraceLog::GetInstance()->process_id()) {
   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(
@@ -137,18 +151,27 @@ void TraceEventAgent::GetCategories(GetCategoriesCallback callback) {
   std::move(callback).Run(base::JoinString(category_vector, ","));
 }
 
-void TraceEventAgent::AddMetadataGeneratorFunction(
+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(
     MetadataGeneratorFunction generator) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   metadata_generator_functions_.push_back(generator);
-#if defined(PERFETTO_AVAILABLE)
-  AddPerfettoMetadataGeneratorFunction(generator);
-#endif
 }
 
-void TraceEventAgent::StartTracing(const std::string& config,
-                                   base::TimeTicks coordinator_time,
-                                   StartTracingCallback callback) {
+void LegacyTraceEventAgent::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
@@ -166,7 +189,7 @@ void TraceEventAgent::StartTracing(const std::string& config,
   std::move(callback).Run(true);
 }
 
-void TraceEventAgent::StopAndFlush(mojom::RecorderPtr recorder) {
+void LegacyTraceEventAgent::StopAndFlush(mojom::RecorderPtr recorder) {
   DCHECK(!recorder_);
   recorder_ = std::move(recorder);
   base::trace_event::TraceLog::GetInstance()->SetDisabled(
@@ -178,18 +201,18 @@ void TraceEventAgent::StopAndFlush(mojom::RecorderPtr recorder) {
       recorder_->AddMetadata(std::move(*metadata));
   }
   trace_log_needs_me_ = true;
-  base::trace_event::TraceLog::GetInstance()->Flush(
-      base::Bind(&TraceEventAgent::OnTraceLogFlush, base::Unretained(this)));
+  base::trace_event::TraceLog::GetInstance()->Flush(base::Bind(
+      &LegacyTraceEventAgent::OnTraceLogFlush, base::Unretained(this)));
 }
 
-void TraceEventAgent::RequestBufferStatus(
+void LegacyTraceEventAgent::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 TraceEventAgent::OnTraceLogFlush(
+void LegacyTraceEventAgent::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,11 +27,6 @@ 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 =
@@ -41,16 +36,40 @@ 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);
+  void AddMetadataGeneratorFunction(
+      MetadataGeneratorFunction generator) override;
 
  private:
-  friend std::default_delete<TraceEventAgent>;      // For Testing
+  friend std::default_delete<LegacyTraceEventAgent>;  // For Testing
   friend class TraceEventAgentTest;                 // For Testing
 
-  ~TraceEventAgent() override;
+  ~LegacyTraceEventAgent() override;
 
   // mojom::Agent
   void StartTracing(const std::string& config,
@@ -58,12 +77,6 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventAgent : public BaseAgent {
                     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,
@@ -76,7 +89,7 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventAgent : public BaseAgent {
 
   THREAD_CHECKER(thread_checker_);
 
-  DISALLOW_COPY_AND_ASSIGN(TraceEventAgent);
+  DISALLOW_COPY_AND_ASSIGN(LegacyTraceEventAgent);
 };
 
 }  // namespace tracing

services/tracing/public/cpp/trace_event_agent_unittest.cc

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

services/tracing/tracing_service.cc

@@ -37,8 +37,7 @@ TracingService::~TracingService() {
   }
 
   if (perfetto_service_) {
-    perfetto_service_->task_runner()->DeleteSoon(FROM_HERE,
-                                                 std::move(perfetto_service_));
+    task_runner_->DeleteSoon(FROM_HERE, std::move(perfetto_service_));
   }
 #endif
 }
@@ -48,15 +47,13 @@ void TracingService::OnStart() {
 
   bool enable_legacy_tracing = true;
 
-#if defined(PERFETTO_SERVICE_AVAILABLE)
-  perfetto_service_ = std::make_unique<tracing::PerfettoService>();
-
-  registry_.AddInterface(
-      base::BindRepeating(&tracing::PerfettoService::BindRequest,
-                          base::Unretained(perfetto_service_.get())));
-
   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())));
 
     auto perfetto_coordinator = std::make_unique<PerfettoTracingCoordinator>(
         tracing_agent_registry_.get());
@@ -64,15 +61,9 @@ void TracingService::OnStart() {
         base::BindRepeating(&PerfettoTracingCoordinator::BindCoordinatorRequest,
                             base::Unretained(perfetto_coordinator.get())));
     perfetto_tracing_coordinator_ = std::move(perfetto_coordinator);
-
     enable_legacy_tracing = false;
-  }
-#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.