From 5902c74a22c14a8c0b0d173575f39c1118b92ffa Mon Sep 17 00:00:00 2001
From: Danil Somsikov <dsv@chromium.org>
Date: Wed, 29 Apr 2026 05:08:52 -0700
Subject: [PATCH] Apply incremental process ID filters to Chrome data sources in Perfetto trace config.

Previously, process ID filtering was only applied to the "track_event"
data source, and it overwrote existing filters. This caused issues with
incremental process discovery (e.g., in auction tests) where core
browser processes were dropped from the trace when a new renderer PID
was appended.

This change: 1. Extends PID filtering to all Chrome-related data sources
(e.g., "org.chromium.*" and "track_event"). 2. Modifies
AddPidsToProcessFilter to append new PIDs to the existing filter list
instead of clearing it, ensuring all discovered processes are tracked.
3. Clears existing filters only when a broad regex filter is present,
ensuring security boundaries are established. 4. Adds a unit test to
verify that multiple PIDs are correctly accumulated in the trace config
over time.

Bug: 501528031
Change-Id: Ia40649fd3b41ba9d9d274eeabbcc7444705558a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7790893
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1622366}
---

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 50b1f9935..3b80e4c 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -6,6 +6,7 @@
 
 #include <cstddef>
 #include <memory>
+#include <optional>
 #include <string_view>
 #include <utility>
 
@@ -3886,6 +3887,88 @@
   WaitForNotification("Tracing.tracingComplete", true);
 }
 
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, TracingPerfettoSiteIsolation) {
+  content::SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL url_a = embedded_test_server()->GetURL("a.com", "/title1.html");
+  GURL url_b = embedded_test_server()->GetURL("b.com", "/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), url_a, 1);
+  Shell* shell_b = CreateBrowser();
+  NavigateToURLBlockUntilNavigationsComplete(shell_b, url_b, 1);
+
+  Attach();
+
+  base::trace_event::TraceConfig chrome_config("blink.user_timing", "");
+  perfetto::TraceConfig perfetto_config =
+      tracing::GetDefaultPerfettoConfig(chrome_config,
+                                        /*privacy_filtering_enabled=*/false,
+                                        /*convert_to_legacy_json=*/true);
+
+  std::optional<perfetto::DataSourceConfig> track_event_config;
+  for (auto& ds : *perfetto_config.mutable_data_sources()) {
+    if (ds.config().name() == "track_event") {
+      ds.add_producer_name_regex_filter(".*");
+      track_event_config = ds.config();
+    }
+  }
+
+  if (track_event_config) {
+    auto* ds2 = perfetto_config.add_data_sources();
+    *ds2->mutable_config() = *track_event_config;
+    ds2->add_producer_name_regex_filter(".*");
+  }
+
+  auto* ds3 = perfetto_config.add_data_sources();
+  ds3->mutable_config()->set_name("org.chromium.sampler_profiler");
+
+  std::string perfetto_config_encoded =
+      base::Base64Encode(perfetto_config.SerializeAsString());
+
+  base::DictValue params;
+  params.Set("perfettoConfig", perfetto_config_encoded);
+  params.Set("transferMode", "ReturnAsStream");
+
+  EXPECT_TRUE(SendCommandSync("Tracing.start", std::move(params)));
+
+  EXPECT_TRUE(content::ExecJs(shell(), "performance.mark('mark_a');"));
+  EXPECT_TRUE(content::ExecJs(shell_b, "performance.mark('mark_b');"));
+
+  EXPECT_TRUE(SendCommandSync("Tracing.end"));
+
+  base::DictValue complete_notification =
+      WaitForNotification("Tracing.tracingComplete", true);
+  const std::string* stream_handle_ptr =
+      complete_notification.FindString("stream");
+  ASSERT_TRUE(stream_handle_ptr);
+  std::string stream_handle = *stream_handle_ptr;
+
+  std::string trace_json;
+  bool eof = false;
+  while (!eof) {
+    base::DictValue read_params;
+    read_params.Set("handle", stream_handle);
+    const base::DictValue* response =
+        SendCommandSync("IO.read", std::move(read_params));
+    ASSERT_TRUE(response);
+    const std::string* data = response->FindString("data");
+    ASSERT_TRUE(data);
+    std::optional<bool> base64_encoded = response->FindBool("base64Encoded");
+    if (base64_encoded.value_or(false)) {
+      std::string decoded;
+      ASSERT_TRUE(base::Base64Decode(*data, &decoded));
+      trace_json += decoded;
+    } else {
+      trace_json += *data;
+    }
+    eof = response->FindBool("eof").value_or(false);
+  }
+
+  EXPECT_THAT(trace_json, testing::HasSubstr("mark_a"));
+  EXPECT_THAT(trace_json, testing::Not(testing::HasSubstr("mark_b")));
+}
+
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, NavigateToAboutBlankLoaderId) {
   NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
   Attach();
diff --git a/content/browser/devtools/protocol/tracing_handler.cc b/content/browser/devtools/protocol/tracing_handler.cc
index 5e418a7e..0877b6f1 100644
--- a/content/browser/devtools/protocol/tracing_handler.cc
+++ b/content/browser/devtools/protocol/tracing_handler.cc
@@ -231,23 +231,6 @@
   return {};
 }
 
