Commit 6a99e866 authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot

Revert "Add browser test for perfetto file tracer"

This reverts commit 6bb27973.

Reason for revert: caused failure https://ci.chromium.org/p/chromium/builders/ci/Win10%20Tests%20x64/35197

Original change's description:
> Add browser test for perfetto file tracer
> 
> Adds a browser test to perfetto file tracer in privacy mode. This
> currently just tests if the output proto only contains the whitelisted
> toplevel fields in TracePacket proto. It does not correctly verify the
> submessages.
> Adds a new library for checking privacy filtering in services/tracing.
> Currently this is marked test_only. But in future we might want to
> enable this in production on dcheck enabled builds.
> 
> BUG=925148
> 
> Change-Id: Ib85e0e67ce9716cd839aeda2158ecf0dcfe9ea13
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1570235
> Commit-Queue: ssid <ssid@chromium.org>
> Reviewed-by: oysteine <oysteine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#654379}

TBR=oysteine@chromium.org,ssid@chromium.org

Change-Id: Ie56f0f779f2aeaac0ae263efaf41722221778e76
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 925148
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1584269Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654393}
parent d3f5e039
......@@ -242,12 +242,10 @@ bool TraceStartupConfig::EnableFromConfigFile() {
bool TraceStartupConfig::EnableFromBackgroundTracing() {
#if defined(OS_ANDROID)
// Tests can enable this value.
is_enabled_from_background_tracing_ =
is_enabled_from_background_tracing_ ||
base::android::GetBackgroundStartupTracingFlag();
#else
// TODO(ssid): Implement saving setting to preference for next startup.
is_enabled_from_background_tracing_ = false;
#endif
// Do not set the flag to false if it's not enabled unnecessarily.
if (!is_enabled_from_background_tracing_)
......
......@@ -17,8 +17,7 @@ struct DefaultSingletonTraits;
} // namespace base
namespace content {
class CommandlineStartupTracingTest;
class BackgroundStartupTracingTest;
class StartupTracingControllerTest;
}
namespace tracing {
......@@ -140,8 +139,7 @@ class TRACING_EXPORT TraceStartupConfig {
// This allows constructor and destructor to be private and usable only
// by the Singleton class.
friend struct base::DefaultSingletonTraits<TraceStartupConfig>;
friend class content::CommandlineStartupTracingTest;
friend class content::BackgroundStartupTracingTest;
friend class content::StartupTracingControllerTest;
TraceStartupConfig();
~TraceStartupConfig();
......
......@@ -147,12 +147,7 @@ void BackgroundTracingActiveScenario::StartTracing(
// Perfetto-related deadlocks are resolved.
if (!TracingControllerImpl::GetInstance()->IsTracing() &&
tracing::TracingUsesPerfettoBackend()) {
// TODO(oysteine): This should pass in |requires_anonymized_data_| instead
// of false only when using consumer API with proto output. But, for JSON
// output we still need this to be false since filtering happens in the JSON
// exporter.
tracing::TraceEventDataSource::GetInstance()->SetupStartupTracing(
/*privacy_filtering_enabled=*/false);
tracing::TraceEventDataSource::GetInstance()->SetupStartupTracing();
}
#endif
......
......@@ -9,7 +9,6 @@
#include "base/command_line.h"
#include "base/files/file.h"
#include "base/files/file_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/task/post_task.h"
#include "base/threading/scoped_blocking_call.h"
#include "components/tracing/common/trace_startup_config.h"
......@@ -17,7 +16,6 @@
#include "content/public/common/service_manager_connection.h"
#include "mojo/public/cpp/system/data_pipe_drainer.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/tracing/public/cpp/perfetto/perfetto_config.h"
#include "services/tracing/public/mojom/constants.mojom.h"
#include "third_party/perfetto/include/perfetto/tracing/core/trace_config.h"
#include "third_party/perfetto/protos/perfetto/config/trace_config.pb.h"
......@@ -91,27 +89,36 @@ PerfettoFileTracer::PerfettoFileTracer()
ServiceManagerConnection::GetForProcess()->GetConnector()->BindInterface(
tracing::mojom::kServiceName, &consumer_host_);
const auto& chrome_config =
tracing::TraceStartupConfig::GetInstance()->GetTraceConfig();
perfetto::TraceConfig trace_config =
tracing::GetDefaultPerfettoConfig(chrome_config);
// TODO(ssid): This should be moved to GetDefaultPerfettoConfig(). But,
// currently we only require this for proto output since JSON exporter still
// needs sensitive fields in trace, which will later be stripped.
for (auto& source : *trace_config.mutable_data_sources()) {
source.mutable_config()
->mutable_chrome_config()
->set_privacy_filtering_enabled(
chrome_config.IsArgumentFilterEnabled());
}
perfetto::TraceConfig trace_config;
int duration_in_seconds =
tracing::TraceStartupConfig::GetInstance()->GetStartupDuration();
trace_config.set_duration_ms(duration_in_seconds * 1000);
// We just need a single global trace buffer, for our data.
trace_config.mutable_buffers()->front().set_size_kb(32 * 1024);
trace_config.add_buffers()->set_size_kb(32 * 1024);
// We need data from two different sources to get the complete trace
// we're interested in both, written into the single buffer we
// configure above.
// This source is the actual trace events themselves coming
// from the base::TraceLog
auto* trace_event_config = trace_config.add_data_sources()->mutable_config();
trace_event_config->set_name(tracing::mojom::kTraceEventDataSourceName);
trace_event_config->set_target_buffer(0);
auto* chrome_config = trace_event_config->mutable_chrome_config();
// The Chrome config string is passed straight through to base::TraceLog
// and defines which tracing categories should be enabled.
auto chrome_raw_config =
tracing::TraceStartupConfig::GetInstance()->GetTraceConfig().ToString();
chrome_config->set_trace_config(chrome_raw_config);
// The second data source we're interested in is the global metadata.
auto* trace_metadata_config =
trace_config.add_data_sources()->mutable_config();
trace_metadata_config->set_name(tracing::mojom::kMetaDataSourceName);
trace_metadata_config->set_target_buffer(0);
tracing::mojom::TracingSessionPtr tracing_session;
binding_.Bind(mojo::MakeRequest(&tracing_session));
......
......@@ -33,8 +33,6 @@ class PerfettoFileTracer : public tracing::mojom::TracingSession {
// tracing::mojom::TracingSession implementation:
void OnTracingEnabled() override;
bool is_finished_for_testing() const { return !background_drainer_; }
private:
void OnNoMorePackets(bool queued_after_disable);
void ReadBuffers();
......
......@@ -11,11 +11,9 @@
#include "build/build_config.h"
#include "components/tracing/common/trace_startup_config.h"
#include "components/tracing/common/tracing_switches.h"
#include "content/browser/tracing/perfetto_file_tracer.h"
#include "content/browser/tracing/tracing_controller_impl.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "services/tracing/perfetto/privacy_filtering_check.h"
#include "services/tracing/public/cpp/perfetto/trace_event_data_source.h"
#include "services/tracing/public/cpp/trace_startup.h"
#include "services/tracing/public/cpp/tracing_features.h"
......@@ -27,7 +25,7 @@ namespace {
// Wait until |condition| returns true.
void WaitForCondition(base::RepeatingCallback<bool()> condition,
const std::string& description) {
const base::TimeDelta kTimeout = base::TimeDelta::FromSeconds(15);
const base::TimeDelta kTimeout = base::TimeDelta::FromSeconds(30);
const base::TimeTicks start_time = base::TimeTicks::Now();
while (!condition.Run() && (base::TimeTicks::Now() - start_time < kTimeout)) {
base::RunLoop run_loop;
......@@ -41,10 +39,7 @@ void WaitForCondition(base::RepeatingCallback<bool()> condition,
} // namespace
class CommandlineStartupTracingTest : public ContentBrowserTest {
public:
CommandlineStartupTracingTest() = default;
class StartupTracingControllerTest : public ContentBrowserTest {
void SetUpCommandLine(base::CommandLine* command_line) override {
base::CreateTemporaryFile(&temp_file_path_);
command_line->AppendSwitch(switches::kTraceStartup);
......@@ -63,13 +58,10 @@ class CommandlineStartupTracingTest : public ContentBrowserTest {
protected:
base::FilePath temp_file_path_;
private:
DISALLOW_COPY_AND_ASSIGN(CommandlineStartupTracingTest);
};
IN_PROC_BROWSER_TEST_F(CommandlineStartupTracingTest, TestStartupTracing) {
NavigateToURL(shell(), GetTestUrl("", "title1.html"));
IN_PROC_BROWSER_TEST_F(StartupTracingControllerTest, TestStartupTracing) {
NavigateToURL(shell(), GetTestUrl("", "title.html"));
WaitForCondition(base::BindRepeating([]() {
return !TracingController::GetInstance()->IsTracing();
}),
......@@ -124,8 +116,7 @@ class LargeTraceEventData : public base::trace_event::ConvertableToTraceFormat {
// deadlocks.
IN_PROC_BROWSER_TEST_F(StartupTracingInProcessTest,
DISABLED_TestFilledStartupBuffer) {
tracing::TraceEventDataSource::GetInstance()->SetupStartupTracing(
/*privacy_filtering_enabled=*/false);
tracing::TraceEventDataSource::GetInstance()->SetupStartupTracing();
auto config = tracing::TraceStartupConfig::GetInstance()
->GetDefaultBrowserStartupConfig();
......@@ -147,56 +138,4 @@ IN_PROC_BROWSER_TEST_F(StartupTracingInProcessTest,
NavigateToURL(shell(), GetTestUrl("", "title1.html"));
}
class BackgroundStartupTracingTest : public ContentBrowserTest {
public:
BackgroundStartupTracingTest() = default;
void SetUpCommandLine(base::CommandLine* command_line) override {
base::CreateTemporaryFile(&temp_file_path_);
auto* startup_config = tracing::TraceStartupConfig::GetInstance();
startup_config->is_enabled_from_background_tracing_ = true;
startup_config->EnableFromBackgroundTracing();
startup_config->startup_duration_ = 3;
tracing::EnableStartupTracingIfNeeded();
command_line->AppendSwitchASCII(switches::kPerfettoOutputFile,
temp_file_path_.AsUTF8Unsafe());
}
protected:
base::FilePath temp_file_path_;
private:
DISALLOW_COPY_AND_ASSIGN(BackgroundStartupTracingTest);
};
#if defined(IS_CHROMECAST) && defined(OS_LINUX)
#define MAYBE_TestStartupTracing DISABLED_TestStartupTracing
#else
#define MAYBE_TestStartupTracing TestStartupTracing
#endif
IN_PROC_BROWSER_TEST_F(BackgroundStartupTracingTest, MAYBE_TestStartupTracing) {
NavigateToURL(shell(), GetTestUrl("", "title1.html"));
EXPECT_FALSE(tracing::TraceStartupConfig::GetInstance()->IsEnabled());
EXPECT_FALSE(TracingController::GetInstance()->IsTracing());
WaitForCondition(base::BindRepeating([]() {
return TracingControllerImpl::GetInstance()
->perfetto_file_tracer_for_testing()
->is_finished_for_testing();
}),
"finish file write");
std::string trace;
base::ScopedAllowBlockingForTesting allow_blocking;
ASSERT_TRUE(base::ReadFileToString(temp_file_path_, &trace));
tracing::PrivacyFilteringCheck checker;
checker.CheckProtoForUnexpectedFields(trace);
EXPECT_GT(checker.counts().track_event, 0u);
EXPECT_EQ(checker.counts().process_desc, 0u);
EXPECT_GT(checker.counts().thread_desc, 0u);
EXPECT_GT(checker.counts().interned_name, 0u);
EXPECT_GT(checker.counts().interned_category, 0u);
EXPECT_GT(checker.counts().interned_source_location, 0u);
}
} // namespace content
......@@ -84,10 +84,6 @@ class TracingControllerImpl : public TracingController,
// exists.
void FinalizeStartupTracingIfNeeded();
const PerfettoFileTracer* perfetto_file_tracer_for_testing() const {
return perfetto_file_tracer_.get();
}
private:
friend std::default_delete<TracingControllerImpl>;
......
......@@ -1038,7 +1038,6 @@ test("content_browsertests") {
"//services/network:test_support",
"//services/service_manager/public/cpp",
"//services/test/echo/public/mojom",
"//services/tracing:privacy_check",
"//services/video_capture/public/cpp",
"//services/video_capture/public/cpp:mocks",
"//services/video_capture/public/mojom:constants",
......
......@@ -58,22 +58,6 @@ source_set("manifest") {
]
}
source_set("privacy_check") {
testonly = true
sources = [
"perfetto/privacy_filtering_check.cc",
"perfetto/privacy_filtering_check.h",
]
deps = [
"//base",
"//third_party/perfetto/include/perfetto/protozero:protozero",
"//third_party/perfetto/protos/perfetto/common:lite",
"//third_party/perfetto/protos/perfetto/trace:lite",
]
}
source_set("tests") {
testonly = true
......
......@@ -139,14 +139,6 @@ void ConsumerHost::EnableTracing(mojom::TracingSessionPtr tracing_session,
privacy_filtering_enabled_ = true;
}
}
#if DCHECK_IS_ON()
if (privacy_filtering_enabled_) {
// If enabled, filtering must be enabled for all data sources.
for (const auto& data_source : trace_config.data_sources()) {
DCHECK(data_source.config().chrome_config().privacy_filtering_enabled());
}
}
#endif
perfetto::TraceConfig trace_config_copy = AdjustTraceConfig(trace_config);
filtered_pids_.clear();
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "services/tracing/perfetto/privacy_filtering_check.h"
#include "base/logging.h"
#include "third_party/perfetto/protos/perfetto/trace/trace.pb.h"
#include "third_party/perfetto/protos/perfetto/trace/trace_packet.pb.h"
namespace tracing {
namespace {
using perfetto::protos::TracePacket;
}
PrivacyFilteringCheck::PrivacyFilteringCheck() = default;
PrivacyFilteringCheck::~PrivacyFilteringCheck() = default;
// static
void PrivacyFilteringCheck::CheckProtoForUnexpectedFields(
const std::string& serialized_trace_proto) {
auto proto = std::make_unique<perfetto::protos::Trace>();
if (!proto->ParseFromArray(serialized_trace_proto.data(),
serialized_trace_proto.size())) {
NOTREACHED();
return;
}
for (auto& packet : *proto->mutable_packet()) {
DCHECK(packet.unknown_fields().empty());
// TODO(ssid): For each of these messages verify that only expected fields
// are present.
if (packet.has_track_event()) {
++counts_.track_event;
} else if (packet.has_process_descriptor()) {
++counts_.process_desc;
} else if (packet.has_thread_descriptor()) {
++counts_.thread_desc;
} else if (packet.has_synchronization_marker() ||
packet.has_trace_stats() || packet.has_trace_config() ||
packet.has_system_info()) {
// TODO(ssid): These should not be present in the traces or must be
// whitelisted.
} else {
DCHECK_EQ(TracePacket::DataCase::DATA_NOT_SET, packet.data_case());
}
if (packet.has_interned_data()) {
counts_.interned_name += packet.interned_data().legacy_event_names_size();
packet.mutable_interned_data()->clear_legacy_event_names();
counts_.interned_category +=
packet.interned_data().event_categories_size();
packet.mutable_interned_data()->clear_event_categories();
counts_.interned_source_location +=
packet.interned_data().source_locations_size();
packet.mutable_interned_data()->clear_source_locations();
std::string serialized;
packet.interned_data().SerializeToString(&serialized);
DCHECK_EQ(serialized.size(), 0u);
}
}
}
} // namespace tracing
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef SERVICES_TRACING_PERFETTO_PRIVACY_FILTERING_CHECK_H_
#define SERVICES_TRACING_PERFETTO_PRIVACY_FILTERING_CHECK_H_
#include <string>
#include "base/macros.h"
namespace tracing {
class PrivacyFilteringCheck {
public:
struct TypeCounts {
size_t track_event = 0;
size_t process_desc = 0;
size_t thread_desc = 0;
size_t interned_name = 0;
size_t interned_category = 0;
size_t interned_source_location = 0;
};
PrivacyFilteringCheck();
~PrivacyFilteringCheck();
void CheckProtoForUnexpectedFields(const std::string& serialized_trace_proto);
const TypeCounts& counts() const { return counts_; }
private:
TypeCounts counts_;
DISALLOW_COPY_AND_ASSIGN(PrivacyFilteringCheck);
};
} // namespace tracing
#endif // SERVICES_TRACING_PERFETTO_PRIVACY_FILTERING_CHECK_H_
......@@ -83,9 +83,6 @@ TraceEventMetadataSource::GenerateTraceConfigMetadataDict() {
void TraceEventMetadataSource::GenerateMetadata(
std::unique_ptr<perfetto::TraceWriter> trace_writer) {
if (privacy_filtering_enabled_) {
return;
}
DCHECK(origin_task_runner_->RunsTasksInCurrentSequence());
auto trace_packet = trace_writer->NewTracePacket();
......@@ -210,17 +207,15 @@ void TraceEventDataSource::UnregisterFromTraceLog() {
TraceLog::GetInstance()->SetAddTraceEventOverrides(nullptr, nullptr, nullptr);
}
void TraceEventDataSource::SetupStartupTracing(bool privacy_filtering_enabled) {
void TraceEventDataSource::SetupStartupTracing() {
{
base::AutoLock lock(lock_);
// No need to do anything if startup tracing has already been set,
// or we know Perfetto has already been setup.
if (startup_writer_registry_ || producer_client_) {
DCHECK(!privacy_filtering_enabled || privacy_filtering_enabled_);
return;
}
privacy_filtering_enabled_ = privacy_filtering_enabled;
startup_writer_registry_ =
std::make_unique<perfetto::StartupTraceWriterRegistry>();
}
......@@ -230,19 +225,13 @@ void TraceEventDataSource::SetupStartupTracing(bool privacy_filtering_enabled) {
void TraceEventDataSource::StartTracing(
PerfettoProducer* producer,
const perfetto::DataSourceConfig& data_source_config) {
privacy_filtering_enabled_ =
data_source_config.chrome_config().privacy_filtering_enabled();
std::unique_ptr<perfetto::StartupTraceWriterRegistry> unbound_writer_registry;
{
base::AutoLock lock(lock_);
bool should_enable_filtering =
data_source_config.chrome_config().privacy_filtering_enabled();
if (should_enable_filtering) {
CHECK(!startup_writer_registry_ || privacy_filtering_enabled_)
<< "Unexpected StartTracing received when startup tracing is "
"running.";
}
privacy_filtering_enabled_ = should_enable_filtering;
DCHECK(!producer_client_);
producer_client_ = producer;
target_buffer_ = data_source_config.target_buffer();
......@@ -253,6 +242,9 @@ void TraceEventDataSource::StartTracing(
session_id_.fetch_add(1u, std::memory_order_relaxed);
if (unbound_writer_registry) {
// TODO(ssid): Startup tracing should know about filtering output.
CHECK(!privacy_filtering_enabled_);
// TODO(oysteine): Investigate why trace events emitted by something in
// BindStartupTraceWriterRegistry() causes deadlocks.
AutoThreadLocalBoolean thread_is_in_trace_event(
......
......@@ -99,7 +99,7 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventDataSource
// is locally buffered until connection to the perfetto service is
// established. Expects a later call to StartTracing() to bind to the perfetto
// service. Should only be called once.
void SetupStartupTracing(bool privacy_filtering_enabled);
void SetupStartupTracing();
// The PerfettoProducer is responsible for calling StopTracing
// which will clear the stored pointer to it, before it
......@@ -150,6 +150,7 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventDataSource
void LogHistogram(base::HistogramBase* histogram);
bool disable_interning_ = false;
bool privacy_filtering_enabled_ = false;
base::OnceClosure stop_complete_callback_;
// Incremented and accessed atomically but without memory order guarantees.
......@@ -167,7 +168,6 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventDataSource
std::unique_ptr<perfetto::StartupTraceWriterRegistry>
startup_writer_registry_;
std::vector<std::string> histograms_;
bool privacy_filtering_enabled_ = false;
DISALLOW_COPY_AND_ASSIGN(TraceEventDataSource);
};
......
......@@ -64,7 +64,7 @@ class COMPONENT_EXPORT(TRACING_CPP) TrackEventThreadLocalEventSink
base::trace_event::TraceEvent complete_event_stack_[kMaxCompleteEventDepth];
uint32_t current_stack_depth_ = 0;
const bool privacy_filtering_enabled_;
bool privacy_filtering_enabled_;
};
} // namespace tracing
......
......@@ -15,11 +15,6 @@
namespace tracing {
namespace {
using base::trace_event::TraceConfig;
using base::trace_event::TraceLog;
} // namespace
void EnableStartupTracingIfNeeded() {
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
......@@ -32,31 +27,29 @@ void EnableStartupTracingIfNeeded() {
// Ensure TraceLog is initialized first.
// https://crbug.com/764357
auto* trace_log = TraceLog::GetInstance();
auto* startup_config = TraceStartupConfig::GetInstance();
if (startup_config->IsEnabled()) {
if (TracingUsesPerfettoBackend()) {
TraceEventDataSource::GetInstance()->SetupStartupTracing(
startup_config->GetBackgroundStartupTracingEnabled());
}
base::trace_event::TraceLog::GetInstance();
const TraceConfig& trace_config = startup_config->GetTraceConfig();
uint8_t modes = TraceLog::RECORDING_MODE;
if (TraceStartupConfig::GetInstance()->IsEnabled()) {
const base::trace_event::TraceConfig& trace_config =
TraceStartupConfig::GetInstance()->GetTraceConfig();
uint8_t modes = base::trace_event::TraceLog::RECORDING_MODE;
if (!trace_config.event_filters().empty())
modes |= TraceLog::FILTERING_MODE;
trace_log->SetEnabled(startup_config->GetTraceConfig(), modes);
modes |= base::trace_event::TraceLog::FILTERING_MODE;
if (TracingUsesPerfettoBackend())
TraceEventDataSource::GetInstance()->SetupStartupTracing();
base::trace_event::TraceLog::GetInstance()->SetEnabled(
TraceStartupConfig::GetInstance()->GetTraceConfig(), modes);
} else if (command_line.HasSwitch(switches::kTraceToConsole)) {
// TODO(eseckler): Remove ability to trace to the console, perfetto doesn't
// support this and noone seems to use it.
TraceConfig trace_config = GetConfigForTraceToConsole();
base::trace_event::TraceConfig trace_config = GetConfigForTraceToConsole();
LOG(ERROR) << "Start " << switches::kTraceToConsole
<< " with CategoryFilter '"
<< trace_config.ToCategoryFilterString() << "'.";
if (TracingUsesPerfettoBackend())
TraceEventDataSource::GetInstance()->SetupStartupTracing(
/*privacy_filtering_enabled=*/false);
trace_log->SetEnabled(trace_config, TraceLog::RECORDING_MODE);
TraceEventDataSource::GetInstance()->SetupStartupTracing();
base::trace_event::TraceLog::GetInstance()->SetEnabled(
trace_config, base::trace_event::TraceLog::RECORDING_MODE);
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment