Enable system perfetto producer on all posix builds.

It remains disabled by default by the tracing flag
--enable-features=kEnablePerfettoSystemTracing, but when enabled will
attempt to connect to the result of GetProducerSocket().

This will be useful for people who want system wide tracing on linux
from crashed browser processes since the tracing service will be alive
even after Chrome dies.

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

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

@@ -1313,7 +1313,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",
@@ -1337,6 +1336,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
@@ -2060,6 +2062,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" ]
   }
 }
 
@@ -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);
@@ -629,7 +587,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 +685,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 +722,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 +776,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 +788,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

@@ -117,12 +117,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" ]
     }
 
     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.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,7 +26,7 @@ class SharedMemory;
 
 namespace tracing {
 
-class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
+class COMPONENT_EXPORT(TRACING_CPP) PosixSystemProducer
     : public SystemProducer {
  public:
   enum class State {
@@ -35,10 +35,10 @@ class COMPONENT_EXPORT(TRACING_CPP) AndroidSystemProducer
     kConnected = 2,
     kDisconnected = 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 +47,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);
 
@@ -128,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();
@@ -155,10 +155,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,43 @@ 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.
+  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;
+            }
+            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);
 };