Commit 60108f63 authored by Eric Seckler's avatar Eric Seckler Committed by Commit Bot

tracing: Attempt to make startup file writing more reliable in tests

Some of our startup tracing tests timeout flakily, and it seems that the
flakiness stems from the file not being written quickly enough on the
background task runner.

This patch overrides the task priority for the task runner for these
tests to USER_BLOCKING, which should make file writing more likely to
finish in time.

Also reenables one of the tests on windows, where it was previously
timing out flakily.

Bug: 1046447
Change-Id: Ibbe2ef61f81567a364f4d3396162e72be22b4457
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032109Reviewed-by: default avataroysteine <oysteine@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738293}
parent 97e4ca7e
......@@ -129,6 +129,16 @@ PerfettoFileTracer::~PerfettoFileTracer() = default;
void PerfettoFileTracer::OnTracingEnabled() {}
void PerfettoFileTracer::SetBackgroundTaskPriorityForTesting(
base::TaskPriority priority) {
background_drainer_.Reset();
background_task_runner_ = base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::MayBlock(), priority,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN});
background_drainer_ =
base::SequenceBound<BackgroundDrainer>(background_task_runner_);
}
void PerfettoFileTracer::OnTracingDisabled() {
has_been_disabled_ = true;
}
......
......@@ -9,7 +9,9 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/task/task_traits.h"
#include "base/threading/sequence_bound.h"
#include "content/common/content_export.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/tracing/public/mojom/perfetto_service.mojom.h"
......@@ -37,13 +39,15 @@ class PerfettoFileTracer : public tracing::mojom::TracingSessionClient {
bool is_finished_for_testing() const { return !background_drainer_; }
CONTENT_EXPORT void SetBackgroundTaskPriorityForTesting(base::TaskPriority);
private:
void OnNoMorePackets(bool queued_after_disable);
void ReadBuffers();
void OnTracingSessionEnded();
const scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
base::SequenceBound<BackgroundDrainer> background_drainer_;
mojo::Receiver<tracing::mojom::TracingSessionClient> receiver_{this};
mojo::Remote<tracing::mojom::TracingSessionHost> tracing_session_host_;
......
......@@ -61,6 +61,14 @@ class CommandlineStartupTracingTest : public ContentBrowserTest {
#endif
}
void PreRunTestOnMainThread() override {
// Ensure that the file is written quickly for these tests.
TracingControllerImpl::GetInstance()
->set_startup_file_endpoint_priority_for_testing(
base::TaskPriority::USER_BLOCKING);
ContentBrowserTest::PreRunTestOnMainThread();
}
protected:
base::FilePath temp_file_path_;
......@@ -68,9 +76,9 @@ class CommandlineStartupTracingTest : public ContentBrowserTest {
DISALLOW_COPY_AND_ASSIGN(CommandlineStartupTracingTest);
};
// Failing on Android ASAN, Linux TSAN and Windows 10. crbug.com/1041392
// Failing on Android ASAN, Linux TSAN. crbug.com/1041392
#if (defined(OS_ANDROID) && defined(ADDRESS_SANITIZER)) || \
(defined(OS_LINUX) && defined(THREAD_SANITIZER)) || defined(OS_WIN)
(defined(OS_LINUX) && defined(THREAD_SANITIZER))
#define MAYBE_TestStartupTracing DISABLED_TestStartupTracing
#else
#define MAYBE_TestStartupTracing TestStartupTracing
......@@ -182,6 +190,15 @@ class BackgroundStartupTracingTest : public ContentBrowserTest {
temp_file_path_.AsUTF8Unsafe());
}
void PreRunTestOnMainThread() override {
// Ensure that the file is written quickly for these tests.
TracingControllerImpl::GetInstance()
->perfetto_file_tracer_for_testing()
->SetBackgroundTaskPriorityForTesting(
base::TaskPriority::USER_BLOCKING);
ContentBrowserTest::PreRunTestOnMainThread();
}
protected:
base::FilePath temp_file_path_;
......
......@@ -465,7 +465,8 @@ void TracingControllerImpl::EndStartupTracing() {
StopTracing(CreateFileEndpoint(
startup_trace_file_,
base::BindRepeating(OnStoppedStartupTracing, startup_trace_file_)));
base::BindRepeating(OnStoppedStartupTracing, startup_trace_file_),
startup_file_endpoint_priority_));
}
void TracingControllerImpl::FinalizeStartupTracingIfNeeded() {
......
......@@ -12,6 +12,7 @@
#include "base/callback_forward.h"
#include "base/memory/ref_counted.h"
#include "base/task/task_traits.h"
#include "base/timer/timer.h"
#include "content/common/content_export.h"
#include "content/public/browser/tracing_controller.h"
......@@ -88,10 +89,15 @@ class TracingControllerImpl : public TracingController,
// exists.
void FinalizeStartupTracingIfNeeded();
const PerfettoFileTracer* perfetto_file_tracer_for_testing() const {
PerfettoFileTracer* perfetto_file_tracer_for_testing() const {
return perfetto_file_tracer_.get();
}
void set_startup_file_endpoint_priority_for_testing(
base::TaskPriority priority) {
startup_file_endpoint_priority_ = priority;
}
private:
friend std::default_delete<TracingControllerImpl>;
......@@ -132,6 +138,8 @@ class TracingControllerImpl : public TracingController,
base::FilePath startup_trace_file_;
// This timer initiates trace file saving.
base::OneShotTimer startup_trace_timer_;
base::TaskPriority startup_file_endpoint_priority_ =
base::TaskPriority::BEST_EFFORT;
#if defined(OS_CHROMEOS)
bool are_statistics_loaded_ = false;
......
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