Commit d79ae88e authored by John Delaney's avatar John Delaney Committed by Commit Bot

Setup Conversions database in memory for browsertests

This is a speculative fix for Conversions browsertest flakes caused
by the database initialization task presumably hanging. This is
a followup of https://crrev.com/c/2225739, as tests are still flaking.

Sets up the Conversions storage in memory rather than on disk for
browsertests. If the database initialization task is just slow and
running into timeouts, running in memory should vastly speed it up.

Bug: 1087775,1058199,1080764,1058199,1087406,1084201
Change-Id: I740acb32ce835553b29709bd854944eee7da5122
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225286
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774466}
parent f7d96be6
...@@ -34,6 +34,11 @@ ConversionManager* ConversionManagerProviderImpl::GetManager( ...@@ -34,6 +34,11 @@ ConversionManager* ConversionManagerProviderImpl::GetManager(
->GetConversionManager(); ->GetConversionManager();
} }
// static
void ConversionManagerImpl::RunInMemoryForTesting() {
ConversionStorageSql::RunInMemoryForTesting();
}
// static // static
std::unique_ptr<ConversionManagerImpl> ConversionManagerImpl::CreateForTesting( std::unique_ptr<ConversionManagerImpl> ConversionManagerImpl::CreateForTesting(
std::unique_ptr<ConversionReporter> reporter, std::unique_ptr<ConversionReporter> reporter,
......
...@@ -64,6 +64,10 @@ class CONTENT_EXPORT ConversionManagerImpl : public ConversionManager { ...@@ -64,6 +64,10 @@ class CONTENT_EXPORT ConversionManagerImpl : public ConversionManager {
base::RepeatingCallback<void(int64_t)> report_sent_callback) = 0; base::RepeatingCallback<void(int64_t)> report_sent_callback) = 0;
}; };
// Configures underlying storage to be setup in memory, rather than on
// disk. This speeds up initialization to avoid timeouts in test environments.
static void RunInMemoryForTesting();
static std::unique_ptr<ConversionManagerImpl> CreateForTesting( static std::unique_ptr<ConversionManagerImpl> CreateForTesting(
std::unique_ptr<ConversionReporter> reporter, std::unique_ptr<ConversionReporter> reporter,
std::unique_ptr<ConversionPolicy> policy, std::unique_ptr<ConversionPolicy> policy,
......
...@@ -26,9 +26,6 @@ namespace content { ...@@ -26,9 +26,6 @@ namespace content {
namespace { namespace {
const base::FilePath::CharType kDatabaseName[] =
FILE_PATH_LITERAL("Conversions");
std::string SerializeOrigin(const url::Origin& origin) { std::string SerializeOrigin(const url::Origin& origin) {
// Conversion API is only designed to be used for secure // Conversion API is only designed to be used for secure
// contexts (targets and reporting endpoints). We should have filtered out bad // contexts (targets and reporting endpoints). We should have filtered out bad
...@@ -50,13 +47,28 @@ base::Time DeserializeTime(int64_t microseconds) { ...@@ -50,13 +47,28 @@ base::Time DeserializeTime(int64_t microseconds) {
base::TimeDelta::FromMicroseconds(microseconds)); base::TimeDelta::FromMicroseconds(microseconds));
} }
const base::FilePath::CharType kInMemoryPath[] = FILE_PATH_LITERAL(":memory");
const base::FilePath::CharType kDatabasePath[] =
FILE_PATH_LITERAL("Conversions");
} // namespace } // namespace
// static
void ConversionStorageSql::RunInMemoryForTesting() {
g_run_in_memory_ = true;
}
// static
bool ConversionStorageSql::g_run_in_memory_ = false;
ConversionStorageSql::ConversionStorageSql( ConversionStorageSql::ConversionStorageSql(
const base::FilePath& path_to_database_dir, const base::FilePath& path_to_database,
std::unique_ptr<Delegate> delegate, std::unique_ptr<Delegate> delegate,
const base::Clock* clock) const base::Clock* clock)
: path_to_database_(path_to_database_dir.Append(kDatabaseName)), : path_to_database_(g_run_in_memory_
? base::FilePath(kInMemoryPath)
: path_to_database.Append(kDatabasePath)),
clock_(clock), clock_(clock),
delegate_(std::move(delegate)), delegate_(std::move(delegate)),
weak_factory_(this) { weak_factory_(this) {
...@@ -81,7 +93,13 @@ bool ConversionStorageSql::Initialize() { ...@@ -81,7 +93,13 @@ bool ConversionStorageSql::Initialize() {
db_.set_page_size(4096); db_.set_page_size(4096);
db_.set_cache_size(32); db_.set_cache_size(32);
db_.set_exclusive_locking(); db_.set_exclusive_locking();
return db_.Open(path_to_database_) && InitializeSchema();
if ((path_to_database_.value() == kInMemoryPath)
? !db_.OpenInMemory()
: !db_.Open(path_to_database_)) {
return false;
}
return InitializeSchema();
} }
void ConversionStorageSql::StoreImpression( void ConversionStorageSql::StoreImpression(
...@@ -700,8 +718,9 @@ bool ConversionStorageSql::InitializeSchema() { ...@@ -700,8 +718,9 @@ bool ConversionStorageSql::InitializeSchema() {
void ConversionStorageSql::DatabaseErrorCallback(int extended_error, void ConversionStorageSql::DatabaseErrorCallback(int extended_error,
sql::Statement* stmt) { sql::Statement* stmt) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Attempt to recover corrupt databases. // Attempt to recover a corrupt database, unless it is setup in memory.
if (sql::Recovery::ShouldRecover(extended_error)) { if (sql::Recovery::ShouldRecover(extended_error) &&
(path_to_database_.value() != kInMemoryPath)) {
// Prevent reentrant calls. // Prevent reentrant calls.
db_.reset_error_callback(); db_.reset_error_callback();
......
...@@ -28,7 +28,9 @@ namespace content { ...@@ -28,7 +28,9 @@ namespace content {
// destroyed on the same sequence. The sequence must outlive |this|. // destroyed on the same sequence. The sequence must outlive |this|.
class CONTENT_EXPORT ConversionStorageSql : public ConversionStorage { class CONTENT_EXPORT ConversionStorageSql : public ConversionStorage {
public: public:
ConversionStorageSql(const base::FilePath& path_to_database_dir, static void RunInMemoryForTesting();
ConversionStorageSql(const base::FilePath& path_to_database,
std::unique_ptr<Delegate> delegate, std::unique_ptr<Delegate> delegate,
const base::Clock* clock); const base::Clock* clock);
ConversionStorageSql(const ConversionStorageSql& other) = delete; ConversionStorageSql(const ConversionStorageSql& other) = delete;
...@@ -62,6 +64,8 @@ class CONTENT_EXPORT ConversionStorageSql : public ConversionStorage { ...@@ -62,6 +64,8 @@ class CONTENT_EXPORT ConversionStorageSql : public ConversionStorage {
void DatabaseErrorCallback(int extended_error, sql::Statement* stmt); void DatabaseErrorCallback(int extended_error, sql::Statement* stmt);
static bool g_run_in_memory_;
const base::FilePath path_to_database_; const base::FilePath path_to_database_;
sql::Database db_; sql::Database db_;
......
...@@ -733,6 +733,9 @@ class StoragePartitionImplTest : public testing::Test { ...@@ -733,6 +733,9 @@ class StoragePartitionImplTest : public testing::Test {
StoragePartitionImplTest() StoragePartitionImplTest()
: task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP), : task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP),
browser_context_(new TestBrowserContext()) { browser_context_(new TestBrowserContext()) {
// Configures the Conversion API to run in memory to speed up it's
// initialization and avoid timeouts. See https://crbug.com/1080764.
ConversionManagerImpl::RunInMemoryForTesting();
feature_list_.InitAndEnableFeature(features::kConversionMeasurement); feature_list_.InitAndEnableFeature(features::kConversionMeasurement);
} }
......
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