Commit 13a84569 authored by Joy Ming's avatar Joy Ming Committed by Commit Bot

In_progress_cache : Read/write implementation.

The in-progress cache is used to store metadata related to in-progress
downloads. The entry for the download is created upon the initiation of
the download and removed after the download is completed. This CL
continues the implementation of the in-progress cache by implementing
the read/write functionality in the following manner:

- making the in-progress cache an interface.
- initializing the cache during the creation of the download item,
halting the completion of the creation until the cache is initialized
(similar to the history database, which persists longer-term info).
- reading from the file during initialization and storing information
in an |entries| object.
- updating the |entries| object when adding/replacing/deleting and then
writing to the file using ImportantFileWriter.
- including unittests for this process.

Bug: 778425
Change-Id: I8f69dd51dd619dbc688c0f2b6a3b079a3b1164ec
Reviewed-on: https://chromium-review.googlesource.com/736907
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521158}
parent 6f6baa4f
...@@ -48,7 +48,7 @@ ...@@ -48,7 +48,7 @@
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/common/safe_browsing/file_type_policies.h" #include "chrome/common/safe_browsing/file_type_policies.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/download/downloader/in_progress/in_progress_cache.h" #include "components/download/downloader/in_progress/in_progress_cache_impl.h"
#include "components/pref_registry/pref_registry_syncable.h" #include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_member.h" #include "components/prefs/pref_member.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
...@@ -269,7 +269,7 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile) ...@@ -269,7 +269,7 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
DCHECK(!profile_->GetPath().empty()); DCHECK(!profile_->GetPath().empty());
base::FilePath metadata_cache_file = base::FilePath metadata_cache_file =
profile_->GetPath().Append(chrome::kDownloadMetadataStoreFilename); profile_->GetPath().Append(chrome::kDownloadMetadataStoreFilename);
download_metadata_cache_.reset(new download::InProgressCache( download_metadata_cache_.reset(new download::InProgressCacheImpl(
metadata_cache_file, disk_access_task_runner_)); metadata_cache_file, disk_access_task_runner_));
} }
...@@ -549,7 +549,6 @@ void ChromeDownloadManagerDelegate::ChooseSavePath( ...@@ -549,7 +549,6 @@ void ChromeDownloadManagerDelegate::ChooseSavePath(
} }
download::InProgressCache* ChromeDownloadManagerDelegate::GetInProgressCache() { download::InProgressCache* ChromeDownloadManagerDelegate::GetInProgressCache() {
// TODO(crbug.com/778425): Make sure the cache is initialized.
DCHECK(download_metadata_cache_ != nullptr); DCHECK(download_metadata_cache_ != nullptr);
return download_metadata_cache_.get(); return download_metadata_cache_.get();
} }
......
...@@ -315,7 +315,10 @@ void DownloadHistory::QueryCallback(std::unique_ptr<InfoVector> infos) { ...@@ -315,7 +315,10 @@ void DownloadHistory::QueryCallback(std::unique_ptr<InfoVector> infos) {
++history_size_; ++history_size_;
} }
notifier_.GetManager()->CheckForHistoryFilesRemoval(); notifier_.GetManager()->CheckForHistoryFilesRemoval();
notifier_.GetManager()->PostInitialization();
// Indicate that the history db is initialized.
notifier_.GetManager()->PostInitialization(
content::DownloadManager::DOWNLOAD_INITIALIZATION_DEPENDENCY_HISTORY_DB);
initial_history_query_complete_ = true; initial_history_query_complete_ = true;
for (Observer& observer : observers_) for (Observer& observer : observers_)
......
...@@ -330,7 +330,10 @@ TEST_F(DownloadUIControllerTest, DownloadUIController_HistoryDownload) { ...@@ -330,7 +330,10 @@ TEST_F(DownloadUIControllerTest, DownloadUIController_HistoryDownload) {
EXPECT_CALL(*item, GetOriginalMimeType()); EXPECT_CALL(*item, GetOriginalMimeType());
EXPECT_CALL(*manager(), CheckForHistoryFilesRemoval()); EXPECT_CALL(*manager(), CheckForHistoryFilesRemoval());
EXPECT_CALL(*manager(), PostInitialization()); EXPECT_CALL(
*manager(),
PostInitialization(content::DownloadManager::
DOWNLOAD_INITIALIZATION_DEPENDENCY_HISTORY_DB));
{ {
testing::InSequence s; testing::InSequence s;
......
...@@ -12,8 +12,9 @@ static_library("in_progress") { ...@@ -12,8 +12,9 @@ static_library("in_progress") {
"download_entry.cc", "download_entry.cc",
"download_entry.h", "download_entry.h",
"download_source.h", "download_source.h",
"in_progress_cache.cc",
"in_progress_cache.h", "in_progress_cache.h",
"in_progress_cache_impl.cc",
"in_progress_cache_impl.h",
"in_progress_conversions.cc", "in_progress_conversions.cc",
"in_progress_conversions.h", "in_progress_conversions.h",
] ]
...@@ -28,6 +29,7 @@ source_set("unit_tests") { ...@@ -28,6 +29,7 @@ source_set("unit_tests") {
testonly = true testonly = true
sources = [ sources = [
"in_progress_cache_impl_unittest.cc",
"in_progress_conversions_unittest.cc", "in_progress_conversions_unittest.cc",
] ]
...@@ -35,6 +37,7 @@ source_set("unit_tests") { ...@@ -35,6 +37,7 @@ source_set("unit_tests") {
":in_progress", ":in_progress",
"//base/test:test_support", "//base/test:test_support",
"//components/download/downloader/in_progress/proto", "//components/download/downloader/in_progress/proto",
"//content/test:test_support",
"//testing/gmock", "//testing/gmock",
"//testing/gtest", "//testing/gtest",
] ]
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/download/downloader/in_progress/in_progress_cache.h"
namespace download {
InProgressCache::InProgressCache(
const base::FilePath& cache_file_path,
const scoped_refptr<base::SequencedTaskRunner>& task_runner) {}
InProgressCache::~InProgressCache() = default;
void InProgressCache::AddOrReplaceEntry(const DownloadEntry& entry) {
// TODO(jming): Implementation.
}
DownloadEntry* InProgressCache::RetrieveEntry(const std::string& guid) {
// TODO(jming): Implementation.
return nullptr;
}
void InProgressCache::RemoveEntry(const std::string& guid) {
// TODO(jming): Implementation.
}
} // namespace download
...@@ -7,11 +7,8 @@ ...@@ -7,11 +7,8 @@
#include <string> #include <string>
#include "base/files/file_path.h" #include "base/optional.h"
#include "base/macros.h"
#include "base/sequenced_task_runner.h"
#include "components/download/downloader/in_progress/download_entry.h" #include "components/download/downloader/in_progress/download_entry.h"
#include "components/download/downloader/in_progress/proto/download_entry.pb.h"
namespace download { namespace download {
...@@ -21,21 +18,20 @@ namespace download { ...@@ -21,21 +18,20 @@ namespace download {
// right away for now (might not be in the case in the long run). // right away for now (might not be in the case in the long run).
class InProgressCache { class InProgressCache {
public: public:
InProgressCache(const base::FilePath& cache_file_path, virtual ~InProgressCache() = default;
const scoped_refptr<base::SequencedTaskRunner>& task_runner);
~InProgressCache(); // Initializes the cache.
virtual void Initialize(const base::RepeatingClosure& callback) = 0;
// Adds or updates an existing entry. // Adds or updates an existing entry.
void AddOrReplaceEntry(const DownloadEntry& entry); virtual void AddOrReplaceEntry(const DownloadEntry& entry) = 0;
// Retrieves an existing entry. // Retrieves an existing entry.
DownloadEntry* RetrieveEntry(const std::string& guid); virtual base::Optional<DownloadEntry> RetrieveEntry(
const std::string& guid) = 0;
// Removes an entry. // Removes an entry.
void RemoveEntry(const std::string& guid); virtual void RemoveEntry(const std::string& guid) = 0;
private:
DISALLOW_COPY_AND_ASSIGN(InProgressCache);
}; };
} // namespace download } // namespace download
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/download/downloader/in_progress/in_progress_cache_impl.h"
#include "base/bind.h"
#include "base/files/file.h"
#include "base/files/file_util.h"
#include "base/files/important_file_writer.h"
#include "base/task_runner_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/download/downloader/in_progress/in_progress_conversions.h"
namespace download {
namespace {
// Helper functions for |entries_| related operations.
int GetIndexFromEntries(const metadata_pb::DownloadEntries& entries,
const std::string& guid) {
int size_of_entries = entries.entries_size();
for (int i = 0; i < size_of_entries; i++) {
if (entries.entries(i).guid() == guid)
return i;
}
return -1;
}
void AddOrReplaceEntryInEntries(metadata_pb::DownloadEntries& entries,
const DownloadEntry& entry) {
metadata_pb::DownloadEntry metadata_entry =
InProgressConversions::DownloadEntryToProto(entry);
int entry_index = GetIndexFromEntries(entries, metadata_entry.guid());
metadata_pb::DownloadEntry* entry_ptr =
(entry_index < 0) ? entries.add_entries()
: entries.mutable_entries(entry_index);
*entry_ptr = metadata_entry;
}
base::Optional<DownloadEntry> GetEntryFromEntries(
const metadata_pb::DownloadEntries& entries,
const std::string& guid) {
int entry_index = GetIndexFromEntries(entries, guid);
if (entry_index < 0)
return base::nullopt;
return InProgressConversions::DownloadEntryFromProto(
entries.entries(entry_index));
}
void RemoveEntryFromEntries(metadata_pb::DownloadEntries& entries,
const std::string& guid) {
int entry_index = GetIndexFromEntries(entries, guid);
if (entry_index >= 0)
entries.mutable_entries()->DeleteSubrange(entry_index, 1);
}
// Helper functions for file read/write operations.
std::vector<char> ReadEntriesFromFile(base::FilePath file_path) {
// Check validity of file.
base::File entries_file(file_path,
base::File::FLAG_OPEN | base::File::FLAG_READ);
if (!entries_file.IsValid()) {
// The file just doesn't exist yet (potentially because no entries have been
// written yet) so we can't read from it.
return std::vector<char>();
}
// Get file info.
base::File::Info info;
if (!entries_file.GetInfo(&info)) {
LOG(ERROR) << "Could not read download entries from file "
<< "because get info failed.";
return std::vector<char>();
}
if (info.is_directory) {
LOG(ERROR) << "Could not read download entries from file "
<< "because file is a directory.";
return std::vector<char>();
}
// Read and parse file.
const int64_t size = base::saturated_cast<int64_t>(info.size);
if (size > INT_MAX || size < 0) {
LOG(ERROR) << "Could not read download entries from file "
<< "because the file size was unexpected.";
return std::vector<char>();
}
auto file_data = std::vector<char>(size);
if (entries_file.Read(0, file_data.data(), size) <= 0) {
LOG(ERROR) << "Could not read download entries from file "
<< "because there was a read failure.";
return std::vector<char>();
}
return file_data;
}
std::string EntriesToString(const metadata_pb::DownloadEntries& entries) {
std::string entries_string;
if (!entries.SerializeToString(&entries_string)) {
// TODO(crbug.com/778425): Have more robust error-handling for serialization
// error.
LOG(ERROR) << "Could not write download entries to file "
<< "because of a serialization issue.";
return std::string();
}
return entries_string;
}
void WriteEntriesToFile(const std::string& entries, base::FilePath file_path) {
DCHECK(!file_path.empty());
if (!base::ImportantFileWriter::WriteFileAtomically(file_path, entries)) {
LOG(ERROR) << "Could not write download entries to file: "
<< file_path.value();
}
}
} // namespace
InProgressCacheImpl::InProgressCacheImpl(
const base::FilePath& cache_file_path,
const scoped_refptr<base::SequencedTaskRunner>& task_runner)
: file_path_(cache_file_path),
initialization_status_(CACHE_UNINITIALIZED),
task_runner_(task_runner),
weak_ptr_factory_(this) {}
InProgressCacheImpl::~InProgressCacheImpl() = default;
void InProgressCacheImpl::Initialize(const base::RepeatingClosure& callback) {
// If it's already initialized, just run the callback.
if (initialization_status_ == CACHE_INITIALIZED) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
return;
}
pending_actions_.push_back(callback);
// If uninitialized, initialize |entries_| by reading from file.
if (initialization_status_ == CACHE_UNINITIALIZED) {
base::PostTaskAndReplyWithResult(
task_runner_.get(), FROM_HERE,
base::BindOnce(&ReadEntriesFromFile, file_path_),
base::BindOnce(&InProgressCacheImpl::OnInitialized,
weak_ptr_factory_.GetWeakPtr()));
}
}
void InProgressCacheImpl::OnInitialized(std::vector<char> entries) {
if (entries.empty()) {
if (!entries_.ParseFromArray(entries.data(), entries.size())) {
// TODO(crbug.com/778425): Get UMA for errors.
LOG(ERROR) << "Could not read download entries from file "
<< "because there was a parse failure.";
return;
}
}
initialization_status_ = CACHE_INITIALIZED;
while (!pending_actions_.empty()) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
pending_actions_.front());
pending_actions_.pop_front();
}
}
void InProgressCacheImpl::AddOrReplaceEntry(const DownloadEntry& entry) {
if (initialization_status_ != CACHE_INITIALIZED) {
LOG(ERROR) << "Cache is not initialized, cannot AddOrReplaceEntry.";
return;
}
// Update |entries_|.
AddOrReplaceEntryInEntries(entries_, entry);
// Serialize |entries_| and write to file.
std::string entries_string = EntriesToString(entries_);
task_runner_->PostTask(FROM_HERE, base::BindOnce(&WriteEntriesToFile,
entries_string, file_path_));
}
base::Optional<DownloadEntry> InProgressCacheImpl::RetrieveEntry(
const std::string& guid) {
if (initialization_status_ != CACHE_INITIALIZED) {
LOG(ERROR) << "Cache is not initialized, cannot RetrieveEntry.";
return base::nullopt;
}
return GetEntryFromEntries(entries_, guid);
}
void InProgressCacheImpl::RemoveEntry(const std::string& guid) {
if (initialization_status_ != CACHE_INITIALIZED) {
LOG(ERROR) << "Cache is not initialized, cannot RemoveEntry.";
return;
}
// Update |entries_|.
RemoveEntryFromEntries(entries_, guid);
// Serialize |entries_| and write to file.
std::string entries_string = EntriesToString(entries_);
task_runner_->PostTask(FROM_HERE, base::BindOnce(&WriteEntriesToFile,
entries_string, file_path_));
}
} // namespace download
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_DOWNLOAD_IN_PROGRESS_CACHE_IMPL_H_
#define COMPONENTS_DOWNLOAD_IN_PROGRESS_CACHE_IMPL_H_
#include <string>
#include "base/containers/circular_deque.h"
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/sequenced_task_runner.h"
#include "components/download/downloader/in_progress/download_entry.h"
#include "components/download/downloader/in_progress/in_progress_cache.h"
#include "components/download/downloader/in_progress/proto/download_entry.pb.h"
namespace download {
// InProgressCache provides a write-through cache that persists
// information related to an in-progress download such as request origin, retry
// count, resumption parameters etc to the disk. The entries are written to disk
// right away.
class InProgressCacheImpl : public InProgressCache {
public:
explicit InProgressCacheImpl(
const base::FilePath& cache_file_path,
const scoped_refptr<base::SequencedTaskRunner>& task_runner);
~InProgressCacheImpl() override;
// InProgressCache implementation.
void Initialize(const base::RepeatingClosure& callback) override;
void AddOrReplaceEntry(const DownloadEntry& entry) override;
base::Optional<DownloadEntry> RetrieveEntry(const std::string& guid) override;
void RemoveEntry(const std::string& guid) override;
private:
// States to keep track of initialization status.
enum InitializationStatus {
CACHE_UNINITIALIZED = 0,
CACHE_INITIALIZING = 1,
CACHE_INITIALIZED = 2,
};
// Steps to execute after initialization is complete.
void OnInitialized(std::vector<char> entries);
metadata_pb::DownloadEntries entries_;
base::FilePath file_path_;
InitializationStatus initialization_status_;
base::circular_deque<base::RepeatingClosure> pending_actions_;
scoped_refptr<base::SequencedTaskRunner> task_runner_;
base::WeakPtrFactory<InProgressCacheImpl> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(InProgressCacheImpl);
};
} // namespace download
#endif // COMPONENTS_DOWNLOAD_IN_PROGRESS_CACHE_IMPL_H_
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/download/downloader/in_progress/in_progress_cache_impl.h"
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/memory/ptr_util.h"
#include "base/run_loop.h"
#include "base/task_runner.h"
#include "base/task_scheduler/post_task.h"
#include "base/test/scoped_task_environment.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using testing::_;
namespace download {
namespace {
const base::FilePath::CharType kInProgressCachePath[] =
FILE_PATH_LITERAL("/test/in_progress_cache");
class InProgressCacheImplTest : public testing::Test {
public:
InProgressCacheImplTest()
: cache_initialized_(false),
task_environment_(
base::test::ScopedTaskEnvironment::MainThreadType::IO) {}
~InProgressCacheImplTest() override = default;
void SetUp() override {
base::FilePath file_path(kInProgressCachePath);
scoped_refptr<base::SequencedTaskRunner> task_runner =
base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BACKGROUND,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN});
cache_ = base::MakeUnique<InProgressCacheImpl>(file_path, task_runner);
}
protected:
void InitializeCache() {
cache_->Initialize(base::BindRepeating(
&InProgressCacheImplTest::OnCacheInitialized, base::Unretained(this)));
}
void OnCacheInitialized() { cache_initialized_ = true; }
std::unique_ptr<InProgressCacheImpl> cache_;
bool cache_initialized_;
base::test::ScopedTaskEnvironment task_environment_;
private:
DISALLOW_COPY_AND_ASSIGN(InProgressCacheImplTest);
}; // class InProgressCacheImplTest
} // namespace
TEST_F(InProgressCacheImplTest, AddAndRetrieveAndReplaceAndRemove) {
// Initialize cache.
InitializeCache();
while (!cache_initialized_) {
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(cache_initialized_);
// Add entry.
std::string guid1("guid");
DownloadEntry entry1(guid1, "request origin", DownloadSource::UNKNOWN);
cache_->AddOrReplaceEntry(entry1);
base::Optional<DownloadEntry> retrieved_entry1 = cache_->RetrieveEntry(guid1);
EXPECT_TRUE(retrieved_entry1.has_value());
EXPECT_EQ(entry1, retrieved_entry1.value());
// Replace entry.
std::string new_request_origin("new request origin");
entry1.request_origin = new_request_origin;
cache_->AddOrReplaceEntry(entry1);
base::Optional<DownloadEntry> replaced_entry1 = cache_->RetrieveEntry(guid1);
EXPECT_TRUE(replaced_entry1.has_value());
EXPECT_EQ(entry1, replaced_entry1.value());
EXPECT_EQ(new_request_origin, replaced_entry1.value().request_origin);
// Add another entry.
std::string guid2("guid2");
DownloadEntry entry2(guid2, "other request origin", DownloadSource::UNKNOWN);
cache_->AddOrReplaceEntry(entry2);
base::Optional<DownloadEntry> retrieved_entry2 = cache_->RetrieveEntry(guid2);
EXPECT_TRUE(retrieved_entry2.has_value());
EXPECT_EQ(entry2, retrieved_entry2.value());
// Remove original entry.
cache_->RemoveEntry(guid1);
base::Optional<DownloadEntry> removed_entry1 = cache_->RetrieveEntry(guid1);
EXPECT_FALSE(removed_entry1.has_value());
// Remove other entry.
cache_->RemoveEntry(guid2);
base::Optional<DownloadEntry> removed_entry2 = cache_->RetrieveEntry(guid2);
EXPECT_FALSE(removed_entry2.has_value());
}
} // namespace download
...@@ -2287,10 +2287,12 @@ void DownloadItemImpl::ResumeInterruptedDownload( ...@@ -2287,10 +2287,12 @@ void DownloadItemImpl::ResumeInterruptedDownload(
if (manager_delegate) { if (manager_delegate) {
download::InProgressCache* in_progress_cache = download::InProgressCache* in_progress_cache =
manager_delegate->GetInProgressCache(); manager_delegate->GetInProgressCache();
download::DownloadEntry* entry = if (in_progress_cache) {
base::Optional<download::DownloadEntry> entry =
in_progress_cache->RetrieveEntry(GetGuid()); in_progress_cache->RetrieveEntry(GetGuid());
if (entry) if (entry)
download_params->set_request_origin(entry->request_origin); download_params->set_request_origin(entry.value().request_origin);
}
} }
// Note that resumed downloads disallow redirects. Hence the referrer URL // Note that resumed downloads disallow redirects. Hence the referrer URL
......
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
#include "base/synchronization/lock.h" #include "base/synchronization/lock.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/download/downloader/in_progress/download_entry.h" #include "components/download/downloader/in_progress/download_entry.h"
#include "components/download/downloader/in_progress/in_progress_cache.h" #include "components/download/downloader/in_progress/in_progress_cache_impl.h"
#include "content/browser/byte_stream.h" #include "content/browser/byte_stream.h"
#include "content/browser/child_process_security_policy_impl.h" #include "content/browser/child_process_security_policy_impl.h"
#include "content/browser/download/download_create_info.h" #include "content/browser/download/download_create_info.h"
...@@ -343,6 +343,9 @@ InProgressDownloadObserver::~InProgressDownloadObserver() = default; ...@@ -343,6 +343,9 @@ InProgressDownloadObserver::~InProgressDownloadObserver() = default;
void InProgressDownloadObserver::OnDownloadUpdated(DownloadItem* download) { void InProgressDownloadObserver::OnDownloadUpdated(DownloadItem* download) {
// TODO(crbug.com/778425): Properly handle fail/resume/retry for downloads // TODO(crbug.com/778425): Properly handle fail/resume/retry for downloads
// that are in the INTERRUPTED state for a long time. // that are in the INTERRUPTED state for a long time.
if (!in_progress_cache_)
return;
switch (download->GetState()) { switch (download->GetState()) {
case DownloadItem::DownloadState::COMPLETE: case DownloadItem::DownloadState::COMPLETE:
case DownloadItem::DownloadState::CANCELLED: case DownloadItem::DownloadState::CANCELLED:
...@@ -359,6 +362,9 @@ void InProgressDownloadObserver::OnDownloadUpdated(DownloadItem* download) { ...@@ -359,6 +362,9 @@ void InProgressDownloadObserver::OnDownloadUpdated(DownloadItem* download) {
} }
void InProgressDownloadObserver::OnDownloadRemoved(DownloadItem* download) { void InProgressDownloadObserver::OnDownloadRemoved(DownloadItem* download) {
if (!in_progress_cache_)
return;
in_progress_cache_->RemoveEntry(download->GetGuid()); in_progress_cache_->RemoveEntry(download->GetGuid());
} }
...@@ -367,6 +373,8 @@ DownloadManagerImpl::DownloadManagerImpl(BrowserContext* browser_context) ...@@ -367,6 +373,8 @@ DownloadManagerImpl::DownloadManagerImpl(BrowserContext* browser_context)
file_factory_(new DownloadFileFactory()), file_factory_(new DownloadFileFactory()),
shutdown_needed_(true), shutdown_needed_(true),
initialized_(false), initialized_(false),
history_db_initialized_(false),
in_progress_cache_initialized_(false),
browser_context_(browser_context), browser_context_(browser_context),
delegate_(nullptr), delegate_(nullptr),
weak_factory_(this) { weak_factory_(this) {
...@@ -446,6 +454,27 @@ bool DownloadManagerImpl::ShouldOpenDownload( ...@@ -446,6 +454,27 @@ bool DownloadManagerImpl::ShouldOpenDownload(
void DownloadManagerImpl::SetDelegate(DownloadManagerDelegate* delegate) { void DownloadManagerImpl::SetDelegate(DownloadManagerDelegate* delegate) {
delegate_ = delegate; delegate_ = delegate;
// If initialization has not occurred and there's a delegate and cache,
// initialize and indicate initialization is complete. Else, just indicate it
// is ok to continue.
if (initialized_ || in_progress_cache_initialized_)
return;
base::RepeatingClosure post_init_callback = base::BindRepeating(
&DownloadManagerImpl::PostInitialization, weak_factory_.GetWeakPtr(),
DOWNLOAD_INITIALIZATION_DEPENDENCY_IN_PROGRESS_CACHE);
if (delegate_) {
download::InProgressCache* in_progress_cache =
delegate_->GetInProgressCache();
if (in_progress_cache) {
in_progress_cache->Initialize(post_init_callback);
return;
}
}
BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, post_init_callback);
} }
DownloadManagerDelegate* DownloadManagerImpl::GetDelegate() const { DownloadManagerDelegate* DownloadManagerImpl::GetDelegate() const {
...@@ -885,11 +914,33 @@ DownloadItem* DownloadManagerImpl::CreateDownloadItem( ...@@ -885,11 +914,33 @@ DownloadItem* DownloadManagerImpl::CreateDownloadItem(
return item; return item;
} }
void DownloadManagerImpl::PostInitialization() { void DownloadManagerImpl::PostInitialization(
DCHECK(!initialized_); DownloadInitializationDependency dependency) {
initialized_ = true; // If initialization has occurred (ie. in tests), skip post init steps.
if (initialized_)
return;
switch (dependency) {
case DOWNLOAD_INITIALIZATION_DEPENDENCY_HISTORY_DB:
history_db_initialized_ = true;
break;
case DOWNLOAD_INITIALIZATION_DEPENDENCY_IN_PROGRESS_CACHE:
in_progress_cache_initialized_ = true;
break;
case DOWNLOAD_INITIALIZATION_DEPENDENCY_NONE:
default:
NOTREACHED();
break;
}
// Download manager is only initialized if both history db and in progress
// cache are initialized.
initialized_ = history_db_initialized_ && in_progress_cache_initialized_;
if (initialized_) {
for (auto& observer : observers_) for (auto& observer : observers_)
observer.OnManagerInitialized(); observer.OnManagerInitialized();
}
} }
bool DownloadManagerImpl::IsManagerInitialized() const { bool DownloadManagerImpl::IsManagerInitialized() const {
......
...@@ -105,7 +105,7 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager, ...@@ -105,7 +105,7 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager,
base::Time last_access_time, base::Time last_access_time,
bool transient, bool transient,
const std::vector<DownloadItem::ReceivedSlice>& received_slices) override; const std::vector<DownloadItem::ReceivedSlice>& received_slices) override;
void PostInitialization() override; void PostInitialization(DownloadInitializationDependency dependency) override;
bool IsManagerInitialized() const override; bool IsManagerInitialized() const override;
int InProgressCount() const override; int InProgressCount() const override;
int NonMaliciousInProgressCount() const override; int NonMaliciousInProgressCount() const override;
...@@ -253,6 +253,10 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager, ...@@ -253,6 +253,10 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager,
// True if the download manager has been initialized and loaded all the data. // True if the download manager has been initialized and loaded all the data.
bool initialized_; bool initialized_;
// Whether the history db and/or in progress cache are initialized.
bool history_db_initialized_;
bool in_progress_cache_initialized_;
// Observers that want to be notified of changes to the set of downloads. // Observers that want to be notified of changes to the set of downloads.
base::ObserverList<Observer> observers_; base::ObserverList<Observer> observers_;
......
...@@ -173,8 +173,17 @@ class CONTENT_EXPORT DownloadManager : public base::SupportsUserData::Data { ...@@ -173,8 +173,17 @@ class CONTENT_EXPORT DownloadManager : public base::SupportsUserData::Data {
bool transient, bool transient,
const std::vector<DownloadItem::ReceivedSlice>& received_slices) = 0; const std::vector<DownloadItem::ReceivedSlice>& received_slices) = 0;
// Called when download manager has loaded all the data. // Enum to describe which dependency was initialized in PostInitialization.
virtual void PostInitialization() = 0; enum DownloadInitializationDependency {
DOWNLOAD_INITIALIZATION_DEPENDENCY_NONE,
DOWNLOAD_INITIALIZATION_DEPENDENCY_HISTORY_DB,
DOWNLOAD_INITIALIZATION_DEPENDENCY_IN_PROGRESS_CACHE,
};
// Called when download manager has loaded all the data, once when the history
// db is initialized and once when the in-progress cache is initialized.
virtual void PostInitialization(
DownloadInitializationDependency dependency) = 0;
// Returns if the manager has been initialized and loaded all the data. // Returns if the manager has been initialized and loaded all the data.
virtual bool IsManagerInitialized() const = 0; virtual bool IsManagerInitialized() const = 0;
......
...@@ -148,7 +148,8 @@ class MockDownloadManager : public DownloadManager { ...@@ -148,7 +148,8 @@ class MockDownloadManager : public DownloadManager {
MOCK_METHOD1(MockCreateDownloadItem, MOCK_METHOD1(MockCreateDownloadItem,
DownloadItem*(CreateDownloadItemAdapter adapter)); DownloadItem*(CreateDownloadItemAdapter adapter));
MOCK_METHOD0(PostInitialization, void()); MOCK_METHOD1(PostInitialization,
void(DownloadInitializationDependency dependency));
MOCK_CONST_METHOD0(IsManagerInitialized, bool()); MOCK_CONST_METHOD0(IsManagerInitialized, bool());
MOCK_CONST_METHOD0(InProgressCount, int()); MOCK_CONST_METHOD0(InProgressCount, int());
MOCK_CONST_METHOD0(NonMaliciousInProgressCount, int()); MOCK_CONST_METHOD0(NonMaliciousInProgressCount, int());
......
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