services/tracing: Clean up session priority setting

This replaces the previous mojo interface parameter for specifying the
priority of a tracing session with an equivalent setting in the Perfetto
trace config. This patch doesn't change the effective priority of existing
tracing sessions; only the redundant parallel setting is removed.

Bug: 1058632
Change-Id: I0f71edcf65c564df286191be4634555a09f93473
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414253
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814014}

components/ui_devtools/tracing_agent.cc

@@ -114,8 +114,7 @@ class TracingAgent::PerfettoTracingSession
     on_recording_enabled_callback_ = std::move(on_recording_enabled_callback);
     consumer_host_->EnableTracing(
         tracing_session_host_.BindNewPipeAndPassReceiver(),
-        std::move(tracing_session_client), std::move(perfetto_config),
-        tracing::mojom::TracingClientPriority::kUserInitiated);
+        std::move(tracing_session_client), std::move(perfetto_config));
 
     tracing_session_host_.set_disconnect_handler(
         base::BindOnce(&PerfettoTracingSession::OnTracingSessionFailed,
@@ -201,7 +200,11 @@ class TracingAgent::PerfettoTracingSession
     last_config_for_perfetto_ = std::move(processfilter_stripped_config);
 #endif
 
-    return tracing::GetDefaultPerfettoConfig(chrome_config);
+    return tracing::GetDefaultPerfettoConfig(
+        chrome_config,
+        /*privacy_filtering_enabled=*/false,
+        /*convert_to_legacy_json=*/false,
+        perfetto::protos::gen::ChromeConfig::USER_INITIATED);
   }
 
   void OnTracingSessionFailed() {

content/browser/devtools/protocol/tracing_handler.cc

@@ -271,8 +271,7 @@ class TracingHandler::PerfettoTracingSession
     on_recording_enabled_callback_ = std::move(on_recording_enabled_callback);
     consumer_host_->EnableTracing(
         tracing_session_host_.BindNewPipeAndPassReceiver(),
-        receiver_.BindNewPipeAndPassRemote(), std::move(perfetto_config),
-        tracing::mojom::TracingClientPriority::kUserInitiated);
+        receiver_.BindNewPipeAndPassRemote(), std::move(perfetto_config));
 
     receiver_.set_disconnect_handler(
         base::BindOnce(&PerfettoTracingSession::OnTracingSessionFailed,
@@ -416,7 +415,11 @@ class TracingHandler::PerfettoTracingSession
     last_config_for_perfetto_ = std::move(processfilter_stripped_config);
 #endif
 
-    return tracing::GetDefaultPerfettoConfig(chrome_config);
+    return tracing::GetDefaultPerfettoConfig(
+        chrome_config,
+        /*privacy_filtering_enabled=*/false,
+        /*convert_to_legacy_json=*/false,
+        perfetto::protos::gen::ChromeConfig::USER_INITIATED);
   }
 
   void OnStartupTracingEnabled() {

content/browser/tracing/perfetto_file_tracer.cc

@@ -107,8 +107,10 @@ PerfettoFileTracer::PerfettoFileTracer()
   // The output is always proto based. So, enable privacy filtering if argument
   // filter is enabled.
   bool privacy_filtering = chrome_config.IsArgumentFilterEnabled();
-  perfetto::TraceConfig trace_config =
-      tracing::GetDefaultPerfettoConfig(chrome_config, privacy_filtering);
+  perfetto::TraceConfig trace_config = tracing::GetDefaultPerfettoConfig(
+      chrome_config, privacy_filtering,
+      /*convert_to_legacy_json=*/false,
+      perfetto::protos::gen::ChromeConfig::USER_INITIATED);
 
   int duration_in_seconds =
       tracing::TraceStartupConfig::GetInstance()->GetStartupDuration();
@@ -119,8 +121,7 @@ PerfettoFileTracer::PerfettoFileTracer()
 
   consumer_host_->EnableTracing(
       tracing_session_host_.BindNewPipeAndPassReceiver(),
-      receiver_.BindNewPipeAndPassRemote(), std::move(trace_config),
-      tracing::mojom::TracingClientPriority::kBackground);
+      receiver_.BindNewPipeAndPassRemote(), std::move(trace_config));
   receiver_.set_disconnect_handler(base::BindOnce(
       &PerfettoFileTracer::OnTracingSessionEnded, base::Unretained(this)));
   ReadBuffers();

content/browser/tracing/tracing_controller_impl.cc

@@ -403,12 +403,14 @@ bool TracingControllerImpl::StartTracing(
   ConnectToServiceIfNeeded();
 
   perfetto::TraceConfig perfetto_config = tracing::GetDefaultPerfettoConfig(
-      trace_config, /*requires_anonymized_data=*/false);
+      trace_config,
+      /*privacy_filtering_enabled=*/false,
+      /*convert_to_legacy_json=*/true,
+      perfetto::protos::gen::ChromeConfig::USER_INITIATED);
 
   consumer_host_->EnableTracing(
       tracing_session_host_.BindNewPipeAndPassReceiver(),
-      receiver_.BindNewPipeAndPassRemote(), std::move(perfetto_config),
-      tracing::mojom::TracingClientPriority::kUserInitiated);
+      receiver_.BindNewPipeAndPassRemote(), std::move(perfetto_config));
   receiver_.set_disconnect_handler(base::BindOnce(
       &TracingControllerImpl::OnTracingFailed, base::Unretained(this)));
   tracing_session_host_.set_disconnect_handler(base::BindOnce(

services/tracing/perfetto/consumer_host.cc

@@ -625,11 +625,31 @@ ConsumerHost::~ConsumerHost() {
 void ConsumerHost::EnableTracing(
     mojo::PendingReceiver<mojom::TracingSessionHost> tracing_session_host,
     mojo::PendingRemote<mojom::TracingSessionClient> tracing_session_client,
-    const perfetto::TraceConfig& trace_config,
-    mojom::TracingClientPriority priority) {
+    const perfetto::TraceConfig& trace_config) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!tracing_session_);
 
+  auto priority = mojom::TracingClientPriority::kUnknown;
+  for (const auto& data_source : trace_config.data_sources()) {
+    if (!data_source.has_config() ||
+        !data_source.config().has_chrome_config()) {
+      continue;
+    }
+    switch (data_source.config().chrome_config().client_priority()) {
+      case perfetto::protos::gen::ChromeConfig::BACKGROUND:
+        priority =
+            std::max(priority, mojom::TracingClientPriority::kBackground);
+        break;
+      case perfetto::protos::gen::ChromeConfig::USER_INITIATED:
+        priority =
+            std::max(priority, mojom::TracingClientPriority::kUserInitiated);
+        break;
+      default:
+      case perfetto::protos::gen::ChromeConfig::UNKNOWN:
+        break;
+    }
+  }
+
   // We create our new TracingSession async, if the PerfettoService allows
   // us to, after it's stopped any currently running lower or equal priority
   // tracing sessions.

services/tracing/perfetto/consumer_host.h

@@ -138,8 +138,7 @@ class ConsumerHost : public perfetto::Consumer, public mojom::ConsumerHost {
   void EnableTracing(
       mojo::PendingReceiver<mojom::TracingSessionHost> tracing_session_host,
       mojo::PendingRemote<mojom::TracingSessionClient> tracing_session_client,
-      const perfetto::TraceConfig& config,
-      mojom::TracingClientPriority priority) override;
+      const perfetto::TraceConfig& config) override;
 
   // perfetto::Consumer implementation.
   // This gets called by the Perfetto service as control signals,

services/tracing/perfetto/consumer_host_unittest.cc

@@ -154,8 +154,7 @@ class ThreadedPerfettoService : public mojom::TracingSessionClient {
 
     consumer_->EnableTracing(
         tracing_session_host_->BindNewPipeAndPassReceiver(),
-        std::move(tracing_session_client), std::move(config),
-        tracing::mojom::TracingClientPriority::kUserInitiated);
+        std::move(tracing_session_client), std::move(config));
   }
 
   void ReadBuffers(mojo::ScopedDataPipeProducerHandle stream,
@@ -379,7 +378,9 @@ class TracingConsumerTest : public testing::Test,
   }
 
   perfetto::TraceConfig GetDefaultTraceConfig(
-      const std::string& data_source_name) {
+      const std::string& data_source_name,
+      perfetto::protos::gen::ChromeConfig::ClientPriority priority =
+          perfetto::protos::gen::ChromeConfig::UNKNOWN) {
     perfetto::TraceConfig trace_config;
     trace_config.add_buffers()->set_size_kb(32 * 1024);
 
@@ -387,6 +388,7 @@ class TracingConsumerTest : public testing::Test,
         trace_config.add_data_sources()->mutable_config();
     trace_event_config->set_name(data_source_name);
     trace_event_config->set_target_buffer(0);
+    trace_event_config->mutable_chrome_config()->set_client_priority(priority);
 
     return trace_config;
   }
@@ -715,8 +717,7 @@ class MockConsumerHost : public mojom::TracingSessionClient {
   explicit MockConsumerHost(PerfettoService* service)
       : consumer_host_(std::make_unique<ConsumerHost>(service)) {}
 
-  void EnableTracing(const perfetto::TraceConfig& config,
-                     mojom::TracingClientPriority priority) {
+  void EnableTracing(const perfetto::TraceConfig& config) {
     mojo::PendingRemote<tracing::mojom::TracingSessionClient>
         tracing_session_client;
     receiver_.Bind(tracing_session_client.InitWithNewPipeAndPassReceiver());
@@ -726,7 +727,7 @@ class MockConsumerHost : public mojom::TracingSessionClient {
 
     consumer_host_->EnableTracing(
         tracing_session_host_.BindNewPipeAndPassReceiver(),
-        std::move(tracing_session_client), config, priority);
+        std::move(tracing_session_client), config);
     tracing_session_host_.set_disconnect_handler(base::BindOnce(
         &MockConsumerHost::OnConnectionLost, base::Unretained(this)));
   }
@@ -765,34 +766,35 @@ class MockConsumerHost : public mojom::TracingSessionClient {
 
 TEST_F(TracingConsumerTest, TestConsumerPriority) {
   PerfettoService::GetInstance()->SetActiveServicePidsInitialized();
-  auto trace_config = GetDefaultTraceConfig(mojom::kTraceEventDataSourceName);
+  auto trace_config_background =
+      GetDefaultTraceConfig(mojom::kTraceEventDataSourceName,
+                            perfetto::protos::gen ::ChromeConfig::BACKGROUND);
+  auto trace_config_user_initiated = GetDefaultTraceConfig(
+      mojom::kTraceEventDataSourceName,
+      perfetto::protos::gen ::ChromeConfig::USER_INITIATED);
 
   MockConsumerHost background_consumer_1(PerfettoService::GetInstance());
-  background_consumer_1.EnableTracing(
-      trace_config, tracing::mojom::TracingClientPriority::kBackground);
+  background_consumer_1.EnableTracing(trace_config_background);
   background_consumer_1.WaitForTracingEnabled();
 
   // Second consumer of the same priority should cause the first one to
   // be disabled and the second to start.
   MockConsumerHost background_consumer_2(PerfettoService::GetInstance());
-  background_consumer_2.EnableTracing(
-      trace_config, tracing::mojom::TracingClientPriority::kBackground);
+  background_consumer_2.EnableTracing(trace_config_background);
   background_consumer_1.WaitForTracingDisabled();
   background_consumer_2.WaitForTracingEnabled();
 
   // Third consumer will have a higher priority, and should kill the second
   // one.
   MockConsumerHost user_initiated_consumer(PerfettoService::GetInstance());
-  user_initiated_consumer.EnableTracing(
-      trace_config, tracing::mojom::TracingClientPriority::kUserInitiated);
+  user_initiated_consumer.EnableTracing(trace_config_user_initiated);
   background_consumer_2.WaitForTracingDisabled();
   user_initiated_consumer.WaitForTracingEnabled();
 
   // Fourth consumer will be another background consumer, and should be
   // itself killed as the third consumer is still running.
   MockConsumerHost background_consumer_3(PerfettoService::GetInstance());
-  background_consumer_3.EnableTracing(
-      trace_config, tracing::mojom::TracingClientPriority::kBackground);
+  background_consumer_3.EnableTracing(trace_config_background);
   background_consumer_3.WaitForConnectionLost();
 
   // If we close the user initiated consumer, the third background consumer
@@ -800,8 +802,7 @@ TEST_F(TracingConsumerTest, TestConsumerPriority) {
   user_initiated_consumer.DisableTracing();
   user_initiated_consumer.WaitForTracingDisabled();
   user_initiated_consumer.CloseTracingSession();
-  background_consumer_3.EnableTracing(
-      trace_config, tracing::mojom::TracingClientPriority::kBackground);
+  background_consumer_3.EnableTracing(trace_config_background);
   background_consumer_3.WaitForTracingEnabled();
 }
 

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

@@ -378,30 +378,9 @@ class ConsumerEndpoint : public perfetto::ConsumerEndpoint,
 
   void StartTracing() override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-    auto priority = mojom::TracingClientPriority::kUnknown;
-    for (const auto& data_source : trace_config_.data_sources()) {
-      if (!data_source.has_config() ||
-          !data_source.config().has_chrome_config()) {
-        continue;
-      }
-      switch (data_source.config().chrome_config().client_priority()) {
-        case perfetto::protos::gen::ChromeConfig::BACKGROUND:
-          priority =
-              std::max(priority, mojom::TracingClientPriority::kBackground);
-          break;
-        case perfetto::protos::gen::ChromeConfig::USER_INITIATED:
-          priority =
-              std::max(priority, mojom::TracingClientPriority::kUserInitiated);
-          break;
-        default:
-        case perfetto::protos::gen::ChromeConfig::UNKNOWN:
-          break;
-      }
-    }
     consumer_host_->EnableTracing(
         tracing_session_host_.BindNewPipeAndPassReceiver(),
-        tracing_session_client_.BindNewPipeAndPassRemote(), trace_config_,
-        priority);
+        tracing_session_client_.BindNewPipeAndPassRemote(), trace_config_);
     tracing_session_host_.set_disconnect_handler(base::BindOnce(
         &ConsumerEndpoint::OnTracingFailed, base::Unretained(this)));
     tracing_session_client_.set_disconnect_handler(base::BindOnce(

services/tracing/public/mojom/chrome_config_mojom_traits.cc

@@ -19,6 +19,22 @@ bool StructTraits<
   out->set_trace_config(std::move(config));
   out->set_privacy_filtering_enabled(data.privacy_filtering_enabled());
   out->set_convert_to_legacy_json(data.convert_to_legacy_json());
+  switch (data.client_priority()) {
+    case tracing::mojom::TracingClientPriority::kBackground:
+      out->set_client_priority(perfetto::protos::gen::ChromeConfig::BACKGROUND);
+      break;
+    case tracing::mojom::TracingClientPriority::kUserInitiated:
+      out->set_client_priority(
+          perfetto::protos::gen::ChromeConfig::USER_INITIATED);
+      break;
+    case tracing::mojom::TracingClientPriority::kUnknown:
+      out->set_client_priority(perfetto::protos::gen::ChromeConfig::UNKNOWN);
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+
   return true;
 }
 }  // namespace mojo

services/tracing/public/mojom/chrome_config_mojom_traits.h

@@ -31,6 +31,21 @@ class StructTraits<tracing::mojom::ChromeConfigDataView,
     return src.convert_to_legacy_json();
   }
 
+  static tracing::mojom::TracingClientPriority client_priority(
+      const perfetto::ChromeConfig& src) {
+    switch (src.client_priority()) {
+      case perfetto::protos::gen::ChromeConfig::BACKGROUND:
+        return tracing::mojom::TracingClientPriority::kBackground;
+      case perfetto::protos::gen::ChromeConfig::USER_INITIATED:
+        return tracing::mojom::TracingClientPriority::kUserInitiated;
+      case perfetto::protos::gen::ChromeConfig::UNKNOWN:
+        return tracing::mojom::TracingClientPriority::kUnknown;
+      default:
+        NOTREACHED();
+        return tracing::mojom::TracingClientPriority::kUnknown;
+    }
+  }
+
   static bool Read(tracing::mojom::ChromeConfigDataView data,
                    perfetto::ChromeConfig* out);
 };

services/tracing/public/mojom/perfetto_service.mojom

@@ -90,6 +90,9 @@ struct ChromeConfig {
   // Whether the final tracing result will be converted to the legacy JSON
   // format.
   bool convert_to_legacy_json;
+
+  // Priority of the tracing session.
+  TracingClientPriority client_priority;
 };
 
 struct DataSourceConfig {
@@ -233,13 +236,13 @@ interface ConsumerHost {
   // Note: Right now only a single concurrent tracing session is supported,
   // as there's no support for multiplexing enabled trace events to multiple
   // consumers. If a new tracing session is attempted while there's an existing
-  // one in progress, the relative priorities will be used to figure out which
-  // one to be able to (keep) tracing; if the priorities are the same, the new
-  // session will take precedence.
+  // one in progress, the relative priorities (configured through
+  // TracingClientPriority in the trace config) will be used to figure out
+  // which one to be able to (keep) tracing; if the priorities are the same,
+  // the new session will take precedence.
   EnableTracing(pending_receiver<TracingSessionHost> tracing_session_host,
                 pending_remote<TracingSessionClient> tracing_session_client,
-                TraceConfig config,
-                TracingClientPriority priority);
+                TraceConfig config);
 };
 
 // Represents the host side of an active tracing session. Closing this

services/tracing/tracing_service_unittest.cc

@@ -145,8 +145,7 @@ class TestTracingClient : public mojom::TracingSessionClient {
 
     consumer_host_->EnableTracing(
         tracing_session_host_.BindNewPipeAndPassReceiver(),
-        receiver_.BindNewPipeAndPassRemote(), std::move(perfetto_config),
-        tracing::mojom::TracingClientPriority::kUserInitiated);
+        receiver_.BindNewPipeAndPassRemote(), std::move(perfetto_config));
 
     tracing_session_host_->RequestBufferUsage(
         base::BindOnce([](base::OnceClosure on_response, bool, float,