Commit e0f18025 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Avoid posting parallel tasks for same download

This CL is fixing the memory leak describe here:
  https://bugs.chromium.org/p/chromium/issues/detail?id=974239

Without this patch, tasks to check the file existence can be
queued for the same download without being executed. This can
lead to terrible scenario with billion of tasks queued.

This CL is adding a set to keep track of the current pending
download queries. This way, there can not be two pending tasks
at the same time for each download item.

Bug: 974239
Change-Id: I52fb23ce50ba8542ed262552dd7f9aaaa5589548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946783Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720600}
parent 30ecdace
......@@ -668,6 +668,11 @@ void DownloadManagerImpl::CheckForFileRemoval(
return;
}
// Check whether an task is already queued or running for the current download
// and skip this check if it is the case.
if (!pending_disk_access_query_.insert(download_item->GetId()).second)
return;
base::PostTaskAndReplyWithResult(
disk_access_task_runner_.get(), FROM_HERE,
base::BindOnce(&base::PathExists, download_item->GetTargetFilePath()),
......@@ -678,6 +683,10 @@ void DownloadManagerImpl::CheckForFileRemoval(
void DownloadManagerImpl::OnFileExistenceChecked(uint32_t download_id,
bool result) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
// Remove the pending check flag for this download to allow new requests.
pending_disk_access_query_.erase(download_id);
if (!result) { // File does not exist.
if (base::Contains(downloads_, download_id))
downloads_[download_id]->OnDownloadedFileRemoved();
......
......@@ -359,6 +359,9 @@ class CONTENT_EXPORT DownloadManagerImpl
// disk operations.
const scoped_refptr<base::SequencedTaskRunner> disk_access_task_runner_;
// DownloadItem for which a query is queued in the |disk_access_task_runner_|.
std::set<uint32_t> pending_disk_access_query_;
base::WeakPtrFactory<DownloadManagerImpl> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(DownloadManagerImpl);
......
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