-void AddPidsToProcessFilter(
-    const std::unordered_set<base::ProcessId>& included_process_ids,
-    perfetto::TraceConfig& trace_config) {
-  const std::string kDataSourceName = kTrackEventDataSourceName;
-  for (auto& data_source : *(trace_config.mutable_data_sources())) {
-    auto* source_config = data_source.mutable_config();
-    if (source_config->name() == kDataSourceName) {
-      for (auto& enabled_pid : included_process_ids) {
-        *data_source.add_producer_name_filter() = base::StrCat(
-            {tracing::kPerfettoProducerNamePrefix,
-             base::NumberToString(static_cast<uint32_t>(enabled_pid))});
-      }
-      break;
-    }
-  }
-}
-
 bool IsChromeDataSource(const std::string& data_source_name) {
   return base::StartsWith(data_source_name, "org.chromium.") ||
          data_source_name == "track_event";
@@ -1181,6 +1164,32 @@
 }
 
 // static
+void TracingHandler::AddPidsToProcessFilter(
+    const std::unordered_set<base::ProcessId>& included_process_ids,
+    perfetto::TraceConfig& trace_config) {
+  for (auto& data_source : *(trace_config.mutable_data_sources())) {
+    auto* source_config = data_source.mutable_config();
+    if (IsChromeDataSource(source_config->name())) {
+      if (data_source.producer_name_regex_filter_size() > 0) {
+        data_source.clear_producer_name_regex_filter();
+        data_source.clear_producer_name_filter();
+      }
+      std::unordered_set<std::string> existing_filters(
+          data_source.producer_name_filter().begin(),
+          data_source.producer_name_filter().end());
+      for (auto& enabled_pid : included_process_ids) {
+        std::string new_filter = base::StrCat(
+            {tracing::kPerfettoProducerNamePrefix,
+             base::NumberToString(static_cast<uint32_t>(enabled_pid))});
+        if (existing_filters.find(new_filter) == existing_filters.end()) {
+          *data_source.add_producer_name_filter() = std::move(new_filter);
+        }
+      }
+    }
+  }
+}
+
+// static
 bool TracingHandler::IsStartupTracingActive() {
   return ::tracing::TraceStartupConfig::GetInstance().IsEnabled();
 }
diff --git a/content/browser/devtools/protocol/tracing_handler.h b/content/browser/devtools/protocol/tracing_handler.h
index 38f06075..f87d191b 100644
--- a/content/browser/devtools/protocol/tracing_handler.h
+++ b/content/browser/devtools/protocol/tracing_handler.h
@@ -11,6 +11,7 @@
 #include <memory>
 #include <set>
 #include <string>
+#include <unordered_set>
 #include <vector>

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 50b1f9935..3b80e4c 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -6,6 +6,7 @@
 
 #include <cstddef>
 #include <memory>
+#include <optional>
 #include <string_view>
 #include <utility>
 
