Commit a89d4e13 authored by Carlos Caballero's avatar Carlos Caballero Committed by Commit Bot

Remove MessageLoopForIO references in components/offline_pages

MessageLoopForIO is going away soon so get rid of it.
Also its replacement has support for mock time so there is no need for
base::TestMockTimeTaskRunner.

Bug: 891670

Change-Id: Ice3a129784431b73f9a9d6c9d4fef6bd1a619f97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1800309
Commit-Queue: Carlos Caballero <carlscab@google.com>
Reviewed-by: default avatarCarlos Knippschild <carlosk@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714242}
parent 758585d3
...@@ -6,9 +6,10 @@ ...@@ -6,9 +6,10 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/threading/thread_task_runner_handle.h"
namespace offline_pages { namespace offline_pages {
ModelTaskTestBase::ModelTaskTestBase() : store_test_util_(task_runner()) {} ModelTaskTestBase::ModelTaskTestBase() {}
ModelTaskTestBase::~ModelTaskTestBase() {} ModelTaskTestBase::~ModelTaskTestBase() {}
void ModelTaskTestBase::SetUp() { void ModelTaskTestBase::SetUp() {
...@@ -17,7 +18,8 @@ void ModelTaskTestBase::SetUp() { ...@@ -17,7 +18,8 @@ void ModelTaskTestBase::SetUp() {
ASSERT_TRUE(private_dir_.CreateUniqueTempDir()); ASSERT_TRUE(private_dir_.CreateUniqueTempDir());
ASSERT_TRUE(public_dir_.CreateUniqueTempDir()); ASSERT_TRUE(public_dir_.CreateUniqueTempDir());
archive_manager_ = std::make_unique<ArchiveManager>( archive_manager_ = std::make_unique<ArchiveManager>(
TemporaryDir(), PrivateDir(), PublicDir(), task_runner()); TemporaryDir(), PrivateDir(), PublicDir(),
base::ThreadTaskRunnerHandle::Get());
generator()->SetArchiveDirectory(TemporaryDir()); generator()->SetArchiveDirectory(TemporaryDir());
store_test_util_.BuildStoreInMemory(); store_test_util_.BuildStoreInMemory();
} }
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/mock_callback.h" #include "base/test/mock_callback.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "base/time/clock.h" #include "base/time/clock.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -87,7 +88,10 @@ class OfflinePageModelTaskifiedTest : public testing::Test, ...@@ -87,7 +88,10 @@ class OfflinePageModelTaskifiedTest : public testing::Test,
// Runs until all of the tasks that are not delayed are gone from the task // Runs until all of the tasks that are not delayed are gone from the task
// queue. // queue.
void PumpLoop() { task_runner_->RunUntilIdle(); } void PumpLoop() { task_environment_.RunUntilIdle(); }
void FastForwardBy(const base::TimeDelta delta) {
task_environment_.FastForwardBy(delta);
}
void BuildStore(); void BuildStore();
void BuildModel(); void BuildModel();
void ResetModel(); void ResetModel();
...@@ -147,8 +151,7 @@ class OfflinePageModelTaskifiedTest : public testing::Test, ...@@ -147,8 +151,7 @@ class OfflinePageModelTaskifiedTest : public testing::Test,
} }
// Getters for private fields. // Getters for private fields.
base::TestMockTimeTaskRunner* task_runner() { return task_runner_.get(); } const base::Clock* clock() { return task_environment_.GetMockClock(); }
base::Clock* clock() { return task_runner_->GetMockClock(); }
OfflinePageModelTaskified* model() { return model_.get(); } OfflinePageModelTaskified* model() { return model_.get(); }
OfflinePageMetadataStore* store() { return store_test_util_.store(); } OfflinePageMetadataStore* store() { return store_test_util_.store(); }
OfflinePageMetadataStoreTestUtil* store_test_util() { OfflinePageMetadataStoreTestUtil* store_test_util() {
...@@ -182,8 +185,8 @@ class OfflinePageModelTaskifiedTest : public testing::Test, ...@@ -182,8 +185,8 @@ class OfflinePageModelTaskifiedTest : public testing::Test,
OfflinePageTestArchivePublisher* publisher() { return publisher_; } OfflinePageTestArchivePublisher* publisher() { return publisher_; }
private: private:
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; base::test::SingleThreadTaskEnvironment task_environment_{
base::ThreadTaskRunnerHandle task_runner_handle_; base::test::TaskEnvironment::TimeSource::MOCK_TIME};
std::unique_ptr<OfflinePageModelTaskified> model_; std::unique_ptr<OfflinePageModelTaskified> model_;
OfflinePageMetadataStoreTestUtil store_test_util_; OfflinePageMetadataStoreTestUtil store_test_util_;
ArchiveManager* archive_manager_; ArchiveManager* archive_manager_;
...@@ -204,11 +207,7 @@ class OfflinePageModelTaskifiedTest : public testing::Test, ...@@ -204,11 +207,7 @@ class OfflinePageModelTaskifiedTest : public testing::Test,
}; };
OfflinePageModelTaskifiedTest::OfflinePageModelTaskifiedTest() OfflinePageModelTaskifiedTest::OfflinePageModelTaskifiedTest()
: task_runner_(new base::TestMockTimeTaskRunner(base::Time::Now(), : clock_(task_environment_.GetMockClock()) {}
base::TimeTicks::Now())),
task_runner_handle_(task_runner_),
store_test_util_(task_runner_),
clock_(task_runner_->GetMockClock()) {}
OfflinePageModelTaskifiedTest::~OfflinePageModelTaskifiedTest() {} OfflinePageModelTaskifiedTest::~OfflinePageModelTaskifiedTest() {}
...@@ -1307,10 +1306,8 @@ TEST_F(OfflinePageModelTaskifiedTest, MAYBE_StartupMaintenanceTaskExecuted) { ...@@ -1307,10 +1306,8 @@ TEST_F(OfflinePageModelTaskifiedTest, MAYBE_StartupMaintenanceTaskExecuted) {
// to trigger StartupMaintenanceTask execution. // to trigger StartupMaintenanceTask execution.
base::MockCallback<MultipleOfflinePageItemCallback> callback; base::MockCallback<MultipleOfflinePageItemCallback> callback;
model()->GetAllPages(callback.Get()); model()->GetAllPages(callback.Get());
task_runner()->FastForwardBy( FastForwardBy(OfflinePageModelTaskified::kMaintenanceTasksDelay +
OfflinePageModelTaskified::kMaintenanceTasksDelay + base::TimeDelta::FromMilliseconds(1));
base::TimeDelta::FromMilliseconds(1));
PumpLoop();
EXPECT_EQ(2LL, store_test_util()->GetPageCount()); EXPECT_EQ(2LL, store_test_util()->GetPageCount());
EXPECT_EQ(0UL, test_utils::GetFileCountInDirectory(temporary_dir_path())); EXPECT_EQ(0UL, test_utils::GetFileCountInDirectory(temporary_dir_path()));
...@@ -1321,8 +1318,7 @@ TEST_F(OfflinePageModelTaskifiedTest, MAYBE_StartupMaintenanceTaskExecuted) { ...@@ -1321,8 +1318,7 @@ TEST_F(OfflinePageModelTaskifiedTest, MAYBE_StartupMaintenanceTaskExecuted) {
TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
// The ClearStorage task should not be executed based on time delays after // The ClearStorage task should not be executed based on time delays after
// launch (aka the model being built). // launch (aka the model being built).
task_runner()->FastForwardBy(base::TimeDelta::FromDays(1)); FastForwardBy(base::TimeDelta::FromDays(1));
PumpLoop();
EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time()); EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time());
// GetAllPages should schedule a delayed task that will eventually run // GetAllPages should schedule a delayed task that will eventually run
...@@ -1341,8 +1337,7 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { ...@@ -1341,8 +1337,7 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
const base::TimeDelta run_delay = const base::TimeDelta run_delay =
OfflinePageModelTaskified::kMaintenanceTasksDelay + OfflinePageModelTaskified::kMaintenanceTasksDelay +
base::TimeDelta::FromMilliseconds(1); base::TimeDelta::FromMilliseconds(1);
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
EXPECT_EQ(last_scheduling_time, last_maintenance_tasks_schedule_time()); EXPECT_EQ(last_scheduling_time, last_maintenance_tasks_schedule_time());
// Check that CleanupVisualsTask ran. // Check that CleanupVisualsTask ran.
histogram_tester()->ExpectUniqueSample("OfflinePages.CleanupThumbnails.Count", histogram_tester()->ExpectUniqueSample("OfflinePages.CleanupThumbnails.Count",
...@@ -1351,13 +1346,12 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { ...@@ -1351,13 +1346,12 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
// Calling GetAllPages after only half of the enforced interval between // Calling GetAllPages after only half of the enforced interval between
// ClearStorage runs should not schedule ClearStorage. // ClearStorage runs should not schedule ClearStorage.
// Note: The previous elapsed delay is discounted from the clock advance here. // Note: The previous elapsed delay is discounted from the clock advance here.
task_runner()->FastForwardBy( FastForwardBy(OfflinePageModelTaskified::kClearStorageInterval / 2 -
OfflinePageModelTaskified::kClearStorageInterval / 2 - run_delay); run_delay);
ASSERT_GT(clock()->Now(), last_scheduling_time); ASSERT_GT(clock()->Now(), last_scheduling_time);
model()->GetAllPages(callback.Get()); model()->GetAllPages(callback.Get());
// And advance the delay too. // And advance the delay too.
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
EXPECT_EQ(last_scheduling_time, last_maintenance_tasks_schedule_time()); EXPECT_EQ(last_scheduling_time, last_maintenance_tasks_schedule_time());
// Confirm a single run happened so far. // Confirm a single run happened so far.
histogram_tester()->ExpectUniqueSample( histogram_tester()->ExpectUniqueSample(
...@@ -1366,9 +1360,8 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { ...@@ -1366,9 +1360,8 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
// Forwarding by the full interval (plus 1 second just in case) should allow // Forwarding by the full interval (plus 1 second just in case) should allow
// the task to be enqueued again. // the task to be enqueued again.
task_runner()->FastForwardBy( FastForwardBy(OfflinePageModelTaskified::kClearStorageInterval / 2 +
OfflinePageModelTaskified::kClearStorageInterval / 2 + base::TimeDelta::FromSeconds(1));
base::TimeDelta::FromSeconds(1));
// Saving a page should also immediately enqueue the ClearStorage task. // Saving a page should also immediately enqueue the ClearStorage task.
auto archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED); auto archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED);
SavePageWithExpectedResult(kTestUrl, kTestClientId1, kTestUrl2, SavePageWithExpectedResult(kTestUrl, kTestClientId1, kTestUrl2,
...@@ -1376,8 +1369,7 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { ...@@ -1376,8 +1369,7 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
SavePageResult::SUCCESS); SavePageResult::SUCCESS);
last_scheduling_time = clock()->Now(); last_scheduling_time = clock()->Now();
// Advance the delay again. // Advance the delay again.
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
EXPECT_EQ(last_scheduling_time, last_maintenance_tasks_schedule_time()); EXPECT_EQ(last_scheduling_time, last_maintenance_tasks_schedule_time());
// Confirm that two runs happened. // Confirm that two runs happened.
...@@ -1403,8 +1395,7 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) { ...@@ -1403,8 +1395,7 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) { TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) {
// The PersistentPageConsistencyCheckTask should not be executed based on time // The PersistentPageConsistencyCheckTask should not be executed based on time
// delays after launch (aka the model being built). // delays after launch (aka the model being built).
task_runner()->FastForwardBy(base::TimeDelta::FromDays(1)); FastForwardBy(base::TimeDelta::FromDays(1));
PumpLoop();
histogram_tester()->ExpectTotalCount( histogram_tester()->ExpectTotalCount(
"OfflinePages.ConsistencyCheck.Persistent.Result", 0); "OfflinePages.ConsistencyCheck.Persistent.Result", 0);
...@@ -1431,8 +1422,7 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) { ...@@ -1431,8 +1422,7 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) {
const base::TimeDelta run_delay = const base::TimeDelta run_delay =
OfflinePageModelTaskified::kMaintenanceTasksDelay + OfflinePageModelTaskified::kMaintenanceTasksDelay +
base::TimeDelta::FromMilliseconds(1); base::TimeDelta::FromMilliseconds(1);
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
// But nothing should change. // But nothing should change.
EXPECT_EQ(1UL, EXPECT_EQ(1UL,
test_utils::GetFileCountInDirectory(public_archive_dir_path())); test_utils::GetFileCountInDirectory(public_archive_dir_path()));
...@@ -1447,12 +1437,11 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) { ...@@ -1447,12 +1437,11 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) {
// Calling GetAllPages after only half of the enforced interval between // Calling GetAllPages after only half of the enforced interval between
// consistency check runs should not schedule the task. // consistency check runs should not schedule the task.
// Note: The previous elapsed delay is discounted from the clock advance here. // Note: The previous elapsed delay is discounted from the clock advance here.
task_runner()->FastForwardBy( FastForwardBy(OfflinePageModelTaskified::kClearStorageInterval / 2 -
OfflinePageModelTaskified::kClearStorageInterval / 2 - run_delay); run_delay);
model()->GetAllPages(callback.Get()); model()->GetAllPages(callback.Get());
// And advance the delay too. // And advance the delay too.
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
// Confirm no persistent page consistency check is executed. // Confirm no persistent page consistency check is executed.
histogram_tester()->ExpectTotalCount( histogram_tester()->ExpectTotalCount(
"OfflinePages.ConsistencyCheck.Persistent.Result", 1); "OfflinePages.ConsistencyCheck.Persistent.Result", 1);
...@@ -1460,13 +1449,11 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) { ...@@ -1460,13 +1449,11 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) {
// Forwarding by the full interval (plus 1 second just in case) should allow // Forwarding by the full interval (plus 1 second just in case) should allow
// the task to be enqueued again and call GetAllPages again to enqueue the // the task to be enqueued again and call GetAllPages again to enqueue the
// task. // task.
task_runner()->FastForwardBy( FastForwardBy(OfflinePageModelTaskified::kClearStorageInterval / 2 +
OfflinePageModelTaskified::kClearStorageInterval / 2 + base::TimeDelta::FromSeconds(1));
base::TimeDelta::FromSeconds(1));
model()->GetAllPages(callback.Get()); model()->GetAllPages(callback.Get());
// And advance the delay too. // And advance the delay too.
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
// Confirm persistent page consistency check is executed, and the page is // Confirm persistent page consistency check is executed, and the page is
// marked as missing file. // marked as missing file.
EXPECT_EQ(0UL, EXPECT_EQ(0UL,
...@@ -1480,15 +1467,14 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) { ...@@ -1480,15 +1467,14 @@ TEST_F(OfflinePageModelTaskifiedTest, PersistentPageConsistencyCheckExecuted) {
// Forwarding by a long time that is enough for the page with missing file to // Forwarding by a long time that is enough for the page with missing file to
// get expired. // get expired.
task_runner()->FastForwardBy(base::TimeDelta::FromDays(400)); FastForwardBy(base::TimeDelta::FromDays(400));
// Saving a page should also immediately enqueue the consistency check task. // Saving a page should also immediately enqueue the consistency check task.
auto archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED); auto archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED);
SavePageWithExpectedResult(kTestUrl, kTestClientId1, kTestUrl2, SavePageWithExpectedResult(kTestUrl, kTestClientId1, kTestUrl2,
kEmptyRequestOrigin, std::move(archiver), kEmptyRequestOrigin, std::move(archiver),
SavePageResult::SUCCESS); SavePageResult::SUCCESS);
// Advance the delay to activate task execution. // Advance the delay to activate task execution.
task_runner()->FastForwardBy(run_delay); FastForwardBy(run_delay);
PumpLoop();
// Confirm persistent page consistency check is executed, and the page is // Confirm persistent page consistency check is executed, and the page is
// deleted from database, also notified system download manager. // deleted from database, also notified system download manager.
EXPECT_EQ(0UL, EXPECT_EQ(0UL,
...@@ -1516,8 +1502,7 @@ TEST_F(OfflinePageModelTaskifiedTest, MaintenanceTasksAreDisabled) { ...@@ -1516,8 +1502,7 @@ TEST_F(OfflinePageModelTaskifiedTest, MaintenanceTasksAreDisabled) {
EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time()); EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time());
// Advance the clock considerably and confirm no runs happened. // Advance the clock considerably and confirm no runs happened.
task_runner()->FastForwardBy(base::TimeDelta::FromDays(1)); FastForwardBy(base::TimeDelta::FromDays(1));
PumpLoop();
EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time()); EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time());
histogram_tester()->ExpectTotalCount( histogram_tester()->ExpectTotalCount(
"OfflinePages.ClearTemporaryPages.Result", 0); "OfflinePages.ClearTemporaryPages.Result", 0);
......
...@@ -9,6 +9,9 @@ ...@@ -9,6 +9,9 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/run_loop.h"
#include "base/test/bind_test_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/offline_pages/core/model/add_page_task.h" #include "components/offline_pages/core/model/add_page_task.h"
#include "components/offline_pages/core/model/get_pages_task.h" #include "components/offline_pages/core/model/get_pages_task.h"
#include "components/offline_pages/core/offline_page_types.h" #include "components/offline_pages/core/offline_page_types.h"
...@@ -31,9 +34,8 @@ int64_t GetPageCountSync(sql::Database* db) { ...@@ -31,9 +34,8 @@ int64_t GetPageCountSync(sql::Database* db) {
} // namespace } // namespace
OfflinePageMetadataStoreTestUtil::OfflinePageMetadataStoreTestUtil( OfflinePageMetadataStoreTestUtil::OfflinePageMetadataStoreTestUtil()
scoped_refptr<base::TestMockTimeTaskRunner> task_runner) : store_ptr_(nullptr) {}
: task_runner_(task_runner), store_ptr_(nullptr) {}
OfflinePageMetadataStoreTestUtil::~OfflinePageMetadataStoreTestUtil() {} OfflinePageMetadataStoreTestUtil::~OfflinePageMetadataStoreTestUtil() {}
...@@ -43,20 +45,20 @@ void OfflinePageMetadataStoreTestUtil::BuildStore() { ...@@ -43,20 +45,20 @@ void OfflinePageMetadataStoreTestUtil::BuildStore() {
return; return;
} }
store_.reset( store_.reset(new OfflinePageMetadataStore(base::ThreadTaskRunnerHandle::Get(),
new OfflinePageMetadataStore(task_runner_, temp_directory_.GetPath())); temp_directory_.GetPath()));
store_ptr_ = store_.get(); store_ptr_ = store_.get();
} }
void OfflinePageMetadataStoreTestUtil::BuildStoreInMemory() { void OfflinePageMetadataStoreTestUtil::BuildStoreInMemory() {
store_.reset(new OfflinePageMetadataStore(task_runner_)); store_.reset(
new OfflinePageMetadataStore(base::ThreadTaskRunnerHandle::Get()));
store_ptr_ = store_.get(); store_ptr_ = store_.get();
} }
void OfflinePageMetadataStoreTestUtil::DeleteStore() { void OfflinePageMetadataStoreTestUtil::DeleteStore() {
store_.reset(); store_.reset();
store_ptr_ = nullptr; store_ptr_ = nullptr;
task_runner_->FastForwardUntilNoTasksRemain();
} }
std::unique_ptr<OfflinePageMetadataStore> std::unique_ptr<OfflinePageMetadataStore>
...@@ -65,45 +67,48 @@ OfflinePageMetadataStoreTestUtil::ReleaseStore() { ...@@ -65,45 +67,48 @@ OfflinePageMetadataStoreTestUtil::ReleaseStore() {
} }
void OfflinePageMetadataStoreTestUtil::InsertItem(const OfflinePageItem& page) { void OfflinePageMetadataStoreTestUtil::InsertItem(const OfflinePageItem& page) {
base::RunLoop run_loop;
AddPageResult result; AddPageResult result;
auto task = std::make_unique<AddPageTask>( auto task = std::make_unique<AddPageTask>(
store(), page, store(), page, base::BindLambdaForTesting([&](AddPageResult cb_result) {
base::BindOnce([](AddPageResult* out_result, result = cb_result;
AddPageResult cb_result) { *out_result = cb_result; }, run_loop.Quit();
&result)); }));
task->Run(); task->Run();
task_runner_->RunUntilIdle(); run_loop.Run();
EXPECT_EQ(AddPageResult::SUCCESS, result); EXPECT_EQ(AddPageResult::SUCCESS, result);
} }
int64_t OfflinePageMetadataStoreTestUtil::GetPageCount() { int64_t OfflinePageMetadataStoreTestUtil::GetPageCount() {
base::RunLoop run_loop;
int64_t count = 0; int64_t count = 0;
store()->Execute( store()->Execute(
base::BindOnce(&GetPageCountSync), base::BindOnce(&GetPageCountSync),
base::BindOnce( base::BindOnce(base::BindLambdaForTesting([&](int64_t cb_count) {
[](int64_t* out_count, int64_t cb_count) { *out_count = cb_count; }, count = cb_count;
&count), run_loop.Quit();
})),
int64_t()); int64_t());
task_runner_->RunUntilIdle(); run_loop.Run();
return count; return count;
} }
std::unique_ptr<OfflinePageItem> std::unique_ptr<OfflinePageItem>
OfflinePageMetadataStoreTestUtil::GetPageByOfflineId(int64_t offline_id) { OfflinePageMetadataStoreTestUtil::GetPageByOfflineId(int64_t offline_id) {
base::RunLoop run_loop;
PageCriteria criteria; PageCriteria criteria;
criteria.offline_ids = std::vector<int64_t>{offline_id}; criteria.offline_ids = std::vector<int64_t>{offline_id};
OfflinePageItem* page = nullptr; OfflinePageItem* page = nullptr;
auto task = std::make_unique<GetPagesTask>( auto task = std::make_unique<GetPagesTask>(
store(), criteria, store(), criteria,
base::BindOnce( base::BindOnce(base::BindLambdaForTesting(
[](OfflinePageItem** out_page, [&](const std::vector<OfflinePageItem>& cb_pages) {
const std::vector<OfflinePageItem>& cb_pages) {
if (!cb_pages.empty()) if (!cb_pages.empty())
*out_page = new OfflinePageItem(cb_pages[0]); page = new OfflinePageItem(cb_pages[0]);
}, run_loop.Quit();
&page)); })));
task->Run(); task->Run();
task_runner_->RunUntilIdle(); run_loop.Run();
return base::WrapUnique<OfflinePageItem>(page); return base::WrapUnique<OfflinePageItem>(page);
} }
......
...@@ -25,8 +25,7 @@ namespace offline_pages { ...@@ -25,8 +25,7 @@ namespace offline_pages {
// operations on the store, for test writing convenience. // operations on the store, for test writing convenience.
class OfflinePageMetadataStoreTestUtil { class OfflinePageMetadataStoreTestUtil {
public: public:
explicit OfflinePageMetadataStoreTestUtil( OfflinePageMetadataStoreTestUtil();
scoped_refptr<base::TestMockTimeTaskRunner> task_runner);
~OfflinePageMetadataStoreTestUtil(); ~OfflinePageMetadataStoreTestUtil();
// Builds a new store in a temporary directory. // Builds a new store in a temporary directory.
...@@ -54,9 +53,6 @@ class OfflinePageMetadataStoreTestUtil { ...@@ -54,9 +53,6 @@ class OfflinePageMetadataStoreTestUtil {
base::SimpleTestClock* clock() { return &clock_; } base::SimpleTestClock* clock() { return &clock_; }
private: private:
void RunUntilIdle();
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
base::ScopedTempDir temp_directory_; base::ScopedTempDir temp_directory_;
// TODO(romax): Refactor the test util along with the similar one used in // TODO(romax): Refactor the test util along with the similar one used in
// Prefetching, to remove the ownership to the store. And clean up related // Prefetching, to remove the ownership to the store. And clean up related
......
...@@ -12,8 +12,13 @@ ...@@ -12,8 +12,13 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/message_loop/message_loop_current.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "components/image_fetcher/core/mock_image_fetcher.h" #include "components/image_fetcher/core/mock_image_fetcher.h"
#include "components/image_fetcher/core/request_metadata.h" #include "components/image_fetcher/core/request_metadata.h"
#include "components/offline_pages/core/client_namespace_constants.h" #include "components/offline_pages/core/client_namespace_constants.h"
...@@ -330,7 +335,7 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase { ...@@ -330,7 +335,7 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase {
offline_model_ = model.get(); offline_model_ = model.get();
taco_->SetOfflinePageModel(std::move(model)); taco_->SetOfflinePageModel(std::move(model));
taco_->SetPrefetchImporter(std::make_unique<PrefetchImporterImpl>( taco_->SetPrefetchImporter(std::make_unique<PrefetchImporterImpl>(
dispatcher_, offline_model_, task_runner())); dispatcher_, offline_model_, base::ThreadTaskRunnerHandle::Get()));
taco_->CreatePrefetchService(); taco_->CreatePrefetchService();
...@@ -389,7 +394,7 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase { ...@@ -389,7 +394,7 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase {
.WillOnce([&, thumbnail_data]( .WillOnce([&, thumbnail_data](
const ClientId& client_id, const ClientId& client_id,
ThumbnailFetcher::ImageDataFetchedCallback callback) { ThumbnailFetcher::ImageDataFetchedCallback callback) {
task_runner()->PostTask( base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), thumbnail_data)); FROM_HERE, base::BindOnce(std::move(callback), thumbnail_data));
}); });
} }
...@@ -445,7 +450,7 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase { ...@@ -445,7 +450,7 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase {
MockThumbnailFetcher* thumbnail_fetcher_; MockThumbnailFetcher* thumbnail_fetcher_;
image_fetcher::MockImageFetcher* thumbnail_image_fetcher_; image_fetcher::MockImageFetcher* thumbnail_image_fetcher_;
PrefetchStoreTestUtil store_util_{task_runner()}; PrefetchStoreTestUtil store_util_;
MockPrefetchItemGenerator item_generator_; MockPrefetchItemGenerator item_generator_;
base::ScopedTempDir archive_directory_; base::ScopedTempDir archive_directory_;
std::unique_ptr<FakeSuggestionsProvider> suggestions_provider_; std::unique_ptr<FakeSuggestionsProvider> suggestions_provider_;
...@@ -652,10 +657,26 @@ TEST_F(PrefetchDispatcherTest, DispatcherReleasesBackgroundTask) { ...@@ -652,10 +657,26 @@ TEST_F(PrefetchDispatcherTest, DispatcherReleasesBackgroundTask) {
EXPECT_THAT(*network_request_factory()->GetAllUrlsRequested(), EXPECT_THAT(*network_request_factory()->GetAllUrlsRequested(),
Contains(prefetch_url.url.spec())); Contains(prefetch_url.url.spec()));
// When the network request finishes, the dispatcher should still hold the // We want to make sure the response is received before the dispatcher goes
// ScopedBackgroundTask because it needs to process the results of the // for the next task. For that we need to make sure that only file handle
// request. // events (and no regular tasks) get processed by the RunLoop().RunUntilIdle()
RespondWithHttpError(net::HTTP_INTERNAL_SERVER_ERROR); // call done inside of RespondWithNetError. This can be acomplished by turning
// that RunLoop into a nested one (which would only run system tasks). By
// posting a task that makes the RespondWithNetError call we will already be
// running a RunLoop when the call happens thus turning the
// RespondWithNetError RunLoop into a nested one.
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindLambdaForTesting([&]() {
// When the network request finishes, the dispatcher should still hold
// the ScopedBackgroundTask because it needs to process the results of
// the request
RespondWithHttpError(net::HTTP_INTERNAL_SERVER_ERROR);
// Stop right after the error is processed, so that we can check
// GetBackgroundTask()
run_loop.Quit();
}));
run_loop.Run();
EXPECT_NE(nullptr, GetBackgroundTask()); EXPECT_NE(nullptr, GetBackgroundTask());
RunUntilIdle(); RunUntilIdle();
...@@ -747,8 +768,19 @@ TEST_F(PrefetchDispatcherTest, SuspendAfterFailedNetworkRequest) { ...@@ -747,8 +768,19 @@ TEST_F(PrefetchDispatcherTest, SuspendAfterFailedNetworkRequest) {
EXPECT_FALSE(dispatcher_suspended()); EXPECT_FALSE(dispatcher_suspended());
// This should trigger suspend. // We want to make sure the response is received before the dispatcher goes
RespondWithNetError(net::ERR_BLOCKED_BY_ADMINISTRATOR); // for the next task. For that we need to make sure that only file handle
// events (and no regular tasks) get processed by the RunLoop().RunUntilIdle()
// call done inside of RespondWithNetError. This can be acomplished by turning
// that RunLoop into a nested one (which would only run system tasks). By
// posting a task that makes the RespondWithNetError call we will already be
// running a RunLoop when the call happens thus turning the
// RespondWithNetError RunLoop into a nested one.
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindLambdaForTesting([this]() {
// This should trigger suspend.
RespondWithNetError(net::ERR_BLOCKED_BY_ADMINISTRATOR);
}));
RunUntilIdle(); RunUntilIdle();
EXPECT_TRUE(reschedule_called()); EXPECT_TRUE(reschedule_called());
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/metrics/field_trial_params.h" #include "base/metrics/field_trial_params.h"
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "base/test/mock_entropy_provider.h" #include "base/test/mock_entropy_provider.h"
#include "base/test/task_environment.h"
#include "components/offline_pages/core/offline_page_feature.h" #include "components/offline_pages/core/offline_page_feature.h"
#include "components/offline_pages/core/prefetch/prefetch_server_urls.h" #include "components/offline_pages/core/prefetch/prefetch_server_urls.h"
#include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_fetcher_delegate.h"
...@@ -21,12 +22,9 @@ const char PrefetchRequestTestBase::kExperimentValueSetInFieldTrial[] = ...@@ -21,12 +22,9 @@ const char PrefetchRequestTestBase::kExperimentValueSetInFieldTrial[] =
"Test Experiment"; "Test Experiment";
PrefetchRequestTestBase::PrefetchRequestTestBase() PrefetchRequestTestBase::PrefetchRequestTestBase()
: task_runner_(new base::TestMockTimeTaskRunner), : test_shared_url_loader_factory_(
test_shared_url_loader_factory_(
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>( base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)) { &test_url_loader_factory_)) {}
message_loop_.SetTaskRunner(task_runner_);
}
PrefetchRequestTestBase::~PrefetchRequestTestBase() {} PrefetchRequestTestBase::~PrefetchRequestTestBase() {}
...@@ -100,11 +98,15 @@ std::string PrefetchRequestTestBase::GetExperiementHeaderValue( ...@@ -100,11 +98,15 @@ std::string PrefetchRequestTestBase::GetExperiementHeaderValue(
} }
void PrefetchRequestTestBase::RunUntilIdle() { void PrefetchRequestTestBase::RunUntilIdle() {
task_runner_->RunUntilIdle(); task_environment_.RunUntilIdle();
}
void PrefetchRequestTestBase::FastForwardBy(base::TimeDelta delta) {
task_environment_.FastForwardBy(delta);
} }
void PrefetchRequestTestBase::FastForwardUntilNoTasksRemain() { void PrefetchRequestTestBase::FastForwardUntilNoTasksRemain() {
task_runner_->FastForwardUntilNoTasksRemain(); task_environment_.FastForwardUntilNoTasksRemain();
} }
} // namespace offline_pages } // namespace offline_pages
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h" #include "services/network/test/test_url_loader_factory.h"
...@@ -45,17 +46,13 @@ class PrefetchRequestTestBase : public testing::Test { ...@@ -45,17 +46,13 @@ class PrefetchRequestTestBase : public testing::Test {
} }
void RunUntilIdle(); void RunUntilIdle();
void FastForwardBy(base::TimeDelta delta);
void FastForwardUntilNoTasksRemain(); void FastForwardUntilNoTasksRemain();
protected:
// Derived classes may need these to construct other members.
scoped_refptr<base::TestMockTimeTaskRunner> task_runner() {
return task_runner_;
}
private: private:
base::MessageLoopForIO message_loop_; base::test::SingleThreadTaskEnvironment task_environment_{
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; base::test::TaskEnvironment::MainThreadType::IO,
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
network::TestURLLoaderFactory test_url_loader_factory_; network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> scoped_refptr<network::SharedURLLoaderFactory>
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "components/offline_pages/core/prefetch/store/prefetch_downloader_quota.h" #include "components/offline_pages/core/prefetch/store/prefetch_downloader_quota.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "components/offline_pages/core/offline_page_feature.h" #include "components/offline_pages/core/offline_page_feature.h"
...@@ -23,12 +24,8 @@ const char kInvalidText[] = "tacos"; ...@@ -23,12 +24,8 @@ const char kInvalidText[] = "tacos";
class PrefetchDownloaderQuotaTest : public testing::Test { class PrefetchDownloaderQuotaTest : public testing::Test {
public: public:
PrefetchDownloaderQuotaTest(); PrefetchDownloaderQuotaTest() { store_test_util_.BuildStoreInMemory(); }
~PrefetchDownloaderQuotaTest() override = default; ~PrefetchDownloaderQuotaTest() override { store_test_util_.DeleteStore(); }
void SetUp() override { store_test_util_.BuildStoreInMemory(); }
void TearDown() override { store_test_util_.DeleteStore(); }
PrefetchStore* store() { return store_test_util_.store(); } PrefetchStore* store() { return store_test_util_.store(); }
...@@ -37,17 +34,11 @@ class PrefetchDownloaderQuotaTest : public testing::Test { ...@@ -37,17 +34,11 @@ class PrefetchDownloaderQuotaTest : public testing::Test {
void SetTestingMaxDailyQuotaBytes(const std::string& max_config); void SetTestingMaxDailyQuotaBytes(const std::string& max_config);
private: private:
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; base::test::SingleThreadTaskEnvironment task_environment_;
base::ThreadTaskRunnerHandle task_runner_handle_;
PrefetchStoreTestUtil store_test_util_; PrefetchStoreTestUtil store_test_util_;
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
}; };
PrefetchDownloaderQuotaTest::PrefetchDownloaderQuotaTest()
: task_runner_(new base::TestMockTimeTaskRunner),
task_runner_handle_(task_runner_),
store_test_util_(task_runner_) {}
void PrefetchDownloaderQuotaTest::SetTestingMaxDailyQuotaBytes( void PrefetchDownloaderQuotaTest::SetTestingMaxDailyQuotaBytes(
const std::string& max_config) { const std::string& max_config) {
scoped_feature_list_.Reset(); scoped_feature_list_.Reset();
......
...@@ -6,7 +6,10 @@ ...@@ -6,7 +6,10 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/scoped_refptr.h"
#include "base/run_loop.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/test/bind_test_util.h"
#include "base/test/simple_test_clock.h" #include "base/test/simple_test_clock.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/time/clock.h" #include "base/time/clock.h"
...@@ -199,9 +202,7 @@ bool SetPrefetchQuotaSync(int64_t available_quota, ...@@ -199,9 +202,7 @@ bool SetPrefetchQuotaSync(int64_t available_quota,
} // namespace } // namespace
PrefetchStoreTestUtil::PrefetchStoreTestUtil( PrefetchStoreTestUtil::PrefetchStoreTestUtil() = default;
scoped_refptr<base::TestMockTimeTaskRunner> task_runner)
: task_runner_(task_runner) {}
PrefetchStoreTestUtil::~PrefetchStoreTestUtil() = default; PrefetchStoreTestUtil::~PrefetchStoreTestUtil() = default;
...@@ -209,13 +210,13 @@ void PrefetchStoreTestUtil::BuildStore() { ...@@ -209,13 +210,13 @@ void PrefetchStoreTestUtil::BuildStore() {
if (!temp_directory_.CreateUniqueTempDir()) if (!temp_directory_.CreateUniqueTempDir())
DVLOG(1) << "temp_directory_ not created"; DVLOG(1) << "temp_directory_ not created";
owned_store_.reset( owned_store_.reset(new PrefetchStore(base::ThreadTaskRunnerHandle::Get(),
new PrefetchStore(task_runner_, temp_directory_.GetPath())); temp_directory_.GetPath()));
store_ = owned_store_.get(); store_ = owned_store_.get();
} }
void PrefetchStoreTestUtil::BuildStoreInMemory() { void PrefetchStoreTestUtil::BuildStoreInMemory() {
owned_store_.reset(new PrefetchStore(task_runner_)); owned_store_.reset(new PrefetchStore(base::ThreadTaskRunnerHandle::Get()));
store_ = owned_store_.get(); store_ = owned_store_.get();
} }
...@@ -229,42 +230,52 @@ void PrefetchStoreTestUtil::DeleteStore() { ...@@ -229,42 +230,52 @@ void PrefetchStoreTestUtil::DeleteStore() {
if (!temp_directory_.Delete()) if (!temp_directory_.Delete())
DVLOG(1) << "temp_directory_ not created"; DVLOG(1) << "temp_directory_ not created";
} }
task_runner_->FastForwardUntilNoTasksRemain(); // The actual deletion happens in a task. So wait until all have been
// processed.
base::RunLoop().RunUntilIdle();
} }
bool PrefetchStoreTestUtil::InsertPrefetchItem(const PrefetchItem& item) { bool PrefetchStoreTestUtil::InsertPrefetchItem(const PrefetchItem& item) {
base::RunLoop run_loop;
bool success = false; bool success = false;
store_->Execute( store_->Execute(base::BindOnce(&InsertPrefetchItemSync, item),
base::BindOnce(&InsertPrefetchItemSync, item), base::BindOnce(base::BindLambdaForTesting([&](bool s) {
base::BindOnce([](bool* alias, bool s) { *alias = s; }, &success), false); success = s;
RunUntilIdle(); run_loop.Quit();
})),
false);
run_loop.Run();
return success; return success;
} }
int PrefetchStoreTestUtil::CountPrefetchItems() { int PrefetchStoreTestUtil::CountPrefetchItems() {
base::RunLoop run_loop;
int count = 0; int count = 0;
store_->Execute( store_->Execute(base::BindOnce(&CountPrefetchItemsSync),
base::BindOnce(&CountPrefetchItemsSync), base::BindOnce(base::BindLambdaForTesting([&](int result) {
base::BindOnce([](int* alias, int result) { *alias = result; }, &count), count = result;
kPrefetchStoreCommandFailed); run_loop.Quit();
RunUntilIdle(); })),
kPrefetchStoreCommandFailed);
run_loop.Run();
return count; return count;
} }
std::unique_ptr<PrefetchItem> PrefetchStoreTestUtil::GetPrefetchItem( std::unique_ptr<PrefetchItem> PrefetchStoreTestUtil::GetPrefetchItem(
int64_t offline_id) { int64_t offline_id) {
base::RunLoop run_loop;
std::unique_ptr<PrefetchItem> item; std::unique_ptr<PrefetchItem> item;
store_->Execute(base::BindOnce(&GetPrefetchItemSync, offline_id), store_->Execute(
base::BindOnce( base::BindOnce(&GetPrefetchItemSync, offline_id),
[](std::unique_ptr<PrefetchItem>* alias, base::BindOnce(
base::Optional<PrefetchItem> result) { base::BindLambdaForTesting([&](base::Optional<PrefetchItem> result) {
if (result) if (result) {
*alias = std::make_unique<PrefetchItem>( item = std::make_unique<PrefetchItem>(std::move(result).value());
std::move(result).value()); }
}, run_loop.Quit();
&item), })),
base::Optional<PrefetchItem>()); base::Optional<PrefetchItem>());
RunUntilIdle(); run_loop.Run();
return item; return item;
} }
...@@ -276,16 +287,16 @@ std::size_t PrefetchStoreTestUtil::GetAllItems( ...@@ -276,16 +287,16 @@ std::size_t PrefetchStoreTestUtil::GetAllItems(
} }
std::set<PrefetchItem> PrefetchStoreTestUtil::GetAllItems() { std::set<PrefetchItem> PrefetchStoreTestUtil::GetAllItems() {
base::RunLoop run_loop;
std::set<PrefetchItem> items; std::set<PrefetchItem> items;
store_->Execute( store_->Execute(base::BindOnce(&GetAllItemsSync),
base::BindOnce(&GetAllItemsSync), base::BindOnce(base::BindLambdaForTesting(
base::BindOnce( [&](std::set<PrefetchItem> result) {
[](std::set<PrefetchItem>* alias, std::set<PrefetchItem> result) { items = std::move(result);
*alias = std::move(result); run_loop.Quit();
}, })),
&items), std::set<PrefetchItem>());
std::set<PrefetchItem>()); run_loop.Run();
RunUntilIdle();
return items; return items;
} }
...@@ -303,51 +314,58 @@ std::string PrefetchStoreTestUtil::ToString() { ...@@ -303,51 +314,58 @@ std::string PrefetchStoreTestUtil::ToString() {
int PrefetchStoreTestUtil::ZombifyPrefetchItems(const std::string& name_space, int PrefetchStoreTestUtil::ZombifyPrefetchItems(const std::string& name_space,
const GURL& url) { const GURL& url) {
base::RunLoop run_loop;
int count = -1; int count = -1;
store_->Execute( store_->Execute(base::BindOnce(&UpdateItemsStateSync, name_space, url.spec(),
base::BindOnce(&UpdateItemsStateSync, name_space, url.spec(), PrefetchItemState::ZOMBIE),
PrefetchItemState::ZOMBIE), base::BindOnce(base::BindLambdaForTesting([&](int result) {
base::BindOnce([](int* alias, int result) { *alias = result; }, &count), count = result;
kPrefetchStoreCommandFailed); run_loop.Quit();
RunUntilIdle(); })),
kPrefetchStoreCommandFailed);
run_loop.Run();
return count; return count;
} }
void PrefetchStoreTestUtil::RunUntilIdle() {
task_runner_->RunUntilIdle();
}
int PrefetchStoreTestUtil::LastCommandChangeCount() { int PrefetchStoreTestUtil::LastCommandChangeCount() {
base::RunLoop run_loop;
int count = 0; int count = 0;
store_->Execute( store_->Execute(base::BindOnce([](sql::Database* connection) {
base::BindOnce([](sql::Database* connection) { return connection->GetLastChangeCount();
return connection->GetLastChangeCount(); }),
}), base::BindOnce(base::BindLambdaForTesting([&](int result) {
base::BindOnce([](int* result, int count) { *result = count; }, &count), count = result;
0); run_loop.Quit();
RunUntilIdle(); })),
0);
run_loop.Run();
return count; return count;
} }
int64_t PrefetchStoreTestUtil::GetPrefetchQuota() { int64_t PrefetchStoreTestUtil::GetPrefetchQuota() {
base::RunLoop run_loop;
int64_t result; int64_t result;
store_->Execute( store_->Execute(base::BindOnce(&GetPrefetchQuotaSync, clock()),
base::BindOnce(&GetPrefetchQuotaSync, clock()), base::BindOnce(base::BindLambdaForTesting([&](int64_t quota) {
base::BindOnce([](int64_t* result, int64_t quota) { *result = quota; }, result = quota;
&result), run_loop.Quit();
int64_t()); })),
RunUntilIdle(); int64_t());
run_loop.Run();
return result; return result;
} }
bool PrefetchStoreTestUtil::SetPrefetchQuota(int64_t available_quota) { bool PrefetchStoreTestUtil::SetPrefetchQuota(int64_t available_quota) {
base::RunLoop run_loop;
bool result; bool result;
store_->Execute( store_->Execute(
base::BindOnce(&SetPrefetchQuotaSync, available_quota, clock()), base::BindOnce(&SetPrefetchQuotaSync, available_quota, clock()),
base::BindOnce([](bool* result, bool success) { *result = success; }, base::BindOnce(base::BindLambdaForTesting([&](bool success) {
&result), result = success;
run_loop.Quit();
})),
false); false);
RunUntilIdle(); run_loop.Run();
return result; return result;
} }
......
...@@ -34,8 +34,7 @@ extern const int kPrefetchStoreCommandFailed; ...@@ -34,8 +34,7 @@ extern const int kPrefetchStoreCommandFailed;
// store, for test writing convenience. // store, for test writing convenience.
class PrefetchStoreTestUtil { class PrefetchStoreTestUtil {
public: public:
explicit PrefetchStoreTestUtil( PrefetchStoreTestUtil();
scoped_refptr<base::TestMockTimeTaskRunner> task_runner);
~PrefetchStoreTestUtil(); ~PrefetchStoreTestUtil();
// Builds a new store in a temporary directory. // Builds a new store in a temporary directory.
...@@ -88,9 +87,6 @@ class PrefetchStoreTestUtil { ...@@ -88,9 +87,6 @@ class PrefetchStoreTestUtil {
base::SimpleTestClock* clock() { return &clock_; } base::SimpleTestClock* clock() { return &clock_; }
private: private:
void RunUntilIdle();
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
base::ScopedTempDir temp_directory_; base::ScopedTempDir temp_directory_;
// TODO(jianli): Refactor this class to avoid owning the store. // TODO(jianli): Refactor this class to avoid owning the store.
std::unique_ptr<PrefetchStore> owned_store_; std::unique_ptr<PrefetchStore> owned_store_;
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "components/offline_pages/core/prefetch/store/prefetch_store.h" #include "components/offline_pages/core/prefetch/store/prefetch_store.h"
#include "base/test/task_environment.h"
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "components/offline_pages/core/offline_store_utils.h" #include "components/offline_pages/core/offline_store_utils.h"
...@@ -22,33 +23,24 @@ using InitializationStatus = SqlStoreBase::InitializationStatus; ...@@ -22,33 +23,24 @@ using InitializationStatus = SqlStoreBase::InitializationStatus;
class PrefetchStoreTest : public testing::Test { class PrefetchStoreTest : public testing::Test {
public: public:
PrefetchStoreTest(); PrefetchStoreTest() { store_test_util_.BuildStoreInMemory(); }
~PrefetchStoreTest() override = default;
void SetUp() override { store_test_util_.BuildStoreInMemory(); } ~PrefetchStoreTest() override { store_test_util_.DeleteStore(); }
void TearDown() override {
store_test_util_.DeleteStore();
}
PrefetchStore* store() { return store_test_util_.store(); } PrefetchStore* store() { return store_test_util_.store(); }
PrefetchStoreTestUtil* store_util() { return &store_test_util_; } PrefetchStoreTestUtil* store_util() { return &store_test_util_; }
MockPrefetchItemGenerator* item_generator() { return &item_generator_; } MockPrefetchItemGenerator* item_generator() { return &item_generator_; }
base::TestMockTimeTaskRunner* task_runner() { return task_runner_.get(); }
protected:
base::test::SingleThreadTaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
private: private:
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
base::ThreadTaskRunnerHandle task_runner_handle_;
PrefetchStoreTestUtil store_test_util_; PrefetchStoreTestUtil store_test_util_;
MockPrefetchItemGenerator item_generator_; MockPrefetchItemGenerator item_generator_;
}; };
PrefetchStoreTest::PrefetchStoreTest()
: task_runner_(new base::TestMockTimeTaskRunner),
task_runner_handle_(task_runner_),
store_test_util_(task_runner_) {}
TEST_F(PrefetchStoreTest, InitializeStore) { TEST_F(PrefetchStoreTest, InitializeStore) {
EXPECT_EQ(0, store_util()->CountPrefetchItems()); EXPECT_EQ(0, store_util()->CountPrefetchItems());
} }
...@@ -93,7 +85,7 @@ TEST_F(PrefetchStoreTest, CloseStore) { ...@@ -93,7 +85,7 @@ TEST_F(PrefetchStoreTest, CloseStore) {
EXPECT_EQ(InitializationStatus::kSuccess, EXPECT_EQ(InitializationStatus::kSuccess,
store()->initialization_status_for_testing()); store()->initialization_status_for_testing());
task_runner()->FastForwardBy(PrefetchStore::kClosingDelay); task_environment_.FastForwardBy(PrefetchStore::kClosingDelay);
EXPECT_EQ(InitializationStatus::kNotInitialized, EXPECT_EQ(InitializationStatus::kNotInitialized,
store()->initialization_status_for_testing()); store()->initialization_status_for_testing());
...@@ -113,7 +105,7 @@ TEST_F(PrefetchStoreTest, CloseStorePostponed) { ...@@ -113,7 +105,7 @@ TEST_F(PrefetchStoreTest, CloseStorePostponed) {
EXPECT_EQ(InitializationStatus::kSuccess, EXPECT_EQ(InitializationStatus::kSuccess,
store()->initialization_status_for_testing()); store()->initialization_status_for_testing());
task_runner()->FastForwardBy(PrefetchStore::kClosingDelay / 2); task_environment_.FastForwardBy(PrefetchStore::kClosingDelay / 2);
EXPECT_EQ(InitializationStatus::kSuccess, EXPECT_EQ(InitializationStatus::kSuccess,
store()->initialization_status_for_testing()); store()->initialization_status_for_testing());
...@@ -127,15 +119,15 @@ TEST_F(PrefetchStoreTest, CloseStorePostponed) { ...@@ -127,15 +119,15 @@ TEST_F(PrefetchStoreTest, CloseStorePostponed) {
// This adds up to more than kClosingDelay after the first call, which means // This adds up to more than kClosingDelay after the first call, which means
// the closing would trigger, it does not however, since second call caused it // the closing would trigger, it does not however, since second call caused it
// to be postponed. // to be postponed.
task_runner()->FastForwardBy(2 * PrefetchStore::kClosingDelay / 3); task_environment_.FastForwardBy(2 * PrefetchStore::kClosingDelay / 3);
// Store should still be initialized. // Store should still be initialized.
EXPECT_EQ(InitializationStatus::kSuccess, EXPECT_EQ(InitializationStatus::kSuccess,
store()->initialization_status_for_testing()); store()->initialization_status_for_testing());
// There is still a pending task to close the store. // There is still a pending task to close the store.
EXPECT_TRUE(task_runner()->HasPendingTask()); EXPECT_NE(0u, task_environment_.GetPendingMainThreadTaskCount());
// After this step the store should be closed. // After this step the store should be closed.
task_runner()->FastForwardBy(PrefetchStore::kClosingDelay); task_environment_.FastForwardBy(PrefetchStore::kClosingDelay);
EXPECT_EQ(InitializationStatus::kNotInitialized, EXPECT_EQ(InitializationStatus::kNotInitialized,
store()->initialization_status_for_testing()); store()->initialization_status_for_testing());
} }
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/numerics/safe_conversions.h" #include "base/numerics/safe_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/time/time.h"
#include "components/offline_pages/core/offline_page_feature.h" #include "components/offline_pages/core/offline_page_feature.h"
#include "components/offline_pages/core/prefetch/prefetch_prefs.h" #include "components/offline_pages/core/prefetch/prefetch_prefs.h"
#include "components/offline_pages/core/prefetch/store/prefetch_downloader_quota.h" #include "components/offline_pages/core/prefetch/store/prefetch_downloader_quota.h"
...@@ -103,8 +104,11 @@ TEST_F(DownloadArchivesTaskTest, NoArchivesToDownload) { ...@@ -103,8 +104,11 @@ TEST_F(DownloadArchivesTaskTest, NoArchivesToDownload) {
} }
TEST_F(DownloadArchivesTaskTest, SingleArchiveToDownload) { TEST_F(DownloadArchivesTaskTest, SingleArchiveToDownload) {
constexpr auto kFreshnessDelta = base::TimeDelta::FromMilliseconds(123);
int64_t dummy_item_id = InsertDummyItem(); int64_t dummy_item_id = InsertDummyItem();
int64_t download_item_id = InsertItemToDownload(kLargeArchiveSize); int64_t download_item_id = InsertItemToDownload(kLargeArchiveSize);
FastForwardBy(kFreshnessDelta);
std::set<PrefetchItem> items_before_run; std::set<PrefetchItem> items_before_run;
EXPECT_EQ(2U, store_util()->GetAllItems(&items_before_run)); EXPECT_EQ(2U, store_util()->GetAllItems(&items_before_run));
...@@ -133,10 +137,8 @@ TEST_F(DownloadArchivesTaskTest, SingleArchiveToDownload) { ...@@ -133,10 +137,8 @@ TEST_F(DownloadArchivesTaskTest, SingleArchiveToDownload) {
ASSERT_TRUE(download_item); ASSERT_TRUE(download_item);
EXPECT_EQ(PrefetchItemState::DOWNLOADING, download_item->state); EXPECT_EQ(PrefetchItemState::DOWNLOADING, download_item->state);
EXPECT_EQ(1, download_item->download_initiation_attempts); EXPECT_EQ(1, download_item->download_initiation_attempts);
// These times are created using base::Time::Now() in short distance from each EXPECT_EQ(kFreshnessDelta, download_item->freshness_time -
// other, therefore putting *_LE was considered too. download_item_before->freshness_time);
EXPECT_LT(download_item_before->freshness_time,
download_item->freshness_time);
const TestPrefetchDownloader::RequestMap& requested_downloads = const TestPrefetchDownloader::RequestMap& requested_downloads =
prefetch_downloader()->requested_downloads(); prefetch_downloader()->requested_downloads();
...@@ -203,6 +205,7 @@ TEST_F(DownloadArchivesTaskTest, MultipleLargeArchivesToDownload) { ...@@ -203,6 +205,7 @@ TEST_F(DownloadArchivesTaskTest, MultipleLargeArchivesToDownload) {
int64_t dummy_item_id = InsertDummyItem(); int64_t dummy_item_id = InsertDummyItem();
// download_item_1 is expected to be fresher, therefore we create it second. // download_item_1 is expected to be fresher, therefore we create it second.
int64_t download_item_id_2 = InsertItemToDownload(kLargeArchiveSize); int64_t download_item_id_2 = InsertItemToDownload(kLargeArchiveSize);
FastForwardBy(base::TimeDelta::FromMilliseconds(1));
int64_t download_item_id_1 = InsertItemToDownload(kLargeArchiveSize); int64_t download_item_id_1 = InsertItemToDownload(kLargeArchiveSize);
std::set<PrefetchItem> items_before_run; std::set<PrefetchItem> items_before_run;
...@@ -252,8 +255,12 @@ TEST_F(DownloadArchivesTaskTest, TooManyArchivesToDownload) { ...@@ -252,8 +255,12 @@ TEST_F(DownloadArchivesTaskTest, TooManyArchivesToDownload) {
const int total_items = DownloadArchivesTask::kMaxConcurrentDownloads + 2; const int total_items = DownloadArchivesTask::kMaxConcurrentDownloads + 2;
// Create more than we allow to download in parallel and put then in the // Create more than we allow to download in parallel and put then in the
// |item_ids| in front. // |item_ids| in front.
for (int i = 0; i < total_items; ++i) for (int i = 0; i < total_items; ++i) {
item_ids.insert(item_ids.begin(), InsertItemToDownload(kSmallArchiveSize)); item_ids.insert(item_ids.begin(), InsertItemToDownload(kSmallArchiveSize));
// Add some time in between them so that the download order is deterministic
// and the checks further down work.
FastForwardBy(base::TimeDelta::FromMilliseconds(1));
}
std::set<PrefetchItem> items_before_run; std::set<PrefetchItem> items_before_run;
EXPECT_EQ(static_cast<size_t>(total_items), EXPECT_EQ(static_cast<size_t>(total_items),
...@@ -317,8 +324,12 @@ TEST_F(DownloadArchivesTaskTest, ...@@ -317,8 +324,12 @@ TEST_F(DownloadArchivesTaskTest,
// and put them in the fresher |item_ids| in front. // and put them in the fresher |item_ids| in front.
const size_t total_items = limitless_max_concurrent_downloads + 1; const size_t total_items = limitless_max_concurrent_downloads + 1;
std::vector<int64_t> item_ids; std::vector<int64_t> item_ids;
for (size_t i = 0; i < total_items; ++i) for (size_t i = 0; i < total_items; ++i) {
item_ids.insert(item_ids.begin(), InsertItemToDownload(kLargeArchiveSize)); item_ids.insert(item_ids.begin(), InsertItemToDownload(kLargeArchiveSize));
// Add some time in between them so that the download order is deterministic
// and the checks further down work.
FastForwardBy(base::TimeDelta::FromMilliseconds(1));
}
std::set<PrefetchItem> items_before_run; std::set<PrefetchItem> items_before_run;
EXPECT_EQ(total_items, store_util()->GetAllItems(&items_before_run)); EXPECT_EQ(total_items, store_util()->GetAllItems(&items_before_run));
......
...@@ -21,8 +21,7 @@ PrefetchTaskTestBase::PrefetchTaskTestBase() ...@@ -21,8 +21,7 @@ PrefetchTaskTestBase::PrefetchTaskTestBase()
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>( base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)), &test_url_loader_factory_)),
prefs_(std::make_unique<TestingPrefServiceSimple>()), prefs_(std::make_unique<TestingPrefServiceSimple>()),
prefetch_request_factory_(test_shared_url_loader_factory_, prefs()), prefetch_request_factory_(test_shared_url_loader_factory_, prefs()) {}
store_test_util_(task_runner()) {}
PrefetchTaskTestBase::~PrefetchTaskTestBase() = default; PrefetchTaskTestBase::~PrefetchTaskTestBase() = default;
......
...@@ -5,17 +5,14 @@ ...@@ -5,17 +5,14 @@
#include "components/offline_pages/task/task_test_base.h" #include "components/offline_pages/task/task_test_base.h"
#include "base/test/mock_callback.h" #include "base/test/mock_callback.h"
#include "components/offline_pages/task/test_task_runner.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
using testing::_; using testing::_;
namespace offline_pages { namespace offline_pages {
TaskTestBase::TaskTestBase() TaskTestBase::TaskTestBase() = default;
: task_runner_(new base::TestMockTimeTaskRunner),
test_task_runner_(task_runner_) {
message_loop_.SetTaskRunner(task_runner_);
}
TaskTestBase::~TaskTestBase() = default; TaskTestBase::~TaskTestBase() = default;
...@@ -24,20 +21,24 @@ void TaskTestBase::SetUp() { ...@@ -24,20 +21,24 @@ void TaskTestBase::SetUp() {
} }
void TaskTestBase::TearDown() { void TaskTestBase::TearDown() {
task_runner_->FastForwardUntilNoTasksRemain(); task_environment_.FastForwardUntilNoTasksRemain();
testing::Test::TearDown(); testing::Test::TearDown();
} }
void TaskTestBase::FastForwardBy(base::TimeDelta delta) {
task_environment_.FastForwardBy(delta);
}
void TaskTestBase::RunUntilIdle() { void TaskTestBase::RunUntilIdle() {
task_runner_->RunUntilIdle(); task_environment_.RunUntilIdle();
} }
void TaskTestBase::RunTask(std::unique_ptr<Task> task) { void TaskTestBase::RunTask(std::unique_ptr<Task> task) {
test_task_runner_.RunTask(std::move(task)); TestTaskRunner::RunTask(std::move(task));
} }
void TaskTestBase::RunTask(Task* task) { void TaskTestBase::RunTask(Task* task) {
test_task_runner_.RunTask(task); TestTaskRunner::RunTask(task);
} }
} // namespace offline_pages } // namespace offline_pages
...@@ -5,12 +5,9 @@ ...@@ -5,12 +5,9 @@
#ifndef COMPONENTS_OFFLINE_PAGES_TASK_TASK_TEST_BASE_H_ #ifndef COMPONENTS_OFFLINE_PAGES_TASK_TASK_TEST_BASE_H_
#define COMPONENTS_OFFLINE_PAGES_TASK_TASK_TEST_BASE_H_ #define COMPONENTS_OFFLINE_PAGES_TASK_TASK_TEST_BASE_H_
#include "testing/gtest/include/gtest/gtest.h" #include "base/test/task_environment.h"
#include "base/message_loop/message_loop.h"
#include "base/test/test_mock_time_task_runner.h"
#include "components/offline_pages/task/task.h" #include "components/offline_pages/task/task.h"
#include "components/offline_pages/task/test_task_runner.h" #include "testing/gtest/include/gtest/gtest.h"
namespace offline_pages { namespace offline_pages {
...@@ -30,15 +27,16 @@ class TaskTestBase : public testing::Test { ...@@ -30,15 +27,16 @@ class TaskTestBase : public testing::Test {
// Task is not cleaned up after completing. // Task is not cleaned up after completing.
void RunTask(Task* task); void RunTask(Task* task);
void RunUntilIdle(); void RunUntilIdle();
void FastForwardBy(base::TimeDelta delta);
scoped_refptr<base::TestMockTimeTaskRunner> task_runner() { scoped_refptr<base::SingleThreadTaskRunner> task_runner() {
return task_runner_; return task_environment_.GetMainThreadTaskRunner();
} }
private: private:
base::MessageLoopForIO message_loop_; base::test::SingleThreadTaskEnvironment task_environment_{
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; base::test::TaskEnvironment::MainThreadType::IO,
TestTaskRunner test_task_runner_; base::test::TaskEnvironment::TimeSource::MOCK_TIME};
}; };
} // namespace offline_pages } // namespace offline_pages
......
...@@ -5,35 +5,23 @@ ...@@ -5,35 +5,23 @@
#include "components/offline_pages/task/test_task_runner.h" #include "components/offline_pages/task/test_task_runner.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/run_loop.h"
#include "components/offline_pages/task/task.h" #include "components/offline_pages/task/task.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace offline_pages { namespace offline_pages {
TestTaskRunner::TestTaskRunner(
scoped_refptr<base::TestMockTimeTaskRunner> task_runner)
: task_runner_(task_runner) {}
TestTaskRunner::~TestTaskRunner() {}
void TestTaskRunner::RunTask(std::unique_ptr<Task> task) { void TestTaskRunner::RunTask(std::unique_ptr<Task> task) {
RunTask(task.get()); TestTaskRunner::RunTask(task.get());
} }
void TestTaskRunner::RunTask(Task* task) { void TestTaskRunner::RunTask(Task* task) {
DCHECK(task); DCHECK(task);
Task* completed_task = nullptr; base::RunLoop run_loop;
task->SetTaskCompletionCallbackForTesting(base::BindOnce( task->SetTaskCompletionCallbackForTesting(base::BindOnce(
&TestTaskRunner::TaskComplete, base::Unretained(this), &completed_task)); [](base::RunLoop* run_loop, Task*) { run_loop->Quit(); }, &run_loop));
task->Run(); task->Run();
task_runner_->RunUntilIdle(); run_loop.Run();
EXPECT_EQ(task, completed_task) << "Task did not complete";
}
void TestTaskRunner::TaskComplete(Task** completed_task_ptr, Task* task) {
auto set_task_callback = [](Task** t_ptr, Task* t) { *t_ptr = t; };
task_runner_->PostTask(
FROM_HERE, base::BindOnce(set_task_callback, completed_task_ptr, task));
} }
} // namespace offline_pages } // namespace offline_pages
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <memory> #include <memory>
#include "base/macros.h" #include "base/macros.h"
#include "base/test/test_mock_time_task_runner.h"
namespace offline_pages { namespace offline_pages {
class Task; class Task;
...@@ -16,27 +15,12 @@ class Task; ...@@ -16,27 +15,12 @@ class Task;
// Tool for running (task queue related) tasks in test. // Tool for running (task queue related) tasks in test.
class TestTaskRunner { class TestTaskRunner {
public: public:
explicit TestTaskRunner(
scoped_refptr<base::TestMockTimeTaskRunner> task_runner);
~TestTaskRunner();
// Runs task with expectation that it correctly completes. // Runs task with expectation that it correctly completes.
// Task is also cleaned up after completing. // Task is also cleaned up after completing.
void RunTask(std::unique_ptr<Task> task); static void RunTask(std::unique_ptr<Task> task);
// Runs task with expectation that it correctly completes. // Runs task with expectation that it correctly completes.
// Task is not cleaned up after completing. // Task is not cleaned up after completing.
void RunTask(Task* task); static void RunTask(Task* task);
private:
void TaskComplete(Task** completed_task_ptr, Task* task);
// Certainly confusing, but internal task runner, is simply a test version of
// a single thread task runner. It is used for running closures.
// The difference between that and the encapsulating task runner, is that a
// task in a Task Queue sense may be build of multiple closures.
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
DISALLOW_COPY_AND_ASSIGN(TestTaskRunner);
}; };
} // namespace offline_pages } // namespace offline_pages
......
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