Commit b687eb30 authored by Charlie Harrison's avatar Charlie Harrison Committed by Commit Bot

Fix slow test CleanUpDatabaseWhenHistoryIsExpired

This test currently waits 30s before completing. This CL speeds it up
by injecting a TestMockTimeTaskRunner and using it to fast forward time.

Bug: 896787
Change-Id: I35cb9ea8f269bfa1a9d2cb35858e3fd697a41ac4
Reviewed-on: https://chromium-review.googlesource.com/c/1334289
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608465}
parent cc92f004
......@@ -11,6 +11,7 @@
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_clock.h"
#include "base/test/test_mock_time_task_runner.h"
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
......@@ -95,11 +96,14 @@ base::Time GetReferenceTime() {
}
std::unique_ptr<KeyedService> BuildTestHistoryService(
scoped_refptr<base::SequencedTaskRunner> backend_runner,
content::BrowserContext* context) {
std::unique_ptr<history::HistoryService> service(
new history::HistoryService());
if (backend_runner)
service->set_backend_task_runner_for_testing(std::move(backend_runner));
service->Init(history::TestHistoryDatabaseParamsForPath(g_temp_history_dir));
return std::move(service);
return service;
}
} // namespace
......@@ -107,13 +111,15 @@ std::unique_ptr<KeyedService> BuildTestHistoryService(
class MediaEngagementServiceTest : public ChromeRenderViewHostTestHarness {
public:
void SetUp() override {
mock_time_task_runner_ =
base::MakeRefCounted<base::TestMockTimeTaskRunner>();
scoped_feature_list_.InitAndEnableFeature(
media::kRecordMediaEngagementScores);
ChromeRenderViewHostTestHarness::SetUp();
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
g_temp_history_dir = temp_dir_.GetPath();
ConfigureHistoryService();
ConfigureHistoryService(nullptr);
test_clock_.SetNow(GetReferenceTime());
service_ = base::WrapUnique(StartNewMediaEngagementService());
......@@ -128,18 +134,21 @@ class MediaEngagementServiceTest : public ChromeRenderViewHostTestHarness {
return service;
}
void ConfigureHistoryService() {
void ConfigureHistoryService(
scoped_refptr<base::SequencedTaskRunner> backend_runner) {
HistoryServiceFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&BuildTestHistoryService));
profile(), base::BindRepeating(&BuildTestHistoryService,
std::move(backend_runner)));
}
void RestartHistoryService() {
void RestartHistoryService(
scoped_refptr<base::SequencedTaskRunner> backend_runner) {
history::HistoryService* history_old = HistoryServiceFactory::GetForProfile(
profile(), ServiceAccessType::IMPLICIT_ACCESS);
history_old->Shutdown();
HistoryServiceFactory::ShutdownForProfile(profile());
ConfigureHistoryService();
ConfigureHistoryService(std::move(backend_runner));
history::HistoryService* history = HistoryServiceFactory::GetForProfile(
profile(), ServiceAccessType::IMPLICIT_ACCESS);
history->AddObserver(service());
......@@ -251,6 +260,9 @@ class MediaEngagementServiceTest : public ChromeRenderViewHostTestHarness {
return GetAllStoredScores(service_.get());
}
protected:
scoped_refptr<base::TestMockTimeTaskRunner> mock_time_task_runner_;
private:
base::SimpleTestClock test_clock_;
......@@ -601,7 +613,12 @@ TEST_F(MediaEngagementServiceTest, CleanUpDatabaseWhenHistoryIsExpired) {
// Expire history older than |threshold|.
MediaEngagementChangeWaiter waiter(profile());
RestartHistoryService();
RestartHistoryService(mock_time_task_runner_);
// First, run the task that schedules backend initialization.
mock_time_task_runner_->RunUntilIdle();
// Now, fast forward time to ensure that the expiration job is completed. 30
// seconds is the value of kExpirationDelaySec.
mock_time_task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(30));
waiter.Wait();
// Check the scores for the test origins.
......
......@@ -18,8 +18,6 @@
#include "components/history/core/browser/history_service.h"
#include <utility>
#include "base/bind_helpers.h"
#include "base/callback.h"
#include "base/command_line.h"
......@@ -939,21 +937,23 @@ bool HistoryService::Init(
TRACE_EVENT0("browser,startup", "HistoryService::Init")
SCOPED_UMA_HISTOGRAM_TIMER("History.HistoryServiceInitTime");
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!backend_task_runner_);
if (thread_) {
base::Thread::Options options;
options.timer_slack = base::TIMER_SLACK_MAXIMUM;
if (!thread_->StartWithOptions(options)) {
Cleanup();
return false;
// Unit tests can inject |backend_task_runner_| before this is called.
if (!backend_task_runner_) {
if (thread_) {
base::Thread::Options options;
options.timer_slack = base::TIMER_SLACK_MAXIMUM;
if (!thread_->StartWithOptions(options)) {
Cleanup();
return false;
}
backend_task_runner_ = thread_->task_runner();
} else {
backend_task_runner_ = base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::WithBaseSyncPrimitives(),
base::TaskPriority::USER_BLOCKING,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN});
}
backend_task_runner_ = thread_->task_runner();
} else {
backend_task_runner_ = base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::WithBaseSyncPrimitives(),
base::TaskPriority::USER_BLOCKING,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN});
}
// Create the history backend.
......
......@@ -11,6 +11,7 @@
#include <memory>
#include <set>
#include <string>
#include <utility>
#include <vector>
#include "base/bind.h"
......@@ -524,6 +525,14 @@ class HistoryService : public syncer::SyncableService, public KeyedService {
std::unique_ptr<syncer::ModelTypeControllerDelegate>
GetTypedURLSyncControllerDelegate();
// Override |backend_task_runner_| for testing; needs to be called before
// Init.
void set_backend_task_runner_for_testing(
scoped_refptr<base::SequencedTaskRunner> task_runner) {
DCHECK(!backend_task_runner_);
backend_task_runner_ = std::move(task_runner);
}
protected:
// These are not currently used, hopefully we can do something in the future
// to ensure that the most important things happen first.
......
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