@@ -3886,6 +3887,88 @@
   WaitForNotification("Tracing.tracingComplete", true);
 }
 
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, TracingPerfettoSiteIsolation) {
+  content::SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL url_a = embedded_test_server()->GetURL("a.com", "/title1.html");
+  GURL url_b = embedded_test_server()->GetURL("b.com", "/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), url_a, 1);
+  Shell* shell_b = CreateBrowser();
+  NavigateToURLBlockUntilNavigationsComplete(shell_b, url_b, 1);
+
+  Attach();
+
+  base::trace_event::TraceConfig chrome_config("blink.user_timing", "");
+  perfetto::TraceConfig perfetto_config =
+      tracing::GetDefaultPerfettoConfig(chrome_config,
+                                        /*privacy_filtering_enabled=*/false,
+                                        /*convert_to_legacy_json=*/true);
+
+  std::optional<perfetto::DataSourceConfig> track_event_config;
+  for (auto& ds : *perfetto_config.mutable_data_sources()) {
+    if (ds.config().name() == "track_event") {
+      ds.add_producer_name_regex_filter(".*");
+      track_event_config = ds.config();
+    }
+  }
+
+  if (track_event_config) {
+    auto* ds2 = perfetto_config.add_data_sources();
+    *ds2->mutable_config() = *track_event_config;
+    ds2->add_producer_name_regex_filter(".*");
+  }
+
+  auto* ds3 = perfetto_config.add_data_sources();
+  ds3->mutable_config()->set_name("org.chromium.sampler_profiler");
+
+  std::string perfetto_config_encoded =
+      base::Base64Encode(perfetto_config.SerializeAsString());
+
+  base::DictValue params;
+  params.Set("perfettoConfig", perfetto_config_encoded);
+  params.Set("transferMode", "ReturnAsStream");
+
+  EXPECT_TRUE(SendCommandSync("Tracing.start", std::move(params)));
+
+  EXPECT_TRUE(content::ExecJs(shell(), "performance.mark('mark_a');"));
+  EXPECT_TRUE(content::ExecJs(shell_b, "performance.mark('mark_b');"));
+
+  EXPECT_TRUE(SendCommandSync("Tracing.end"));
+
+  base::DictValue complete_notification =
+      WaitForNotification("Tracing.tracingComplete", true);
+  const std::string* stream_handle_ptr =
+      complete_notification.FindString("stream");
+  ASSERT_TRUE(stream_handle_ptr);
+  std::string stream_handle = *stream_handle_ptr;
+
+  std::string trace_json;
+  bool eof = false;
+  while (!eof) {
+    base::DictValue read_params;
+    read_params.Set("handle", stream_handle);
+    const base::DictValue* response =
+        SendCommandSync("IO.read", std::move(read_params));
+    ASSERT_TRUE(response);
+    const std::string* data = response->FindString("data");
+    ASSERT_TRUE(data);
+    std::optional<bool> base64_encoded = response->FindBool("base64Encoded");
+    if (base64_encoded.value_or(false)) {
+      std::string decoded;
+      ASSERT_TRUE(base::Base64Decode(*data, &decoded));
+      trace_json += decoded;
+    } else {
+      trace_json += *data;
+    }
+    eof = response->FindBool("eof").value_or(false);
+  }
+
+  EXPECT_THAT(trace_json, testing::HasSubstr("mark_a"));
+  EXPECT_THAT(trace_json, testing::Not(testing::HasSubstr("mark_b")));
+}
+
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, NavigateToAboutBlankLoaderId) {
   NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
   Attach();
diff --git a/content/browser/devtools/protocol/tracing_handler_unittest.cc b/content/browser/devtools/protocol/tracing_handler_unittest.cc
index 41cb50d1..df9ff9d 100644
--- a/content/browser/devtools/protocol/tracing_handler_unittest.cc
+++ b/content/browser/devtools/protocol/tracing_handler_unittest.cc
@@ -2,12 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "content/browser/devtools/protocol/tracing_handler.h"
+
 #include <memory>
 
 #include "base/json/json_reader.h"
 #include "base/trace_event/trace_config.h"
 #include "base/values.h"
-#include "content/browser/devtools/protocol/tracing_handler.h"
+#include "services/tracing/public/cpp/perfetto/perfetto_data_source_names.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
@@ -137,5 +140,84 @@
             GetValidTraceFragment(trace_data.substr(trace_data.size() - 1, 1)));
 }
 
