Commit 079fa669 authored by Min Qin's avatar Min Qin Committed by Commit Bot

Fix a DCHECK failure as RemoveEntry() can be called before UpdateDownloadDB

When AddOrReplaceEntry() posts a UpdateDownloadDB task, RemoveEntry()
could be called before the posted task to get a executed.
This will cause the DCHECK to fail.

BUG=872279

Change-Id: I90c19a66814b050ef4cded72ee35380790d16d4c
Reviewed-on: https://chromium-review.googlesource.com/1169979Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581964}
parent bf5264fd
......@@ -186,7 +186,8 @@ void DownloadDBCache::RemoveEntry(const std::string& guid) {
}
void DownloadDBCache::UpdateDownloadDB() {
DCHECK(!updated_guids_.empty());
if (updated_guids_.empty())
return;
std::vector<DownloadDBEntry> entries;
for (auto guid : updated_guids_) {
......
......@@ -286,6 +286,38 @@ TEST_F(DownloadDBCacheTest, RemoveEntry) {
ASSERT_EQ(guid2, loaded_entries[0].GetGuid());
}
// Test that removing an entry during the middle of modifying it should work.
TEST_F(DownloadDBCacheTest, RemoveWhileModifyExistingEntry) {
PrepopulateSampleEntries();
CreateDBCache();
std::vector<DownloadDBEntry> loaded_entries;
db_cache_->Initialize(
std::vector<DownloadEntry>(),
base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this),
&loaded_entries));
db_->InitCallback(true);
db_->LoadCallback(true);
ASSERT_EQ(loaded_entries.size(), 2u);
loaded_entries[0].download_info->id = 100;
db_cache_->AddOrReplaceEntry(loaded_entries[0]);
ASSERT_EQ(task_runner_->GetPendingTaskCount(), 1u);
ASSERT_GT(task_runner_->NextPendingTaskDelay(), base::TimeDelta());
db_cache_->RemoveEntry(loaded_entries[0].GetGuid());
task_runner_->FastForwardUntilNoTasksRemain();
DownloadDBEntry remaining = loaded_entries[1];
loaded_entries.clear();
DownloadDB* download_db = GetDownloadDB();
download_db->LoadEntries(base::BindOnce(&DownloadDBCacheTest::InitCallback,
base::Unretained(this),
&loaded_entries));
db_->LoadCallback(true);
ASSERT_EQ(loaded_entries.size(), 1u);
ASSERT_EQ(remaining, loaded_entries[0]);
}
// Tests that Migrating a DownloadEntry from InProgressCache should store
// a DownloadDBEntry in the DownloadDB.
TEST_F(DownloadDBCacheTest, MigrateFromInProgressCache) {
......
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