Commit f379a6ef authored by gavinp's avatar gavinp Committed by Commit bot

Use net::TestClosureCallback in SimpleCache tests.

Reduces flakiness; improves test readability. Also introduces a
callback on SimpleIndexFile to signal completion of writing the index,
useful for deflaking cache serialization.

R=jkarlin@chromium.org
BUG=255775,429672

Review URL: https://codereview.chromium.org/699743002

Cr-Commit-Position: refs/heads/master@{#302653}
parent fa3a3cc3
...@@ -470,7 +470,7 @@ void SimpleIndex::WriteToDisk() { ...@@ -470,7 +470,7 @@ void SimpleIndex::WriteToDisk() {
last_write_to_disk_ = start; last_write_to_disk_ = start;
index_file_->WriteToDisk(entries_set_, cache_size_, index_file_->WriteToDisk(entries_set_, cache_size_,
start, app_on_background_); start, app_on_background_, base::Closure());
} }
} // namespace disk_cache } // namespace disk_cache
...@@ -278,18 +278,18 @@ void SimpleIndexFile::LoadIndexEntries(base::Time cache_last_modified, ...@@ -278,18 +278,18 @@ void SimpleIndexFile::LoadIndexEntries(base::Time cache_last_modified,
void SimpleIndexFile::WriteToDisk(const SimpleIndex::EntrySet& entry_set, void SimpleIndexFile::WriteToDisk(const SimpleIndex::EntrySet& entry_set,
uint64 cache_size, uint64 cache_size,
const base::TimeTicks& start, const base::TimeTicks& start,
bool app_on_background) { bool app_on_background,
const base::Closure& callback) {
IndexMetadata index_metadata(entry_set.size(), cache_size); IndexMetadata index_metadata(entry_set.size(), cache_size);
scoped_ptr<Pickle> pickle = Serialize(index_metadata, entry_set); scoped_ptr<Pickle> pickle = Serialize(index_metadata, entry_set);
cache_thread_->PostTask(FROM_HERE, base::Closure task =
base::Bind(&SimpleIndexFile::SyncWriteToDisk, base::Bind(&SimpleIndexFile::SyncWriteToDisk,
cache_type_, cache_type_, cache_directory_, index_file_, temp_index_file_,
cache_directory_, base::Passed(&pickle), start, app_on_background);
index_file_, if (callback.is_null())
temp_index_file_, cache_thread_->PostTask(FROM_HERE, task);
base::Passed(&pickle), else
base::TimeTicks::Now(), cache_thread_->PostTaskAndReply(FROM_HERE, task, callback);
app_on_background));
} }
// static // static
......
...@@ -90,7 +90,8 @@ class NET_EXPORT_PRIVATE SimpleIndexFile { ...@@ -90,7 +90,8 @@ class NET_EXPORT_PRIVATE SimpleIndexFile {
virtual void WriteToDisk(const SimpleIndex::EntrySet& entry_set, virtual void WriteToDisk(const SimpleIndex::EntrySet& entry_set,
uint64 cache_size, uint64 cache_size,
const base::TimeTicks& start, const base::TimeTicks& start,
bool app_on_background); bool app_on_background,
const base::Closure& callback);
private: private:
friend class WrappedSimpleIndexFile; friend class WrappedSimpleIndexFile;
......
...@@ -98,24 +98,6 @@ class SimpleIndexFileTest : public testing::Test { ...@@ -98,24 +98,6 @@ class SimpleIndexFileTest : public testing::Test {
b.last_used_time_seconds_since_epoch_ && b.last_used_time_seconds_since_epoch_ &&
a.entry_size_ == b.entry_size_; a.entry_size_ == b.entry_size_;
} }
protected:
SimpleIndexFileTest() : callback_called_(false) {}
base::Closure GetCallback() {
return base::Bind(&SimpleIndexFileTest::LoadIndexEntriesCallback,
base::Unretained(this));
}
bool callback_called() { return callback_called_; }
private:
void LoadIndexEntriesCallback() {
EXPECT_FALSE(callback_called_);
callback_called_ = true;
}
bool callback_called_;
}; };
TEST_F(SimpleIndexFileTest, Serialize) { TEST_F(SimpleIndexFileTest, Serialize) {
...@@ -203,11 +185,12 @@ TEST_F(SimpleIndexFileTest, WriteThenLoadIndex) { ...@@ -203,11 +185,12 @@ TEST_F(SimpleIndexFileTest, WriteThenLoadIndex) {
} }
const uint64 kCacheSize = 456U; const uint64 kCacheSize = 456U;
net::TestClosure closure;
{ {
WrappedSimpleIndexFile simple_index_file(cache_dir.path()); WrappedSimpleIndexFile simple_index_file(cache_dir.path());
simple_index_file.WriteToDisk(entries, kCacheSize, simple_index_file.WriteToDisk(entries, kCacheSize, base::TimeTicks(),
base::TimeTicks(), false); false, closure.closure());
base::RunLoop().RunUntilIdle(); closure.WaitForResult();
EXPECT_TRUE(base::PathExists(simple_index_file.GetIndexFilePath())); EXPECT_TRUE(base::PathExists(simple_index_file.GetIndexFilePath()));
} }
...@@ -215,13 +198,11 @@ TEST_F(SimpleIndexFileTest, WriteThenLoadIndex) { ...@@ -215,13 +198,11 @@ TEST_F(SimpleIndexFileTest, WriteThenLoadIndex) {
base::Time fake_cache_mtime; base::Time fake_cache_mtime;
ASSERT_TRUE(simple_util::GetMTime(cache_dir.path(), &fake_cache_mtime)); ASSERT_TRUE(simple_util::GetMTime(cache_dir.path(), &fake_cache_mtime));
SimpleIndexLoadResult load_index_result; SimpleIndexLoadResult load_index_result;
simple_index_file.LoadIndexEntries(fake_cache_mtime, simple_index_file.LoadIndexEntries(fake_cache_mtime, closure.closure(),
GetCallback(),
&load_index_result); &load_index_result);
base::RunLoop().RunUntilIdle(); closure.WaitForResult();
EXPECT_TRUE(base::PathExists(simple_index_file.GetIndexFilePath())); EXPECT_TRUE(base::PathExists(simple_index_file.GetIndexFilePath()));
ASSERT_TRUE(callback_called());
EXPECT_TRUE(load_index_result.did_load); EXPECT_TRUE(load_index_result.did_load);
EXPECT_FALSE(load_index_result.flush_required); EXPECT_FALSE(load_index_result.flush_required);
...@@ -246,15 +227,13 @@ TEST_F(SimpleIndexFileTest, LoadCorruptIndex) { ...@@ -246,15 +227,13 @@ TEST_F(SimpleIndexFileTest, LoadCorruptIndex) {
&fake_cache_mtime)); &fake_cache_mtime));
EXPECT_FALSE(WrappedSimpleIndexFile::LegacyIsIndexFileStale(fake_cache_mtime, EXPECT_FALSE(WrappedSimpleIndexFile::LegacyIsIndexFileStale(fake_cache_mtime,
index_path)); index_path));
SimpleIndexLoadResult load_index_result; SimpleIndexLoadResult load_index_result;
simple_index_file.LoadIndexEntries(fake_cache_mtime, net::TestClosure closure;
GetCallback(), simple_index_file.LoadIndexEntries(fake_cache_mtime, closure.closure(),
&load_index_result); &load_index_result);
base::RunLoop().RunUntilIdle(); closure.WaitForResult();
EXPECT_FALSE(base::PathExists(index_path)); EXPECT_FALSE(base::PathExists(index_path));
ASSERT_TRUE(callback_called());
EXPECT_TRUE(load_index_result.did_load); EXPECT_TRUE(load_index_result.did_load);
EXPECT_TRUE(load_index_result.flush_required); EXPECT_TRUE(load_index_result.flush_required);
} }
......
...@@ -68,7 +68,8 @@ class MockSimpleIndexFile : public SimpleIndexFile, ...@@ -68,7 +68,8 @@ class MockSimpleIndexFile : public SimpleIndexFile,
void WriteToDisk(const SimpleIndex::EntrySet& entry_set, void WriteToDisk(const SimpleIndex::EntrySet& entry_set,
uint64 cache_size, uint64 cache_size,
const base::TimeTicks& start, const base::TimeTicks& start,
bool app_on_background) override { bool app_on_background,
const base::Closure& callback) override {
disk_writes_++; disk_writes_++;
disk_write_entry_set_ = entry_set; disk_write_entry_set_ = entry_set;
} }
......
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