+TEST_F(TracingHandlerTest, ProcessFilterClearsRegex) {
+  perfetto::TraceConfig trace_config;
+
+  auto* data_source = trace_config.add_data_sources();
+  auto* config = data_source->mutable_config();
+  config->set_name("track_event");
+  data_source->add_producer_name_regex_filter(".*");
+  data_source->add_producer_name_filter("old_filter");
+
+  auto* data_source2 = trace_config.add_data_sources();
+  auto* config2 = data_source2->mutable_config();
+  config2->set_name("org.chromium.foo");
+  data_source2->add_producer_name_regex_filter(".*");
+
+  auto* data_source3 = trace_config.add_data_sources();
+  auto* config3 = data_source3->mutable_config();
+  config3->set_name("other_source");
+  data_source3->add_producer_name_regex_filter(".*");
+
+  std::unordered_set<base::ProcessId> pids = {1234};
+  TracingHandler::AddPidsToProcessFilter(pids, trace_config);
+
+  ASSERT_EQ(3, trace_config.data_sources_size());
+
+  // Chrome track_event data source: regex cleared, pid added
+  EXPECT_EQ("track_event", trace_config.data_sources()[0].config().name());
+  EXPECT_EQ(0, trace_config.data_sources()[0].producer_name_regex_filter_size());
+  ASSERT_EQ(1, trace_config.data_sources()[0].producer_name_filter_size());
+  EXPECT_EQ(std::string(tracing::kPerfettoProducerNamePrefix) + "1234",
+            trace_config.data_sources()[0].producer_name_filter()[0]);
+
+  // Chrome data source: regex cleared, pid added
+  EXPECT_EQ("org.chromium.foo", trace_config.data_sources()[1].config().name());
+  EXPECT_EQ(0, trace_config.data_sources()[1].producer_name_regex_filter_size());
+  ASSERT_EQ(1, trace_config.data_sources()[1].producer_name_filter_size());
+  EXPECT_EQ(std::string(tracing::kPerfettoProducerNamePrefix) + "1234",
+            trace_config.data_sources()[1].producer_name_filter()[0]);
+
+  // Other data source: regex retained, pid not added
+  EXPECT_EQ("other_source", trace_config.data_sources()[2].config().name());
+  ASSERT_EQ(1, trace_config.data_sources()[2].producer_name_regex_filter_size());
+  EXPECT_EQ(".*", trace_config.data_sources()[2].producer_name_regex_filter()[0]);
+  EXPECT_EQ(0, trace_config.data_sources()[2].producer_name_filter_size());
+}
+
+TEST_F(TracingHandlerTest, ProcessFilterAppendsPids) {
+  perfetto::TraceConfig trace_config;
+
+  auto* data_source = trace_config.add_data_sources();
+  auto* config = data_source->mutable_config();
+  config->set_name("track_event");
+
+  // Initial PIDs
+  std::unordered_set<base::ProcessId> pids1 = {1234, 5678};
+  TracingHandler::AddPidsToProcessFilter(pids1, trace_config);
+
+  ASSERT_EQ(1, trace_config.data_sources_size());
+  EXPECT_EQ(2, trace_config.data_sources()[0].producer_name_filter_size());
+
+  // Append new PID
+  std::unordered_set<base::ProcessId> pids2 = {9012};
+  TracingHandler::AddPidsToProcessFilter(pids2, trace_config);
+
+  ASSERT_EQ(1, trace_config.data_sources_size());
+  // Should have 3 PIDs now
+  EXPECT_EQ(3, trace_config.data_sources()[0].producer_name_filter_size());
+
+  EXPECT_THAT(trace_config.data_sources()[0].producer_name_filter(),
+              testing::Contains(
+                  std::string(tracing::kPerfettoProducerNamePrefix) + "1234"));
+  EXPECT_THAT(trace_config.data_sources()[0].producer_name_filter(),
+              testing::Contains(
+                  std::string(tracing::kPerfettoProducerNamePrefix) + "5678"));
+  EXPECT_THAT(trace_config.data_sources()[0].producer_name_filter(),
+              testing::Contains(
+                  std::string(tracing::kPerfettoProducerNamePrefix) + "9012"));
+}
+
+
 }  // namespace protocol
 }  // namespace content