tracing: Disable startup tracing for child processes for system traces

The system tracing producer doesn't currently support startup tracing
in the child processes. If we pass on startup tracing flags, the
ProducerClient will enable itself in spawned child processes, and thus
prevent the SystemProducer to enable tracing when instructed by the
service later. Until the SystemProducer supports startup tracing, we
shouldn't try to enable it for the child processes.

Change-Id: I6d42b2d705f60d935bd104fe66bb159793bebda2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116054Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753600}

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

@@ -100,7 +100,7 @@ bool PosixSystemProducer::SetupStartupTracing() {
 }
 
 perfetto::SharedMemoryArbiter* PosixSystemProducer::MaybeSharedMemoryArbiter() {
-  base::AutoLock lock(services_lock_);
+  base::AutoLock lock(lock_);
   DCHECK(GetService());
   return GetService()->MaybeSharedMemoryArbiter();
 }
@@ -134,7 +134,7 @@ void PosixSystemProducer::NewDataSourceAdded(
 }
 
 bool PosixSystemProducer::IsTracingActive() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  base::AutoLock lock(lock_);
   return data_sources_tracing_ > 0;
 }
 
@@ -233,7 +233,7 @@ void PosixSystemProducer::OnDisconnect() {
               return;
             }
             if (weak_ptr->state_ == State::kConnecting) {
-              base::AutoLock lock(weak_ptr->services_lock_);
+              base::AutoLock lock(weak_ptr->lock_);
               // 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
@@ -284,7 +284,10 @@ void PosixSystemProducer::StartDataSource(
                   return;
                 }
                 DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_);
-                ++weak_ptr->data_sources_tracing_;
+                {
+                  base::AutoLock lock(weak_ptr->lock_);
+                  ++weak_ptr->data_sources_tracing_;
+                }
                 data_source->StartTracingWithID(
                     id, weak_ptr.get(),
                     EnsureGuardRailsAreFollowed(data_source_config));
@@ -312,7 +315,10 @@ void PosixSystemProducer::StopDataSource(perfetto::DataSourceInstanceID id) {
             }
             DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_);
             weak_ptr->GetService()->NotifyDataSourceStopped(id);
-            --weak_ptr->data_sources_tracing_;
+            {
+              base::AutoLock lock(weak_ptr->lock_);
+              --weak_ptr->data_sources_tracing_;
+            }
             if (!weak_ptr->IsTracingActive()) {
               // If this is the last data source to be shut down then
               // perhaps we need to invoke any callbacks that were stored
@@ -375,7 +381,7 @@ void PosixSystemProducer::ConnectSocket() {
       task_runner(),
       perfetto::TracingService::ProducerSMBScrapingMode::kEnabled);
 
-  base::AutoLock lock(services_lock_);
+  base::AutoLock lock(lock_);
   services_.push_back(std::move(service));
 }
 
@@ -473,7 +479,7 @@ perfetto::TracingService::ProducerEndpoint* PosixSystemProducer::GetService() {
 #if DCHECK_IS_ON()
   // Requires lock to be held when called on a non-producer sequence/thread.
   if (!sequence_checker_.CalledOnValidSequence()) {
-    services_lock_.AssertAcquired();
+    lock_.AssertAcquired();
   }
 #endif  // DCHECK_IS_ON()
 

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

@@ -124,26 +124,31 @@ class COMPONENT_EXPORT(TRACING_CPP) PosixSystemProducer
   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::kDisconnected;
   std::vector<base::OnceClosure> on_disconnect_callbacks_;
+  // 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_;
+
+  // -- Begin lock-protected members. --
+  base::Lock lock_;
 
-  // |services_| is accessed by MaybeSharedMemoryArbiter() on any thread. This
-  // access and any modifications to |services_| are protected by this lock.
-  base::Lock services_lock_;
   // 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).
   //
+  // |services_| is accessed by MaybeSharedMemoryArbiter() on any thread. This
+  // access and any modifications to |services_| are protected by the |lock_|.
+  //
   // 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_;
+
+  uint64_t data_sources_tracing_ = 0;
+  // -- End lock-protected members. --
 
   SEQUENCE_CHECKER(sequence_checker_);
   // NOTE: Weak pointers must be invalidated before all other member variables.

services/tracing/public/cpp/trace_startup.cc

@@ -170,6 +170,12 @@ void PropagateTracingFlagsToChildProcessCmdLine(base::CommandLine* cmd_line) {
   if (!TraceEventDataSource::GetInstance()->IsEnabled())
     return;
 
+  // (Posix)SystemProducer doesn't currently support startup tracing, so don't
+  // attempt to enable startup tracing in child processes if system tracing is
+  // active.
+  if (PerfettoTracedProcess::Get()->system_producer()->IsTracingActive())
+    return;
+
   // The child process startup may race with a concurrent disabling of the
   // tracing session by the tracing service. To avoid being stuck in startup
   // tracing mode forever, the child process will discard the startup session