Reland "Revert "Revert "Enable system perfetto producer on all posix builds."""

Reason for reland: Keep ProducerEndpoint around forever.

Managed to repro the flakiness locally and therefore fix the issue. The
SMB was kept forever but the ProducerEndpoint contains the
unique_ptr<SharedMemory> that the SMB uses which meant that when it went
away we were left with a dangling pointer.

Bug: 1007310
Change-Id: I0e5e09169e369f4c3c7d93d3b23f1931d977fc64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2027368Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Stephen Nusko <nuskos@chromium.org>
Auto-Submit: Stephen Nusko <nuskos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737311}

content/browser/tracing/background_tracing_manager_browsertest.cc

@@ -38,7 +38,7 @@
 #include "services/tracing/public/cpp/tracing_features.h"
 #include "third_party/zlib/zlib.h"
 
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
 // Used by PerfettoSystemBackgroundScenario, nogncheck is required because this
 // is only included and used on Android.
 #include "services/tracing/perfetto/system_test_utils.h"  // nogncheck
@@ -65,7 +65,7 @@ perfetto::TraceConfig StopTracingTriggerConfig(
 }
 
 void SetSystemProducerSocketAndChecksAsync(const std::string& producer_socket) {
-  // We need to let the AndroidSystemProducer know the MockSystemService socket
+  // We need to let the PosixSystemProducer know the MockSystemService socket
   // address and that if we're running on Android devices older then Pie to
   // still connect.
   tracing::PerfettoTracedProcess::GetTaskRunner()
@@ -75,12 +75,12 @@ void SetSystemProducerSocketAndChecksAsync(const std::string& producer_socket) {
           base::BindOnce(
               [](const std::string& producer_socket) {
                 // The only other type of system producer is
-                // AndroidSystemProducer so this assert ensures that the
+                // PosixSystemProducer so this assert ensures that the
                 // static_cast below is safe.
                 ASSERT_FALSE(tracing::PerfettoTracedProcess::Get()
                                  ->SystemProducerForTesting()
                                  ->IsDummySystemProducerForTesting());
-                auto* producer = static_cast<tracing::AndroidSystemProducer*>(
+                auto* producer = static_cast<tracing::PosixSystemProducer*>(
                     tracing::PerfettoTracedProcess::Get()
                         ->SystemProducerForTesting());
                 producer->SetNewSocketForTesting(producer_socket.c_str());
@@ -104,7 +104,7 @@ std::unique_ptr<tracing::MockConsumer> CreateDefaultConsumer(
 }
 }  // namespace
 }  // namespace content
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 
 using base::trace_event::TraceLog;
 
@@ -1698,7 +1698,7 @@ IN_PROC_BROWSER_TEST_F(ProtoBackgroundTracingTest, ProtoTraceReceived) {
   EXPECT_TRUE(checker.stats().has_interned_source_locations);
 }
 
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
 IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
                        PerfettoSystemBackgroundScenarioDefaultName) {
   // This test will ensure that a BackgroundTracing scenario set to SYSTEM mode
@@ -1832,6 +1832,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
   // received we won't see any packets and |received_packets()| will be 0.
   EXPECT_LT(0u, system_consumer->received_packets());
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 
 }  // namespace content

content/test/BUILD.gn

@@ -1321,7 +1321,6 @@ test("content_browsertests") {
       "//content/shell:content_shell_lib",
       "//content/shell/android:content_shell_assets",
       "//content/shell/android:content_shell_jni_headers",
-      "//services/tracing:test_utils",
       "//testing/android/native_test:native_test_support",
       "//ui/android:android",
       "//ui/touch_selection:touch_selection",
@@ -1345,6 +1344,9 @@ test("content_browsertests") {
     ]
     deps += [ "//content/browser/speech/proto" ]
   }
+  if (is_posix) {
+    deps += [ "//services/tracing:test_utils" ]
+  }
 
   # HID support is not available without udev.
   is_linux_without_udev = is_linux && !use_udev
@@ -2069,6 +2071,7 @@ test("content_unittests") {
 
   if (is_posix) {
     sources += [ "../browser/posix_file_descriptor_info_impl_unittest.cc" ]
+    deps += [ "//services/tracing:test_utils" ]
   }
 
   if (enable_plugins) {

services/tracing/BUILD.gn

@@ -66,13 +66,11 @@ source_set("test_utils") {
     "//third_party/perfetto/protos/perfetto/trace:lite",
   ]
 
-  if (is_android) {
+  if (is_posix) {
     sources += [
       "perfetto/system_test_utils.cc",
       "perfetto/system_test_utils.h",
     ]
-
-    deps += [ "//third_party/perfetto/src/tracing/ipc/service" ]
   }
 }
 
@@ -110,9 +108,8 @@ source_set("tests") {
     "//third_party/perfetto/protos/perfetto/trace/track_event:lite",
   ]
 
-  if (is_android) {
+  if (is_posix) {
     sources += [ "perfetto/system_perfetto_unittest.cc" ]
-    deps += [ "//third_party/perfetto/include/perfetto/ext/tracing/ipc" ]
 
     if (can_unwind_with_cfi_table && is_official_build) {
       sources +=

services/tracing/perfetto/system_perfetto_unittest.cc

@@ -9,7 +9,6 @@
 #include <thread>
 #include <utility>
 
-#include "base/android/build_info.h"
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -20,6 +19,7 @@
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
+#include "build/build_config.h"
 #include "services/tracing/perfetto/perfetto_service.h"
 #include "services/tracing/perfetto/producer_host.h"
 #include "services/tracing/perfetto/system_test_utils.h"
@@ -33,6 +33,10 @@
 #include "third_party/perfetto/protos/perfetto/config/trace_config.pb.h"
 #include "third_party/perfetto/protos/perfetto/trace/trace.pb.h"
 
+#if defined(OS_ANDROID)
+#include "base/android/build_info.h"  // nogncheck
+#endif                                // defined(OS_ANDROID)
+
 namespace tracing {
 
 namespace {
@@ -114,22 +118,22 @@ class SystemPerfettoTest : public testing::Test {
     RunUntilIdle();
   }
 
-  std::unique_ptr<MockAndroidSystemProducer> CreateMockAndroidSystemProducer(
+  std::unique_ptr<MockPosixSystemProducer> CreateMockPosixSystemProducer(
       MockSystemService* service,
       int num_data_sources_expected = 0,
       base::RunLoop* system_data_source_enabled_runloop = nullptr,
       base::RunLoop* system_data_source_disabled_runloop = nullptr,
       bool check_sdk_level = false) {
-    std::unique_ptr<MockAndroidSystemProducer> result;
+    std::unique_ptr<MockPosixSystemProducer> result;
     base::RunLoop loop_finished;
-    // When we construct a MockAndroidSystemProducer it needs to be on the
+    // When we construct a MockPosixSystemProducer it needs to be on the
     // correct sequence. Construct it on the task runner and wait on the
     // |loop_finished| to ensure it is completely set up.
     PerfettoTracedProcess::GetTaskRunner()
         ->GetOrCreateTaskRunner()
         ->PostTaskAndReply(
             FROM_HERE, base::BindLambdaForTesting([&]() {
-              result.reset(new MockAndroidSystemProducer(
+              result.reset(new MockPosixSystemProducer(
                   service->producer(), check_sdk_level,
                   num_data_sources_expected,
                   system_data_source_enabled_runloop
@@ -235,7 +239,7 @@ TEST_F(SystemPerfettoTest, SystemTraceEndToEnd) {
   // Set up the producer to talk to the system.
   base::RunLoop system_data_source_enabled_runloop;
   base::RunLoop system_data_source_disabled_runloop;
-  auto system_producer = CreateMockAndroidSystemProducer(
+  auto system_producer = CreateMockPosixSystemProducer(
       system_service.get(),
       /* num_data_sources = */ 1, &system_data_source_enabled_runloop,
       &system_data_source_disabled_runloop);
@@ -269,52 +273,6 @@ TEST_F(SystemPerfettoTest, SystemTraceEndToEnd) {
   PerfettoProducer::DeleteSoonForTesting(std::move(system_producer));
 }
 
-// TODO(crbug/964324): We need to run this test in permissive mode, but
-// currently the bots don't do that. We should switch this to a telemetry
-// test to ensure our integration works on P+ Android devices.
-TEST_F(SystemPerfettoTest, DISABLED_SystemTraceEndToEndRealService) {
-  if (base::android::BuildInfo::GetInstance()->sdk_int() <
-      base::android::SDK_VERSION_P) {
-    LOG(INFO)
-        << "Skipping SystemTraceEndToEndRealService test, this phone "
-        << "is pre P SDK, which means there is no 'real' service running.";
-    return;
-  }
-
-  perfetto::protos::TraceConfig trace_config;
-  trace_config.add_buffers()->set_size_kb(1024);
-  trace_config.add_data_sources()->mutable_config()->set_name(
-      data_sources_[0]->name());
-  trace_config.add_data_sources()->mutable_config()->set_name(
-      data_sources_[2]->name());
-  trace_config.set_duration_ms(100);
-
-  std::string path = "/data/misc/perfetto-traces/trace";
-  path += RandomASCII(16);
-  EXPECT_TRUE(
-      ExecPerfetto({"-o", path, "-c"}, trace_config.SerializeAsString()))
-      << "failed with stderr: \"" << stderr_ << "\"";
-
-  char* data = new char[1024 * 1024];
-  ASSERT_TRUE(data);
-  int num_bytes = base::ReadFile(base::FilePath(path), data, (1024 * 1024) - 1);
-  EXPECT_NE(num_bytes, -1);
-  std::string output(data, num_bytes);
-  delete[] data;
-
-  perfetto::protos::Trace trace;
-  EXPECT_TRUE(trace.ParseFromString(output));
-
-  int count = 0;
-  for (const auto& packet : trace.packet()) {
-    if (packet.has_for_testing()) {
-      ++count;
-    }
-  }
-  EXPECT_EQ(1 + 7, count);
-  EXPECT_EQ(0, remove(path.c_str()));
-}
-
 TEST_F(SystemPerfettoTest, OneSystemSourceWithMultipleLocalSources) {
   auto system_service = CreateMockSystemService();
 
@@ -330,7 +288,7 @@ TEST_F(SystemPerfettoTest, OneSystemSourceWithMultipleLocalSources) {
 
   base::RunLoop system_data_source_enabled_runloop;
   base::RunLoop system_data_source_disabled_runloop;
-  auto system_producer = CreateMockAndroidSystemProducer(
+  auto system_producer = CreateMockPosixSystemProducer(
       system_service.get(),
       /* num_data_sources = */ 1, &system_data_source_enabled_runloop,
       &system_data_source_disabled_runloop);
@@ -472,7 +430,7 @@ TEST_F(SystemPerfettoTest, MultipleSystemSourceWithOneLocalSourcesLocalFirst) {
 
   base::RunLoop system_data_source_enabled_runloop;
   base::RunLoop system_data_source_disabled_runloop;
-  auto system_producer = CreateMockAndroidSystemProducer(
+  auto system_producer = CreateMockPosixSystemProducer(
       system_service.get(),
       /* num_data_sources = */ 3, &system_data_source_enabled_runloop,
       &system_data_source_disabled_runloop);
@@ -544,7 +502,7 @@ TEST_F(SystemPerfettoTest, MultipleSystemAndLocalSources) {
 
   base::RunLoop system_data_source_enabled_runloop;
   base::RunLoop system_data_source_disabled_runloop;
-  auto system_producer = CreateMockAndroidSystemProducer(
+  auto system_producer = CreateMockPosixSystemProducer(
       system_service.get(),
       /* num_data_sources = */ 3, &system_data_source_enabled_runloop,
       &system_data_source_disabled_runloop);
@@ -591,6 +549,16 @@ TEST_F(SystemPerfettoTest, MultipleSystemAndLocalSources) {
       system_data_source_reenabled_runloop.QuitClosure());
   system_producer->SetDataSourceDisabledCallback(
       system_data_source_redisabled_runloop.QuitClosure());
+  // |system_data_source_enabled| is called by the MockPosixSystemProducer after
+  // calling PosixSystemProducer::StartDataSource, BUT
+  // PosixSystemProducer::StartDataSource could not actual start the DataSource
+  // if local tracing hasn't yet finished. So waiting for data to be written
+  // (and thus implying the data source has started) is a better option.
+  std::vector<base::RunLoop> data_sources_wrote_data{data_sources_.size()};
+  for (size_t i = 0; i < data_sources_.size(); ++i) {
+    data_sources_[i]->set_start_tracing_callback(
+        data_sources_wrote_data[i].QuitClosure());
+  }
 
   local_consumer.StopTracing();
   local_data_source_disabled_runloop.Run();
@@ -599,6 +567,9 @@ TEST_F(SystemPerfettoTest, MultipleSystemAndLocalSources) {
 
   // Wait for system tracing to return before stopping.
   system_data_source_reenabled_runloop.Run();
+  for (auto& loop : data_sources_wrote_data) {
+    loop.Run();
+  }
   system_consumer.WaitForAllDataSourcesStarted();
 
   base::RunLoop stop_tracing;
@@ -629,7 +600,7 @@ TEST_F(SystemPerfettoTest, MultipleSystemAndLocalSourcesLocalFirst) {
   // trace starts.
   base::RunLoop system_data_source_enabled_runloop;
   base::RunLoop system_data_source_disabled_runloop;
-  auto system_producer = CreateMockAndroidSystemProducer(
+  auto system_producer = CreateMockPosixSystemProducer(
       system_service.get(),
       /* num_data_sources = */ 3, &system_data_source_enabled_runloop,
       &system_data_source_disabled_runloop);
@@ -727,6 +698,7 @@ TEST_F(SystemPerfettoTest, MultipleSystemAndLocalSourcesLocalFirst) {
   PerfettoProducer::DeleteSoonForTesting(std::move(system_producer));
 }
 
+#if defined(OS_ANDROID)
 TEST_F(SystemPerfettoTest, SystemToLowAPILevel) {
   if (base::android::BuildInfo::GetInstance()->sdk_int() >=
       base::android::SDK_VERSION_P) {
@@ -763,7 +735,7 @@ TEST_F(SystemPerfettoTest, SystemToLowAPILevel) {
 
     base::RunLoop system_data_source_enabled_runloop;
     base::RunLoop system_data_source_disabled_runloop;
-    auto system_producer = CreateMockAndroidSystemProducer(
+    auto system_producer = CreateMockPosixSystemProducer(
         system_service.get(),
         /* num_data_sources = */ 1, &system_data_source_enabled_runloop,
         &system_data_source_disabled_runloop, check_sdk_level);
@@ -817,8 +789,10 @@ TEST_F(SystemPerfettoTest, EnabledOnDebugBuilds) {
                     ->IsDummySystemProducerForTesting());
   }
 }
+#endif  // defined(OS_ANDROID)
 
 TEST_F(SystemPerfettoTest, RespectsFeatureList) {
+#if defined(OS_ANDROID)
   if (base::android::BuildInfo::GetInstance()->is_debug_android()) {
     // The feature list is ignored on debug android builds so we should have a
     // real system producer so just bail out of this test.
@@ -827,6 +801,7 @@ TEST_F(SystemPerfettoTest, RespectsFeatureList) {
                      ->IsDummySystemProducerForTesting());
     return;
   }
+#endif  // defined(OS_ANDROID)
   {
     base::test::ScopedFeatureList feature_list;
     feature_list.InitAndEnableFeature(features::kEnablePerfettoSystemTracing);

services/tracing/perfetto/system_test_utils.cc

@@ -82,13 +82,13 @@ perfetto::TracingService* MockSystemService::GetService() {
   return service_->service();
 }
 
-MockAndroidSystemProducer::MockAndroidSystemProducer(
+MockPosixSystemProducer::MockPosixSystemProducer(
     const std::string& socket,
     bool check_sdk_level,
     uint32_t num_data_sources,
     base::OnceClosure data_source_enabled_callback,
     base::OnceClosure data_source_disabled_callback)
-    : AndroidSystemProducer(socket.c_str(),
+    : PosixSystemProducer(socket.c_str(),
                           PerfettoTracedProcess::Get()->GetTaskRunner()),
       num_data_sources_expected_(num_data_sources),
       data_source_enabled_callback_(std::move(data_source_enabled_callback)),
@@ -97,7 +97,7 @@ MockAndroidSystemProducer::MockAndroidSystemProducer(
   // ownership of ourselves to PerfettoTracedProcess. Since someone else manages
   // our deletion we need to be careful in the deconstructor to not double free
   // ourselves (so we must call release once we get back our pointer.
-  std::unique_ptr<MockAndroidSystemProducer> client;
+  std::unique_ptr<MockPosixSystemProducer> client;
   client.reset(this);
   old_producer_ = PerfettoTracedProcess::Get()->SetSystemProducerForTesting(
       std::move(client));
@@ -105,7 +105,7 @@ MockAndroidSystemProducer::MockAndroidSystemProducer(
   Connect();
 }
 
-MockAndroidSystemProducer::~MockAndroidSystemProducer() {
+MockPosixSystemProducer::~MockPosixSystemProducer() {
   // See comment in the constructor.
   auto client = PerfettoTracedProcess::Get()->SetSystemProducerForTesting(
       std::move(old_producer_));
@@ -113,10 +113,10 @@ MockAndroidSystemProducer::~MockAndroidSystemProducer() {
   client.release();
 }
 
-void MockAndroidSystemProducer::StartDataSource(
+void MockPosixSystemProducer::StartDataSource(
     perfetto::DataSourceInstanceID id,
     const perfetto::DataSourceConfig& data_source_config) {
-  AndroidSystemProducer::StartDataSource(id, data_source_config);
+  PosixSystemProducer::StartDataSource(id, data_source_config);
   CHECK_LT(num_data_sources_active_, num_data_sources_expected_);
   if (++num_data_sources_active_ == num_data_sources_expected_ &&
       data_source_enabled_callback_) {
@@ -124,27 +124,27 @@ void MockAndroidSystemProducer::StartDataSource(
   }
 }
 
-void MockAndroidSystemProducer::StopDataSource(
+void MockPosixSystemProducer::StopDataSource(
     perfetto::DataSourceInstanceID id) {
-  AndroidSystemProducer::StopDataSource(id);
+  PosixSystemProducer::StopDataSource(id);
   CHECK_GT(num_data_sources_active_, 0u);
   if (--num_data_sources_active_ == 0 && data_source_disabled_callback_) {
     std::move(data_source_disabled_callback_).Run();
   }
 }
 
-void MockAndroidSystemProducer::CommitData(
+void MockPosixSystemProducer::CommitData(
     const perfetto::CommitDataRequest& commit,
     CommitDataCallback callback) {
-  AndroidSystemProducer::CommitData(commit, callback);
+  PosixSystemProducer::CommitData(commit, callback);
 }
 
-void MockAndroidSystemProducer::SetDataSourceEnabledCallback(
+void MockPosixSystemProducer::SetDataSourceEnabledCallback(
     base::OnceClosure data_source_enabled_callback) {
   data_source_enabled_callback_ = std::move(data_source_enabled_callback);
 }
 
-void MockAndroidSystemProducer::SetDataSourceDisabledCallback(
+void MockPosixSystemProducer::SetDataSourceDisabledCallback(
     base::OnceClosure data_source_disabled_callback) {
   data_source_disabled_callback_ = std::move(data_source_disabled_callback);
 }

services/tracing/perfetto/system_test_utils.h

@@ -7,7 +7,7 @@
 
 #include <string>
 
-#include "services/tracing/public/cpp/perfetto/android_system_producer.h"
+#include "services/tracing/public/cpp/perfetto/posix_system_producer.h"
 
 namespace base {
 class ScopedTempDir;
@@ -45,16 +45,16 @@ class MockSystemService {
   std::unique_ptr<perfetto::base::TaskRunner> task_runner_;
 };
 
-class MockAndroidSystemProducer : public AndroidSystemProducer {
+class MockPosixSystemProducer : public PosixSystemProducer {
  public:
-  MockAndroidSystemProducer(
+  MockPosixSystemProducer(
       const std::string& socket,
       bool check_sdk_level = false,
       uint32_t num_data_sources = 0,
       base::OnceClosure data_source_enabled_callback = base::OnceClosure(),
       base::OnceClosure data_source_disabled_callback = base::OnceClosure());
 
-  ~MockAndroidSystemProducer() override;
+  ~MockPosixSystemProducer() override;
 
   void StartDataSource(
       perfetto::DataSourceInstanceID id,

services/tracing/public/cpp/BUILD.gn

@@ -116,12 +116,11 @@ target(tracing_lib_type, "cpp") {
     ]
 
     # Add the SystemProducer implementations.
-    if (is_android) {
+    if (is_posix) {
       sources += [
-        "perfetto/android_system_producer.cc",
-        "perfetto/android_system_producer.h",
+        "perfetto/posix_system_producer.cc",
+        "perfetto/posix_system_producer.h",
       ]
-      deps += [ "//third_party/perfetto/src/tracing/ipc/producer" ]
     }
 
     if (is_android && can_unwind_with_cfi_table && is_official_build) {

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

@@ -15,7 +15,7 @@
 #include "services/tracing/public/cpp/trace_startup.h"
 #include "services/tracing/public/cpp/tracing_features.h"
 
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
 // As per 'gn help check':
 /*
   If you have conditional includes, make sure the build conditions and the
@@ -24,29 +24,29 @@
 */
 // We add the nogncheck to ensure this doesn't trigger incorrect errors on
 // non-android builds.
-#include "services/tracing/public/cpp/perfetto/android_system_producer.h"  // nogncheck
+#include "services/tracing/public/cpp/perfetto/posix_system_producer.h"  // nogncheck
 #include "third_party/perfetto/include/perfetto/ext/tracing/ipc/default_socket.h"  // nogncheck
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 
 namespace tracing {
 namespace {
 std::unique_ptr<SystemProducer> NewSystemProducer(PerfettoTaskRunner* runner,
                                                   const char* socket_name) {
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
   if (ShouldSetupSystemTracing()) {
     DCHECK(socket_name);
-    return std::make_unique<AndroidSystemProducer>(socket_name, runner);
+    return std::make_unique<PosixSystemProducer>(socket_name, runner);
   }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
   return std::make_unique<DummyProducer>(runner);
 }
 
 const char* MaybeSocket() {
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
   return perfetto::GetProducerSocket();
 #else
   return nullptr;
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 }
 }  // namespace
 
@@ -217,7 +217,7 @@ bool PerfettoTracedProcess::CanStartTracing(
     }
   } else {
     // In tests this is possible due to the periodic polling of CanStartTracing
-    // by the AndroidSystemProducer, when we swap it out for a
+    // by the PosixSystemProducer, when we swap it out for a
     // MockSystemProducer there can be three PerfettoProducers calling this
     // function. In production nothing ever calls the
     // |Set.*ProducerForTesting()| functions so this should never be reached.

services/tracing/public/cpp/perfetto/android_system_producer.cc → services/tracing/public/cpp/perfetto/posix_system_producer.cc

@@ -2,15 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "services/tracing/public/cpp/perfetto/android_system_producer.h"
+#include "services/tracing/public/cpp/perfetto/posix_system_producer.h"
 
 #include <utility>
 
-#include "base/android/build_info.h"
 #include "base/bind.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/trace_event/trace_log.h"
+#include "build/build_config.h"
 #include "services/tracing/public/cpp/perfetto/shared_memory.h"
 #include "services/tracing/public/cpp/traced_process_impl.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/commit_data_request.h"
@@ -22,6 +22,10 @@
 #include "third_party/perfetto/include/perfetto/protozero/scattered_stream_writer.h"
 #include "third_party/perfetto/protos/perfetto/common/track_event_descriptor.pbzero.h"
 
+#if defined(OS_ANDROID)
+#include "base/android/build_info.h"
+#endif  // defined(OS_ANDROID)
+
 namespace tracing {
 namespace {
 constexpr uint32_t kInitialConnectionBackoffMs = 100;
@@ -45,7 +49,7 @@ uint32_t IncreaseBackoff(uint32_t current, uint32_t max) {
 }
 }  // namespace
 
-AndroidSystemProducer::AndroidSystemProducer(const char* socket,
+PosixSystemProducer::PosixSystemProducer(const char* socket,
                                          PerfettoTaskRunner* task_runner)
     : SystemProducer(task_runner),
       socket_name_(socket),
@@ -53,25 +57,25 @@ AndroidSystemProducer::AndroidSystemProducer(const char* socket,
   Connect();
 }
 
-AndroidSystemProducer::~AndroidSystemProducer() {
+PosixSystemProducer::~PosixSystemProducer() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-void AndroidSystemProducer::SetDisallowPreAndroidPieForTesting(bool disallow) {
+void PosixSystemProducer::SetDisallowPreAndroidPieForTesting(bool disallow) {
   disallow_pre_android_pie = disallow;
-  if (!disallow && state_ == State::kUninitialized) {
+  if (!disallow && state_ == State::kDisconnected) {
     // If previously we would not have connected, we now attempt to connect
     // since we are now skipping a check.
     Connect();
   }
 }
 
-void AndroidSystemProducer::SetNewSocketForTesting(const char* socket) {
+void PosixSystemProducer::SetNewSocketForTesting(const char* socket) {
   socket_name_ = socket;
   if (state_ == State::kConnected) {
     // If we are fully connected we need to reset the service before we
     // reconnect.
-    DisconnectWithReply(base::BindOnce(&AndroidSystemProducer::OnDisconnect,
+    DisconnectWithReply(base::BindOnce(&PosixSystemProducer::OnDisconnect,
                                        base::Unretained(this)));
   } else {
     // In any other case we just need to do a normal disconnect and
@@ -81,21 +85,21 @@ void AndroidSystemProducer::SetNewSocketForTesting(const char* socket) {
   }
 }
 
-void AndroidSystemProducer::ResetSequenceForTesting() {
+void PosixSystemProducer::ResetSequenceForTesting() {
   // DETACH the sequence and then immediately attach it. This is needed in tests
   // because we might be executing in a TaskEnvironment, but the global
-  // PerfettoTracedProcess (which contains a pointer to AndroidSystemProducer)
+  // PerfettoTracedProcess (which contains a pointer to PosixSystemProducer)
   // will leak between tests, but the sequence will no longer be valid.
   DETACH_FROM_SEQUENCE(sequence_checker_);
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-bool AndroidSystemProducer::IsTracingActive() {
+bool PosixSystemProducer::IsTracingActive() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return data_sources_tracing_ > 0;
 }
 
-void AndroidSystemProducer::NewDataSourceAdded(
+void PosixSystemProducer::NewDataSourceAdded(
     const PerfettoTracedProcess::DataSourceBase* const data_source) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (state_ != State::kConnected) {
@@ -124,10 +128,10 @@ void AndroidSystemProducer::NewDataSourceAdded(
   std::string track_event_descriptor_raw(raw_proto.begin(), raw_proto.end());
   new_registration.set_track_event_descriptor_raw(track_event_descriptor_raw);
 
-  service_->RegisterDataSource(new_registration);
+  GetSerivce()->RegisterDataSource(new_registration);
 }
 
-void AndroidSystemProducer::DisconnectWithReply(
+void PosixSystemProducer::DisconnectWithReply(
     base::OnceClosure on_disconnect_complete) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (state_ == State::kConnected) {
@@ -142,9 +146,10 @@ void AndroidSystemProducer::DisconnectWithReply(
     // trace in StartDataSource().
     for (const auto* const data_source :
          PerfettoTracedProcess::Get()->data_sources()) {
-      DCHECK(service_.get());
-      service_->UnregisterDataSource(data_source->name());
+      DCHECK(GetSerivce());
+      GetSerivce()->UnregisterDataSource(data_source->name());
     }
+    state_ = State::kUnregistered;
   }
   // If we are tracing we need to wait until we're fully disconnected
   // to run the callback, otherwise we run it immediately (we will
@@ -160,10 +165,13 @@ void AndroidSystemProducer::DisconnectWithReply(
   DelayedReconnect();
 }
 
-void AndroidSystemProducer::OnConnect() {
+void PosixSystemProducer::OnConnect() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!PerfettoTracedProcess::Get()->CanStartTracing(this,
                                                      base::OnceClosure())) {
+    // We are succesfully connected, but we can't register the data sources
+    // right now, so move into "kUnregistered".
+    state_ = State::kUnregistered;
     DisconnectWithReply();
     return;
   }
@@ -175,9 +183,9 @@ void AndroidSystemProducer::OnConnect() {
   }
 }
 
-void AndroidSystemProducer::OnDisconnect() {
+void PosixSystemProducer::OnDisconnect() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(service_.get());
+  DCHECK(GetSerivce());
   // Currently our data sources don't support the concept of the service
   // disappearing and thus can't shut down cleanly (they would attempt to flush
   // data across the broken socket). Add a CHECK to catch this if its a problem.
@@ -187,49 +195,58 @@ void AndroidSystemProducer::OnDisconnect() {
   // This PostTask is needed because we want to clean up the state AFTER the
   // |ProducerEndpoint| has finished cleaning up.
   task_runner()->GetOrCreateTaskRunner()->PostTask(
-      FROM_HERE, base::BindOnce(
-                     [](base::WeakPtr<AndroidSystemProducer> weak_ptr) {
+      FROM_HERE,
+      base::BindOnce(
+          [](base::WeakPtr<PosixSystemProducer> weak_ptr) {
             if (!weak_ptr) {
               return;
             }
-                       weak_ptr->service_.reset();
-                       weak_ptr->shared_memory_arbiter_.reset();
+            if (weak_ptr->state_ == State::kConnecting) {
+              // We never connected, which means this disconnect is
+              // an error from connecting, which means we don't need
+              // to keep this endpoint (and associated memory around
+              // forever) this prevents the memory leak from getting
+              // excessive.
+              weak_ptr->services_.erase(weak_ptr->services_.end() - 1);
+            }
+            weak_ptr->state_ = State::kDisconnected;
             weak_ptr->shared_memory_ = nullptr;
             weak_ptr->DelayedReconnect();
           },
           weak_ptr_factory_.GetWeakPtr()));
 }
 
-void AndroidSystemProducer::OnTracingSetup() {
+void PosixSystemProducer::OnTracingSetup() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // TODO(oysteine): plumb this through the service.
-  const size_t kShmemBufferPageSize = 4096;
   DCHECK(!shared_memory_);
-  DCHECK(!shared_memory_arbiter_);
-  shared_memory_ = service_->shared_memory();
+  shared_memory_ = GetSerivce()->shared_memory();
   DCHECK(shared_memory_);
-  shared_memory_arbiter_ = perfetto::SharedMemoryArbiter::CreateInstance(
-      shared_memory_, kShmemBufferPageSize, this,
-      PerfettoTracedProcess::GetTaskRunner());
 }
 
-void AndroidSystemProducer::SetupDataSource(perfetto::DataSourceInstanceID,
+void PosixSystemProducer::SetupDataSource(perfetto::DataSourceInstanceID,
                                           const perfetto::DataSourceConfig&) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // Always called before StartDataSource but not used for any setup currently.
 }
 
-void AndroidSystemProducer::StartDataSource(
+void PosixSystemProducer::StartDataSource(
     perfetto::DataSourceInstanceID id,
     const perfetto::DataSourceConfig& config) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (state_ != State::kConnected) {
+    // Because StartDataSource is async, its possible a previous StartDataSource
+    // is still in the PostTask queue, so we just ignore it here (We'll get
+    // a new one if/when we re-register the DataSource once we've moved into
+    // kConnected).
+    return;
+  }
 
   for (auto* const data_source : PerfettoTracedProcess::Get()->data_sources()) {
     if (data_source->name() == config.name()) {
       auto can_trace = PerfettoTracedProcess::Get()->CanStartTracing(
           this,
           base::BindOnce(
-              [](base::WeakPtr<AndroidSystemProducer> weak_ptr,
+              [](base::WeakPtr<PosixSystemProducer> weak_ptr,
                  PerfettoTracedProcess::DataSourceBase* data_source,
                  perfetto::DataSourceInstanceID id,
                  const perfetto::DataSourceConfig& data_source_config) {
@@ -241,7 +258,7 @@ void AndroidSystemProducer::StartDataSource(
                 data_source->StartTracingWithID(
                     id, weak_ptr.get(),
                     EnsureGuardRailsAreFollowed(data_source_config));
-                weak_ptr->service_->NotifyDataSourceStarted(id);
+                weak_ptr->GetSerivce()->NotifyDataSourceStarted(id);
               },
               weak_ptr_factory_.GetWeakPtr(), data_source, id, config));
       if (!can_trace) {
@@ -252,19 +269,19 @@ void AndroidSystemProducer::StartDataSource(
   }
 }
 
-void AndroidSystemProducer::StopDataSource(perfetto::DataSourceInstanceID id) {
+void PosixSystemProducer::StopDataSource(perfetto::DataSourceInstanceID id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   for (auto* const data_source : PerfettoTracedProcess::Get()->data_sources()) {
     if (data_source->data_source_id() == id &&
         data_source->producer() == this) {
       data_source->StopTracing(base::BindOnce(
-          [](base::WeakPtr<AndroidSystemProducer> weak_ptr,
+          [](base::WeakPtr<PosixSystemProducer> weak_ptr,
              perfetto::DataSourceInstanceID id) {
             if (!weak_ptr) {
               return;
             }
             DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_);
-            weak_ptr->service_->NotifyDataSourceStopped(id);
+            weak_ptr->GetSerivce()->NotifyDataSourceStopped(id);
             --weak_ptr->data_sources_tracing_;
             if (!weak_ptr->IsTracingActive()) {
               // If this is the last data source to be shut down then
@@ -279,7 +296,7 @@ void AndroidSystemProducer::StopDataSource(perfetto::DataSourceInstanceID id) {
   }
 }
 
-void AndroidSystemProducer::Flush(
+void PosixSystemProducer::Flush(
     perfetto::FlushRequestID id,
     const perfetto::DataSourceInstanceID* data_source_ids,
     size_t num_data_sources) {
@@ -290,7 +307,7 @@ void AndroidSystemProducer::Flush(
                   data_source->data_source_id()) !=
         data_source_ids + num_data_sources) {
       data_source->Flush(base::BindRepeating(
-          [](base::WeakPtr<AndroidSystemProducer> weak_ptr,
+          [](base::WeakPtr<PosixSystemProducer> weak_ptr,
              perfetto::FlushRequestID flush_id) {
             if (weak_ptr) {
               weak_ptr->NotifyFlushComplete(flush_id);
@@ -301,7 +318,7 @@ void AndroidSystemProducer::Flush(
   }
 }
 
-void AndroidSystemProducer::ClearIncrementalState(
+void PosixSystemProducer::ClearIncrementalState(
     const perfetto::DataSourceInstanceID* data_source_ids,
     size_t num_data_sources) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -316,19 +333,19 @@ void AndroidSystemProducer::ClearIncrementalState(
   }
 }
 
-void AndroidSystemProducer::CommitData(
-    const perfetto::CommitDataRequest& commit,
+void PosixSystemProducer::CommitData(const perfetto::CommitDataRequest& commit,
                                      CommitDataCallback callback) {
+  NOTREACHED();
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(service_);
-  service_->CommitData(commit, std::move(callback));
+  DCHECK(GetSerivce());
+  GetSerivce()->CommitData(commit, std::move(callback));
 }
 
-perfetto::SharedMemory* AndroidSystemProducer::shared_memory() const {
+perfetto::SharedMemory* PosixSystemProducer::shared_memory() const {
   return shared_memory_;
 }
 
-void AndroidSystemProducer::NotifyFlushComplete(perfetto::FlushRequestID id) {
+void PosixSystemProducer::NotifyFlushComplete(perfetto::FlushRequestID id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (pending_replies_for_latest_flush_.first != id) {
     // Ignore; completed flush was for an earlier request.
@@ -342,80 +359,81 @@ void AndroidSystemProducer::NotifyFlushComplete(perfetto::FlushRequestID id) {
   }
 }
 
-void AndroidSystemProducer::RegisterTraceWriter(uint32_t writer_id,
+void PosixSystemProducer::RegisterTraceWriter(uint32_t writer_id,
                                               uint32_t target_buffer) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(service_);
-  service_->RegisterTraceWriter(writer_id, target_buffer);
+  // Never called by SharedMemoryArbiter/TraceWriter.
+  NOTREACHED();
 }
 
-void AndroidSystemProducer::UnregisterTraceWriter(uint32_t writer_id) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(service_);
-  service_->UnregisterTraceWriter(writer_id);
+void PosixSystemProducer::UnregisterTraceWriter(uint32_t writer_id) {
+  // Never called by SharedMemoryArbiter/TraceWriter.
+  NOTREACHED();
 }
 
-void AndroidSystemProducer::RegisterDataSource(
+void PosixSystemProducer::RegisterDataSource(
     const perfetto::DataSourceDescriptor&) {
   // Never called by SharedMemoryArbiter/TraceWriter.
   NOTREACHED();
 }
 
-void AndroidSystemProducer::UnregisterDataSource(const std::string& name) {
+void PosixSystemProducer::UnregisterDataSource(const std::string& name) {
   // Never called by SharedMemoryArbiter/TraceWriter.
   NOTREACHED();
 }
 
-void AndroidSystemProducer::NotifyDataSourceStopped(
+void PosixSystemProducer::NotifyDataSourceStopped(
     perfetto::DataSourceInstanceID id) {
   // Never called by SharedMemoryArbiter/TraceWriter.
   NOTREACHED();
 }
 
-void AndroidSystemProducer::NotifyDataSourceStarted(
+void PosixSystemProducer::NotifyDataSourceStarted(
     perfetto::DataSourceInstanceID id) {
   // Never called by SharedMemoryArbiter/TraceWriter.
   NOTREACHED();
 }
 
-size_t AndroidSystemProducer::shared_buffer_page_size_kb() const {
+size_t PosixSystemProducer::shared_buffer_page_size_kb() const {
   // Never called by SharedMemoryArbiter/TraceWriter.
   NOTREACHED();
   return 0;
 }
 
-perfetto::SharedMemoryArbiter*
-AndroidSystemProducer::MaybeSharedMemoryArbiter() {
-  return service_->MaybeSharedMemoryArbiter();
+perfetto::SharedMemoryArbiter* PosixSystemProducer::MaybeSharedMemoryArbiter() {
+  DCHECK(GetSerivce());
+  return GetSerivce()->MaybeSharedMemoryArbiter();
 }
 
-void AndroidSystemProducer::ActivateTriggers(
+void PosixSystemProducer::ActivateTriggers(
     const std::vector<std::string>& triggers) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (state_ == State::kConnected) {
-    service_->ActivateTriggers(triggers);
+    GetSerivce()->ActivateTriggers(triggers);
   }
 }
 
-void AndroidSystemProducer::ConnectSocket() {
+void PosixSystemProducer::ConnectSocket() {
   state_ = State::kConnecting;
-  service_ = perfetto::ProducerIPCClient::Connect(
+  services_.push_back(perfetto::ProducerIPCClient::Connect(
       socket_name_.c_str(), this,
       base::StrCat(
           {mojom::kPerfettoProducerNamePrefix,
            base::NumberToString(
                base::trace_event::TraceLog::GetInstance()->process_id())}),
       task_runner(),
-      perfetto::TracingService::ProducerSMBScrapingMode::kEnabled);
+      perfetto::TracingService::ProducerSMBScrapingMode::kEnabled));
 }
 
-bool AndroidSystemProducer::SkipIfPreAndroidPie() const {
+bool PosixSystemProducer::SkipIfOnAndroidAndPreAndroidPie() const {
+#if defined(OS_ANDROID)
   return disallow_pre_android_pie &&
          base::android::BuildInfo::GetInstance()->sdk_int() <
              base::android::SDK_VERSION_P;
+#endif  // defined(OS_ANDROID)
+  return false;
 }
 
-void AndroidSystemProducer::InvokeStoredOnDisconnectCallbacks() {
+void PosixSystemProducer::InvokeStoredOnDisconnectCallbacks() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   for (auto& callback : on_disconnect_callbacks_) {
     DCHECK(!callback.is_null());
@@ -424,53 +442,49 @@ void AndroidSystemProducer::InvokeStoredOnDisconnectCallbacks() {
   on_disconnect_callbacks_.clear();
 }
 
-void AndroidSystemProducer::Connect() {
+void PosixSystemProducer::Connect() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (SkipIfPreAndroidPie()) {
+  if (SkipIfOnAndroidAndPreAndroidPie()) {
     return;
   }
   switch (state_) {
-    case State::kUninitialized:
+    case State::kDisconnected:
       ConnectSocket();
       break;
     case State::kConnecting:
     case State::kConnected:
-      // We are already connected (in which case do nothing). Or we're currently
-      // connecting the socket and waiting for the OnConnect call from the
-      // service.
+      // We are already connected (in which case do nothing). Or we're
+      // currently connecting the socket and waiting for the OnConnect call
+      // from the service.
       return;
-    case State::kDisconnected:
-      if (service_) {
+    case State::kUnregistered:
+      DCHECK(GetSerivce());
       // We unregistered all our data sources due to a concurrent tracing
       // session but still have an open connection so just reregister
       // everything.
       OnConnect();
-      } else {
-        // We were disconnected by a connection error, so we need to recreate
-        // our service pipe.
-        ConnectSocket();
-      }
       break;
   }
 }
 
-void AndroidSystemProducer::DelayedReconnect() {
+void PosixSystemProducer::DelayedReconnect() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (SkipIfPreAndroidPie()) {
+  if (SkipIfOnAndroidAndPreAndroidPie()) {
     return;
   }
-  if (state_ == State::kDisconnected) {
+  if (retrying_) {
     return;
   }
-  state_ = State::kDisconnected;
+  retrying_ = true;
 
   task_runner()->GetOrCreateTaskRunner()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(
-          [](base::WeakPtr<AndroidSystemProducer> weak_ptr) {
+          [](base::WeakPtr<PosixSystemProducer> weak_ptr) {
             if (!weak_ptr) {
               return;
             }
+            weak_ptr->retrying_ = false;
             if (PerfettoTracedProcess::Get()->CanStartTracing(
                     weak_ptr.get(), base::OnceClosure())) {
               weak_ptr->Connect();
@@ -485,4 +499,15 @@ void AndroidSystemProducer::DelayedReconnect() {
       IncreaseBackoff(connection_backoff_ms_, kMaxConnectionBackoffMs);
 }
 
+perfetto::TracingService::ProducerEndpoint* PosixSystemProducer::GetSerivce() {
+  switch (state_) {
+    case State::kConnecting:
+    case State::kConnected:
+    case State::kUnregistered:
+      return services_.back().get();
+    default:
+      return nullptr;
+  }
+}
+
 }  // namespace tracing

services/tracing/public/cpp/perfetto/android_system_producer.h → services/tracing/public/cpp/perfetto/posix_system_producer.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SERVICES_TRACING_PUBLIC_CPP_PERFETTO_ANDROID_SYSTEM_PRODUCER_H_
-#define SERVICES_TRACING_PUBLIC_CPP_PERFETTO_ANDROID_SYSTEM_PRODUCER_H_
+#ifndef SERVICES_TRACING_PUBLIC_CPP_PERFETTO_POSIX_SYSTEM_PRODUCER_H_
+#define SERVICES_TRACING_PUBLIC_CPP_PERFETTO_POSIX_SYSTEM_PRODUCER_H_
 
 #include <memory>
 #include <set>
@@ -26,19 +26,20 @@ class SharedMemory;
 
 namespace tracing {
 
-class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
+class COMPONENT_EXPORT(TRACING_CPP) PosixSystemProducer
     : public SystemProducer {
  public:
   enum class State {
-    kUninitialized = 0,
+    kDisconnected = 0,
     kConnecting = 1,
-    kConnected = 2,
-    kDisconnected = 3
+    // Connected but all data sources unregistered.
+    kUnregistered = 2,
+    kConnected = 3,
   };
-  AndroidSystemProducer(const char* socket, PerfettoTaskRunner* task_runner);
-  ~AndroidSystemProducer() override;
+  PosixSystemProducer(const char* socket, PerfettoTaskRunner* task_runner);
+  ~PosixSystemProducer() override;
 
-  // Functions needed for AndroidSystemProducer only.
+  // Functions needed for PosixSystemProducer only.
   //
   // Lets tests ignore the SDK check (Perfetto only runs on post Android Pie
   // devices by default, so for trybots on older OSs we need to ignore the check
@@ -47,7 +48,7 @@ class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
   // TODO(nuskos): We need to make this possible for telemetry as well, since
   // they might have side loaded the app.
   void SetDisallowPreAndroidPieForTesting(bool disallow);
-  // |socket| must remain alive as long as AndroidSystemProducer is around
+  // |socket| must remain alive as long as PosixSystemProducer is around
   // trying to connect to it.
   void SetNewSocketForTesting(const char* socket);
 
@@ -127,7 +128,7 @@ class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
   void ConnectSocket();
   // Returns true if we should skip setup because this Android device is Android
   // O or below.
-  bool SkipIfPreAndroidPie() const;
+  bool SkipIfOnAndroidAndPreAndroidPie() const;
   // If any OnDisconnect callbacks are stored, this will invoke them and delete
   // references to them must be called on the proper sequence.
   void InvokeStoredOnDisconnectCallbacks();
@@ -136,17 +137,27 @@ class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
   // later.
   void DelayedReconnect();
 
+  perfetto::TracingService::ProducerEndpoint* GetSerivce();
+
+  bool retrying_ = false;
   std::string socket_name_;
   uint32_t connection_backoff_ms_;
   uint64_t data_sources_tracing_ = 0;
   bool disallow_pre_android_pie = true;
-  State state_ = State::kUninitialized;
+  State state_ = State::kDisconnected;
   std::vector<base::OnceClosure> on_disconnect_callbacks_;
 
   // Connection to the Perfetto service and the shared memory that it provides.
   perfetto::SharedMemory* shared_memory_ = nullptr;
-  std::unique_ptr<perfetto::SharedMemoryArbiter> shared_memory_arbiter_;
-  std::unique_ptr<perfetto::TracingService::ProducerEndpoint> service_;
+  // ProducerEndpoints must outlive all trace writers, but some trace writers
+  // will never flush until future tracing sessions, which means even on
+  // disconnecting we have to keep these around. So instead of destroying any
+  // Endpoints (which hold the SharedMemory and SharedMemoryArbiters) we store
+  // them forever (leaking their amount of memory).
+  //
+  // TODO(nuskos): We should improve this once we're on the client library.
+  std::vector<std::unique_ptr<perfetto::TracingService::ProducerEndpoint>>
+      services_;
   // First value is the flush ID, the second is the number of
   // replies we're still waiting for.
   std::pair<uint64_t, size_t> pending_replies_for_latest_flush_;
@@ -154,10 +165,10 @@ class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
   SEQUENCE_CHECKER(sequence_checker_);
   // NOTE: Weak pointers must be invalidated before all other member variables.
   // and thus must be the last member variable.
-  base::WeakPtrFactory<AndroidSystemProducer> weak_ptr_factory_{this};
-  DISALLOW_COPY_AND_ASSIGN(AndroidSystemProducer);
+  base::WeakPtrFactory<PosixSystemProducer> weak_ptr_factory_{this};
+  DISALLOW_COPY_AND_ASSIGN(PosixSystemProducer);
 };
 
 }  // namespace tracing
 
-#endif  // SERVICES_TRACING_PUBLIC_CPP_PERFETTO_ANDROID_SYSTEM_PRODUCER_H_
+#endif  // SERVICES_TRACING_PUBLIC_CPP_PERFETTO_POSIX_SYSTEM_PRODUCER_H_

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

@@ -27,9 +27,9 @@ PerfettoTaskRunner::PerfettoTaskRunner(
 
 PerfettoTaskRunner::~PerfettoTaskRunner() {
   DCHECK(GetOrCreateTaskRunner()->RunsTasksInCurrentSequence());
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
   fd_controllers_.clear();
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 }
 
 void PerfettoTaskRunner::PostTask(std::function<void()> task) {
@@ -79,26 +79,49 @@ bool PerfettoTaskRunner::RunsTasksOnCurrentThread() const {
 void PerfettoTaskRunner::AddFileDescriptorWatch(
     int fd,
     std::function<void()> callback) {
-#if !defined(OS_ANDROID)
+#if !defined(OS_POSIX)
   NOTREACHED();
 #else
   DCHECK(GetOrCreateTaskRunner()->RunsTasksInCurrentSequence());
   DCHECK(!base::Contains(fd_controllers_, fd));
-  fd_controllers_[fd] = base::FileDescriptorWatcher::WatchReadable(
-      fd,
-      base::BindRepeating([](std::function<void()> callback) { callback(); },
+  // Set it up as a nullptr to signal intent to add a watch. We need to PostTask
+  // the WatchReadable creation because if we do it in this task we'll race with
+  // perfetto setting up the connection on this task and the IO thread setting
+  // up epoll on the |fd|. By posting the task we ensure the Connection has
+  // either succeeded (we find the |fd| in the map) or the connection failed
+  // (the |fd| is not in the map), and we can gracefully handle either case.
+  fd_controllers_[fd];
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](PerfettoTaskRunner* perfetto_runner, int fd,
+             std::function<void()> callback) {
+            DCHECK(perfetto_runner->GetOrCreateTaskRunner()
+                       ->RunsTasksInCurrentSequence());
+            auto it = perfetto_runner->fd_controllers_.find(fd);
+            // If we can't find this fd, then RemoveFileDescriptor has already
+            // been called so just early out.
+            if (it == perfetto_runner->fd_controllers_.end()) {
+              return;
+            }
+            DCHECK(!it->second);
+            it->second = base::FileDescriptorWatcher::WatchReadable(
+                fd, base::BindRepeating(
+                        [](std::function<void()> callback) { callback(); },
                         std::move(callback)));
-#endif  // !defined(OS_ANDROID)
+          },
+          base::Unretained(this), fd, std::move(callback)));
+#endif  // !defined(OS_POSIX)
 }
 
 void PerfettoTaskRunner::RemoveFileDescriptorWatch(int fd) {
-#if !defined(OS_ANDROID)
+#if !defined(OS_POSIX)
   NOTREACHED();
 #else
   DCHECK(GetOrCreateTaskRunner()->RunsTasksInCurrentSequence());
   DCHECK(base::Contains(fd_controllers_, fd));
   fd_controllers_.erase(fd);
-#endif  // !defined(OS_ANDROID)
+#endif  // !defined(OS_POSIX)
 }
 
 void PerfettoTaskRunner::ResetTaskRunnerForTesting(

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

@@ -16,12 +16,12 @@
 #include "services/tracing/public/mojom/perfetto_service.mojom.h"
 #include "third_party/perfetto/include/perfetto/base/task_runner.h"
 
-#if defined(OS_ANDROID)
-#include <map>
+#if defined(OS_POSIX)
 // Needed for base::FileDescriptorWatcher::Controller and for implementing
-// AddFileDescriptorWatch & RemoveFileDescriptorWatch on Android.
+// AddFileDescriptorWatch & RemoveFileDescriptorWatch.
+#include <map>
 #include "base/files/file_descriptor_watcher_posix.h"
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 
 namespace tracing {
 
@@ -62,10 +62,10 @@ class COMPONENT_EXPORT(TRACING_CPP) PerfettoTaskRunner
   void OnDeferredTasksDrainTimer();
 
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
-#if defined(OS_ANDROID)
+#if defined(OS_POSIX)
   std::map<int, std::unique_ptr<base::FileDescriptorWatcher::Controller>>
       fd_controllers_;
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_POSIX)
 
   DISALLOW_COPY_AND_ASSIGN(PerfettoTaskRunner);
 };