diff options
Diffstat (limited to 'chromium/components/download')
69 files changed, 1600 insertions, 224 deletions
diff --git a/chromium/components/download/content/factory/download_service_factory.h b/chromium/components/download/content/factory/download_service_factory.h index a1142c179e5..0f7a1dc6182 100644 --- a/chromium/components/download/content/factory/download_service_factory.h +++ b/chromium/components/download/content/factory/download_service_factory.h @@ -10,6 +10,7 @@ #include "base/files/file_path.h" #include "base/memory/ref_counted.h" #include "base/sequenced_task_runner.h" +#include "base/single_thread_task_runner.h" #include "components/download/internal/background_service/blob_task_proxy.h" #include "components/download/public/background_service/clients.h" diff --git a/chromium/components/download/content/internal/download_driver_impl.cc b/chromium/components/download/content/internal/download_driver_impl.cc index 050d79d7922..f0d9a07cfa1 100644 --- a/chromium/components/download/content/internal/download_driver_impl.cc +++ b/chromium/components/download/content/internal/download_driver_impl.cc @@ -84,7 +84,7 @@ DriverEntry DownloadDriverImpl::CreateDriverEntry( : item->GetFullPath(); entry.completion_time = item->GetEndTime(); entry.response_headers = item->GetResponseHeaders(); - if (entry.response_headers.get()) { + if (entry.response_headers) { entry.can_resume = entry.response_headers->HasHeaderValue("Accept-Ranges", "bytes") || (entry.response_headers->HasHeader("Content-Range") && @@ -171,7 +171,9 @@ void DownloadDriverImpl::Start( download::DownloadSource::INTERNAL_API); download_url_params->set_post_body(post_body); - download_manager_->DownloadUrl(std::move(download_url_params), nullptr); + download_manager_->DownloadUrl(std::move(download_url_params), + nullptr /* blob_data_handle */, + nullptr /* blob_url_loader_factory */); } void DownloadDriverImpl::Remove(const std::string& guid) { diff --git a/chromium/components/download/content/public/all_download_item_notifier_unittest.cc b/chromium/components/download/content/public/all_download_item_notifier_unittest.cc index 98a9394b551..b893c8ab19a 100644 --- a/chromium/components/download/content/public/all_download_item_notifier_unittest.cc +++ b/chromium/components/download/content/public/all_download_item_notifier_unittest.cc @@ -20,7 +20,7 @@ namespace { class MockNotifierObserver : public AllDownloadItemNotifier::Observer { public: MockNotifierObserver() {} - virtual ~MockNotifierObserver() {} + ~MockNotifierObserver() override {} MOCK_METHOD2(OnDownloadCreated, void(content::DownloadManager* manager, DownloadItem* item)); @@ -40,9 +40,9 @@ class AllDownloadItemNotifierTest : public testing::Test { AllDownloadItemNotifierTest() : download_manager_(new content::MockDownloadManager) {} - virtual ~AllDownloadItemNotifierTest() {} + ~AllDownloadItemNotifierTest() override {} - content::MockDownloadManager& manager() { return *download_manager_.get(); } + content::MockDownloadManager& manager() { return *download_manager_; } download::MockDownloadItem& item() { return item_; } @@ -57,7 +57,7 @@ class AllDownloadItemNotifierTest : public testing::Test { MockNotifierObserver& observer() { return observer_; } void SetNotifier() { - EXPECT_CALL(*download_manager_.get(), AddObserver(_)); + EXPECT_CALL(*download_manager_, AddObserver(_)); notifier_.reset( new AllDownloadItemNotifier(download_manager_.get(), &observer_)); } diff --git a/chromium/components/download/downloader/in_progress/BUILD.gn b/chromium/components/download/downloader/in_progress/BUILD.gn index bd728e0ec35..fea2be16b81 100644 --- a/chromium/components/download/downloader/in_progress/BUILD.gn +++ b/chromium/components/download/downloader/in_progress/BUILD.gn @@ -9,13 +9,21 @@ if (is_android) { source_set("in_progress") { sources = [ + "download_db_entry.cc", + "download_db_entry.h", "download_entry.cc", "download_entry.h", + "download_info.cc", + "download_info.h", "in_progress_cache.h", "in_progress_cache_impl.cc", "in_progress_cache_impl.h", "in_progress_conversions.cc", "in_progress_conversions.h", + "in_progress_info.cc", + "in_progress_info.h", + "ukm_info.cc", + "ukm_info.h", ] deps = [ @@ -23,6 +31,7 @@ source_set("in_progress") { "//components/download/downloader/in_progress/proto", "//net", "//services/metrics/public/cpp:metrics_cpp", + "//services/network/public/mojom", ] } diff --git a/chromium/components/download/downloader/in_progress/download_db_entry.cc b/chromium/components/download/downloader/in_progress/download_db_entry.cc new file mode 100644 index 00000000000..b36192a39a6 --- /dev/null +++ b/chromium/components/download/downloader/in_progress/download_db_entry.cc @@ -0,0 +1,19 @@ +// Copyright 2018 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/download_db_entry.h" + +namespace download { + +DownloadDBEntry::DownloadDBEntry() = default; + +DownloadDBEntry::DownloadDBEntry(const DownloadDBEntry& other) = default; + +DownloadDBEntry::~DownloadDBEntry() = default; + +bool DownloadDBEntry::operator==(const DownloadDBEntry& other) const { + return id == other.id && download_info == other.download_info; +} + +} // namespace download diff --git a/chromium/components/download/downloader/in_progress/download_db_entry.h b/chromium/components/download/downloader/in_progress/download_db_entry.h new file mode 100644 index 00000000000..0aee2cf6175 --- /dev/null +++ b/chromium/components/download/downloader/in_progress/download_db_entry.h @@ -0,0 +1,33 @@ +// Copyright 2018 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_DOWNLOADER_IN_PROGRESS_DOWNLOAD_DB_ENTRY_H_ +#define COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_DOWNLOAD_DB_ENTRY_H_ + +#include <string> + +#include "base/optional.h" +#include "components/download/downloader/in_progress/download_info.h" + +namespace download { + +// Representing one entry in the DownloadDB. +struct DownloadDBEntry { + public: + DownloadDBEntry(); + DownloadDBEntry(const DownloadDBEntry& other); + ~DownloadDBEntry(); + + bool operator==(const DownloadDBEntry& other) const; + + // ID of the entry, this should be namespace + GUID of the download. + std::string id; + + // Information about a regular download. + base::Optional<DownloadInfo> download_info; +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_DOWNLOAD_DB_ENTRY_H_ diff --git a/chromium/components/download/downloader/in_progress/download_entry.h b/chromium/components/download/downloader/in_progress/download_entry.h index 4881ae859a8..c06ac01e531 100644 --- a/chromium/components/download/downloader/in_progress/download_entry.h +++ b/chromium/components/download/downloader/in_progress/download_entry.h @@ -7,6 +7,7 @@ #include <string> +#include "components/download/public/common/download_item.h" #include "components/download/public/common/download_source.h" #include "components/download/public/common/download_url_parameters.h" #include "services/metrics/public/cpp/ukm_source_id.h" diff --git a/chromium/components/download/downloader/in_progress/download_info.cc b/chromium/components/download/downloader/in_progress/download_info.cc new file mode 100644 index 00000000000..80b7f6257d6 --- /dev/null +++ b/chromium/components/download/downloader/in_progress/download_info.cc @@ -0,0 +1,20 @@ +// Copyright 2018 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/download_info.h" + +namespace download { + +DownloadInfo::DownloadInfo() = default; + +DownloadInfo::DownloadInfo(const DownloadInfo& other) = default; + +DownloadInfo::~DownloadInfo() = default; + +bool DownloadInfo::operator==(const DownloadInfo& other) const { + return guid == other.guid && ukm_info == other.ukm_info && + in_progress_info == other.in_progress_info; +} + +} // namespace download diff --git a/chromium/components/download/downloader/in_progress/download_info.h b/chromium/components/download/downloader/in_progress/download_info.h new file mode 100644 index 00000000000..80085ee1406 --- /dev/null +++ b/chromium/components/download/downloader/in_progress/download_info.h @@ -0,0 +1,37 @@ +// Copyright 2018 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_DOWNLOADER_IN_PROGRESS_DOWNLOAD_INFO_H_ +#define COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_DOWNLOAD_INFO_H_ + +#include <string> + +#include "base/optional.h" +#include "components/download/downloader/in_progress/in_progress_info.h" +#include "components/download/downloader/in_progress/ukm_info.h" + +namespace download { + +// Contains needed information to reconstruct a download item. +struct DownloadInfo { + public: + DownloadInfo(); + DownloadInfo(const DownloadInfo& other); + ~DownloadInfo(); + + bool operator==(const DownloadInfo& other) const; + + // Download GUID. + std::string guid; + + // UKM information for reporting. + base::Optional<UkmInfo> ukm_info; + + // In progress information for active download. + base::Optional<InProgressInfo> in_progress_info; +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_DOWNLOAD_INFO_H_ diff --git a/chromium/components/download/downloader/in_progress/in_progress_cache.h b/chromium/components/download/downloader/in_progress/in_progress_cache.h index af2c859929d..02459c0e026 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_cache.h +++ b/chromium/components/download/downloader/in_progress/in_progress_cache.h @@ -12,6 +12,8 @@ namespace download { +extern const base::FilePath::CharType kDownloadMetadataStoreFilename[]; + // 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 @@ -21,7 +23,7 @@ class InProgressCache { virtual ~InProgressCache() = default; // Initializes the cache. - virtual void Initialize(const base::RepeatingClosure& callback) = 0; + virtual void Initialize(base::OnceClosure callback) = 0; // Adds or updates an existing entry. virtual void AddOrReplaceEntry(const DownloadEntry& entry) = 0; diff --git a/chromium/components/download/downloader/in_progress/in_progress_cache_impl.cc b/chromium/components/download/downloader/in_progress/in_progress_cache_impl.cc index 5640de21ab9..6088307f523 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_cache_impl.cc +++ b/chromium/components/download/downloader/in_progress/in_progress_cache_impl.cc @@ -14,6 +14,9 @@ namespace download { +const base::FilePath::CharType kDownloadMetadataStoreFilename[] = + FILE_PATH_LITERAL("in_progress_download_metadata_store"); + namespace { // Helper functions for |entries_| related operations. @@ -57,6 +60,9 @@ void RemoveEntryFromEntries(metadata_pb::DownloadEntries& entries, // Helper functions for file read/write operations. std::vector<char> ReadEntriesFromFile(base::FilePath file_path) { + if (file_path.empty()) + return std::vector<char>(); + // Check validity of file. base::File entries_file(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ); @@ -110,7 +116,8 @@ std::string EntriesToString(const metadata_pb::DownloadEntries& entries) { } void WriteEntriesToFile(const std::string& entries, base::FilePath file_path) { - DCHECK(!file_path.empty()); + if (file_path.empty()) + return; if (!base::ImportantFileWriter::WriteFileAtomically(file_path, entries)) { LOG(ERROR) << "Could not write download entries to file: " @@ -129,26 +136,26 @@ InProgressCacheImpl::InProgressCacheImpl( InProgressCacheImpl::~InProgressCacheImpl() = default; -void InProgressCacheImpl::Initialize(const base::RepeatingClosure& callback) { +void InProgressCacheImpl::Initialize(base::OnceClosure callback) { // If it's already initialized, just run the callback. if (initialization_status_ == CACHE_INITIALIZED) { - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback); + base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, + std::move(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())); + weak_ptr_factory_.GetWeakPtr(), std::move(callback))); } } -void InProgressCacheImpl::OnInitialized(const std::vector<char>& entries) { +void InProgressCacheImpl::OnInitialized(base::OnceClosure callback, + const std::vector<char>& entries) { if (!entries.empty()) { if (!entries_.ParseFromArray(entries.data(), entries.size())) { // TODO(crbug.com/778425): Get UMA for errors. @@ -160,11 +167,7 @@ void InProgressCacheImpl::OnInitialized(const std::vector<char>& entries) { initialization_status_ = CACHE_INITIALIZED; - while (!pending_actions_.empty()) { - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, - pending_actions_.front()); - pending_actions_.pop_front(); - } + std::move(callback).Run(); } void InProgressCacheImpl::AddOrReplaceEntry(const DownloadEntry& entry) { diff --git a/chromium/components/download/downloader/in_progress/in_progress_cache_impl.h b/chromium/components/download/downloader/in_progress/in_progress_cache_impl.h index 6dd5fcea7f9..23687e0b7b8 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_cache_impl.h +++ b/chromium/components/download/downloader/in_progress/in_progress_cache_impl.h @@ -7,7 +7,6 @@ #include <string> -#include "base/containers/circular_deque.h" #include "base/files/file_path.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" @@ -24,13 +23,13 @@ namespace download { // right away. class InProgressCacheImpl : public InProgressCache { public: - explicit InProgressCacheImpl( + 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 Initialize(base::OnceClosure callback) override; void AddOrReplaceEntry(const DownloadEntry& entry) override; base::Optional<DownloadEntry> RetrieveEntry(const std::string& guid) override; void RemoveEntry(const std::string& guid) override; @@ -44,12 +43,12 @@ class InProgressCacheImpl : public InProgressCache { }; // Steps to execute after initialization is complete. - void OnInitialized(const std::vector<char>& entries); + void OnInitialized(base::OnceClosure callback, + const 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_; diff --git a/chromium/components/download/downloader/in_progress/in_progress_cache_impl_unittest.cc b/chromium/components/download/downloader/in_progress/in_progress_cache_impl_unittest.cc index 12012af4938..8150815bf31 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_cache_impl_unittest.cc +++ b/chromium/components/download/downloader/in_progress/in_progress_cache_impl_unittest.cc @@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/run_loop.h" +#include "base/sequenced_task_runner.h" #include "base/task_runner.h" #include "base/task_scheduler/post_task.h" #include "base/test/scoped_task_environment.h" diff --git a/chromium/components/download/downloader/in_progress/in_progress_conversions.cc b/chromium/components/download/downloader/in_progress/in_progress_conversions.cc index a5e3c8d819f..5620961d2c6 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_conversions.cc +++ b/chromium/components/download/downloader/in_progress/in_progress_conversions.cc @@ -6,6 +6,7 @@ #include <utility> #include "base/logging.h" +#include "base/pickle.h" namespace download { @@ -37,7 +38,6 @@ metadata_pb::DownloadEntry InProgressConversions::DownloadEntryToProto( auto* proto_header = proto.add_request_headers(); *proto_header = HttpRequestHeaderToProto(header); } - return proto; } @@ -139,4 +139,151 @@ InProgressConversions::HttpRequestHeaderFromProto( return std::make_pair(proto.key(), proto.value()); } +// static +metadata_pb::InProgressInfo InProgressConversions::InProgressInfoToProto( + const InProgressInfo& in_progress_info) { + metadata_pb::InProgressInfo proto; + for (size_t i = 0; i < in_progress_info.url_chain.size(); ++i) + proto.add_url_chain(in_progress_info.url_chain[i].spec()); + proto.set_fetch_error_body(in_progress_info.fetch_error_body); + for (const auto& header : in_progress_info.request_headers) { + auto* proto_header = proto.add_request_headers(); + *proto_header = HttpRequestHeaderToProto(header); + } + proto.set_etag(in_progress_info.etag); + proto.set_last_modified(in_progress_info.last_modified); + proto.set_total_bytes(in_progress_info.total_bytes); + base::Pickle current_path; + in_progress_info.current_path.WriteToPickle(¤t_path); + proto.set_current_path(current_path.data(), current_path.size()); + base::Pickle target_path; + in_progress_info.target_path.WriteToPickle(&target_path); + proto.set_target_path(target_path.data(), target_path.size()); + proto.set_received_bytes(in_progress_info.received_bytes); + proto.set_end_time( + in_progress_info.end_time.ToDeltaSinceWindowsEpoch().InMilliseconds()); + for (size_t i = 0; i < in_progress_info.received_slices.size(); ++i) { + metadata_pb::ReceivedSlice* slice = proto.add_received_slices(); + slice->set_received_bytes( + in_progress_info.received_slices[i].received_bytes); + slice->set_offset(in_progress_info.received_slices[i].offset); + slice->set_finished(in_progress_info.received_slices[i].finished); + } + proto.set_hash(in_progress_info.hash); + proto.set_transient(in_progress_info.transient); + proto.set_state(in_progress_info.state); + proto.set_danger_type(in_progress_info.danger_type); + proto.set_interrupt_reason(in_progress_info.interrupt_reason); + proto.set_paused(in_progress_info.paused); + proto.set_metered(in_progress_info.metered); + proto.set_request_origin(in_progress_info.request_origin); + proto.set_bytes_wasted(in_progress_info.bytes_wasted); + return proto; +} + +// static +InProgressInfo InProgressConversions::InProgressInfoFromProto( + const metadata_pb::InProgressInfo& proto) { + InProgressInfo info; + for (const auto& url : proto.url_chain()) + info.url_chain.emplace_back(url); + info.fetch_error_body = proto.fetch_error_body(); + for (const auto& header : proto.request_headers()) + info.request_headers.emplace_back(HttpRequestHeaderFromProto(header)); + info.etag = proto.etag(); + info.last_modified = proto.last_modified(); + info.total_bytes = proto.total_bytes(); + base::PickleIterator current_path( + base::Pickle(proto.current_path().data(), proto.current_path().size())); + info.current_path.ReadFromPickle(¤t_path); + base::PickleIterator target_path( + base::Pickle(proto.target_path().data(), proto.target_path().size())); + info.target_path.ReadFromPickle(&target_path); + info.received_bytes = proto.received_bytes(); + info.end_time = base::Time::FromDeltaSinceWindowsEpoch( + base::TimeDelta::FromMilliseconds(proto.end_time())); + + for (int i = 0; i < proto.received_slices_size(); ++i) { + info.received_slices.emplace_back(proto.received_slices(i).offset(), + proto.received_slices(i).received_bytes(), + proto.received_slices(i).finished()); + } + info.hash = proto.hash(); + info.transient = proto.transient(); + info.state = static_cast<DownloadItem::DownloadState>(proto.state()); + info.danger_type = static_cast<DownloadDangerType>(proto.danger_type()); + info.interrupt_reason = + static_cast<DownloadInterruptReason>(proto.interrupt_reason()); + info.paused = proto.paused(); + info.metered = proto.metered(); + info.request_origin = proto.request_origin(); + info.bytes_wasted = proto.bytes_wasted(); + return info; +} + +UkmInfo InProgressConversions::UkmInfoFromProto( + const metadata_pb::UkmInfo& proto) { + UkmInfo info; + info.download_source = DownloadSourceFromProto(proto.download_source()); + info.ukm_download_id = proto.ukm_download_id(); + return info; +} + +metadata_pb::UkmInfo InProgressConversions::UkmInfoToProto( + const UkmInfo& info) { + metadata_pb::UkmInfo proto; + proto.set_download_source(DownloadSourceToProto(info.download_source)); + proto.set_ukm_download_id(info.ukm_download_id); + return proto; +} + +DownloadInfo InProgressConversions::DownloadInfoFromProto( + const metadata_pb::DownloadInfo& proto) { + DownloadInfo info; + info.guid = proto.guid(); + if (proto.has_ukm_info()) + info.ukm_info = UkmInfoFromProto(proto.ukm_info()); + if (proto.has_in_progress_info()) + info.in_progress_info = InProgressInfoFromProto(proto.in_progress_info()); + return info; +} + +metadata_pb::DownloadInfo InProgressConversions::DownloadInfoToProto( + const DownloadInfo& info) { + metadata_pb::DownloadInfo proto; + proto.set_guid(info.guid); + if (info.ukm_info.has_value()) { + auto ukm_info = std::make_unique<metadata_pb::UkmInfo>( + UkmInfoToProto(info.ukm_info.value())); + proto.set_allocated_ukm_info(ukm_info.release()); + } + if (info.in_progress_info.has_value()) { + auto in_progress_info = std::make_unique<metadata_pb::InProgressInfo>( + InProgressInfoToProto(info.in_progress_info.value())); + proto.set_allocated_in_progress_info(in_progress_info.release()); + } + return proto; +} + +DownloadDBEntry InProgressConversions::DownloadDBEntryFromProto( + const metadata_pb::DownloadDBEntry& proto) { + DownloadDBEntry entry; + entry.id = proto.id(); + if (proto.has_download_info()) + entry.download_info = DownloadInfoFromProto(proto.download_info()); + return entry; +} + +metadata_pb::DownloadDBEntry InProgressConversions::DownloadDBEntryToProto( + const DownloadDBEntry& info) { + metadata_pb::DownloadDBEntry proto; + proto.set_id(info.id); + if (info.download_info.has_value()) { + auto download_info = std::make_unique<metadata_pb::DownloadInfo>( + DownloadInfoToProto(info.download_info.value())); + proto.set_allocated_download_info(download_info.release()); + } + return proto; +} + } // namespace download diff --git a/chromium/components/download/downloader/in_progress/in_progress_conversions.h b/chromium/components/download/downloader/in_progress/in_progress_conversions.h index d5b97cc8497..e95f6084bba 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_conversions.h +++ b/chromium/components/download/downloader/in_progress/in_progress_conversions.h @@ -6,9 +6,13 @@ #define COMPONENTS_DOWNLOAD_IN_PROGRESS_IN_PROGRESS_CONVERSIONS_H_ #include "base/macros.h" +#include "components/download/downloader/in_progress/download_db_entry.h" #include "components/download/downloader/in_progress/download_entry.h" +#include "components/download/downloader/in_progress/download_info.h" +#include "components/download/downloader/in_progress/in_progress_info.h" #include "components/download/downloader/in_progress/proto/download_entry.pb.h" #include "components/download/downloader/in_progress/proto/download_source.pb.h" +#include "components/download/downloader/in_progress/ukm_info.h" namespace download { @@ -37,6 +41,28 @@ class InProgressConversions { static std::pair<std::string, std::string> HttpRequestHeaderFromProto( const metadata_pb::HttpRequestHeader& proto); + + static metadata_pb::InProgressInfo InProgressInfoToProto( + const InProgressInfo& in_progress_info); + + static InProgressInfo InProgressInfoFromProto( + const metadata_pb::InProgressInfo& proto); + + static metadata_pb::UkmInfo UkmInfoToProto(const UkmInfo& ukm_info); + + static UkmInfo UkmInfoFromProto(const metadata_pb::UkmInfo& proto); + + static metadata_pb::DownloadInfo DownloadInfoToProto( + const DownloadInfo& download_info); + + static DownloadInfo DownloadInfoFromProto( + const metadata_pb::DownloadInfo& proto); + + static metadata_pb::DownloadDBEntry DownloadDBEntryToProto( + const DownloadDBEntry& entry); + + static DownloadDBEntry DownloadDBEntryFromProto( + const metadata_pb::DownloadDBEntry& proto); }; } // namespace download diff --git a/chromium/components/download/downloader/in_progress/in_progress_conversions_unittest.cc b/chromium/components/download/downloader/in_progress/in_progress_conversions_unittest.cc index 4fcda352b45..df5fb673b1c 100644 --- a/chromium/components/download/downloader/in_progress/in_progress_conversions_unittest.cc +++ b/chromium/components/download/downloader/in_progress/in_progress_conversions_unittest.cc @@ -9,6 +9,48 @@ namespace download { +namespace { + +InProgressInfo CreateInProgressInfo() { + InProgressInfo info; + // InProgressInfo with valid fields. + info.current_path = base::FilePath(FILE_PATH_LITERAL("/tmp.crdownload")); + info.target_path = base::FilePath(FILE_PATH_LITERAL("/tmp")); + info.url_chain.emplace_back("http://foo"); + info.url_chain.emplace_back("http://foo2"); + info.end_time = base::Time::NowFromSystemTime().LocalMidnight(); + info.etag = "A"; + info.last_modified = "Wed, 1 Oct 2018 07:00:00 GMT"; + info.received_bytes = 1000; + info.total_bytes = 10000; + info.state = DownloadItem::IN_PROGRESS; + info.danger_type = DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS; + info.interrupt_reason = DOWNLOAD_INTERRUPT_REASON_NONE; + info.transient = false; + info.paused = false; + info.hash = "abcdefg"; + info.metered = true; + info.received_slices.emplace_back(0, 500, false); + info.received_slices.emplace_back(5000, 500, false); + info.request_origin = "request origin"; + info.bytes_wasted = 1234; + info.fetch_error_body = true; + info.request_headers.emplace_back( + std::make_pair<std::string, std::string>("123", "456")); + info.request_headers.emplace_back( + std::make_pair<std::string, std::string>("ABC", "def")); + return info; +} + +DownloadInfo CreateDownloadInfo() { + DownloadInfo info; + info.in_progress_info = CreateInProgressInfo(); + info.ukm_info = UkmInfo(DownloadSource::FROM_RENDERER, 100); + return info; +} + +} // namespace + class InProgressConversionsTest : public testing::Test, public InProgressConversions { public: @@ -79,4 +121,38 @@ TEST_F(InProgressConversionsTest, HttpRequestHeaders) { HttpRequestHeaderFromProto(HttpRequestHeaderToProto(header))); } +TEST_F(InProgressConversionsTest, InProgressInfo) { + // InProgressInfo with no fields. + InProgressInfo info; + EXPECT_EQ(false, info.fetch_error_body); + EXPECT_TRUE(info.request_headers.empty()); + EXPECT_EQ(info, InProgressInfoFromProto(InProgressInfoToProto(info))); + + // InProgressInfo with valid fields. + info = CreateInProgressInfo(); + EXPECT_EQ(info, InProgressInfoFromProto(InProgressInfoToProto(info))); +} + +TEST_F(InProgressConversionsTest, UkmInfo) { + UkmInfo info(DownloadSource::FROM_RENDERER, 100); + EXPECT_EQ(info, UkmInfoFromProto(UkmInfoToProto(info))); +} + +TEST_F(InProgressConversionsTest, DownloadInfo) { + DownloadInfo info; + EXPECT_EQ(info, DownloadInfoFromProto(DownloadInfoToProto(info))); + + info = CreateDownloadInfo(); + EXPECT_EQ(info, DownloadInfoFromProto(DownloadInfoToProto(info))); +} + +TEST_F(InProgressConversionsTest, DownloadDBEntry) { + DownloadDBEntry entry; + EXPECT_EQ(entry, DownloadDBEntryFromProto(DownloadDBEntryToProto(entry))); + + entry.id = "abc"; + entry.download_info = CreateDownloadInfo(); + EXPECT_EQ(entry, DownloadDBEntryFromProto(DownloadDBEntryToProto(entry))); +} + } // namespace download diff --git a/chromium/components/download/downloader/in_progress/in_progress_info.cc b/chromium/components/download/downloader/in_progress/in_progress_info.cc new file mode 100644 index 00000000000..2a622e6da5f --- /dev/null +++ b/chromium/components/download/downloader/in_progress/in_progress_info.cc @@ -0,0 +1,32 @@ +// Copyright 2018 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_info.h" + +namespace download { + +InProgressInfo::InProgressInfo() = default; + +InProgressInfo::InProgressInfo(const InProgressInfo& other) = default; + +InProgressInfo::~InProgressInfo() = default; + +bool InProgressInfo::operator==(const InProgressInfo& other) const { + return url_chain == other.url_chain && + fetch_error_body == other.fetch_error_body && + request_headers == other.request_headers && etag == other.etag && + last_modified == other.last_modified && + total_bytes == other.total_bytes && + current_path == other.current_path && + target_path == other.target_path && + received_bytes == other.received_bytes && end_time == other.end_time && + received_slices == other.received_slices && hash == other.hash && + transient == other.transient && state == other.state && + danger_type == other.danger_type && + interrupt_reason == other.interrupt_reason && paused == other.paused && + metered == other.metered && request_origin == other.request_origin && + bytes_wasted == other.bytes_wasted; +} + +} // namespace download diff --git a/chromium/components/download/downloader/in_progress/in_progress_info.h b/chromium/components/download/downloader/in_progress/in_progress_info.h new file mode 100644 index 00000000000..b075e9bd336 --- /dev/null +++ b/chromium/components/download/downloader/in_progress/in_progress_info.h @@ -0,0 +1,105 @@ +// Copyright 2018 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_DOWNLOADER_IN_PROGRESS_IN_PROGRESS_INFO_H_ +#define COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_IN_PROGRESS_INFO_H_ + +#include <string> +#include <vector> + +#include "components/download/public/common/download_danger_type.h" +#include "components/download/public/common/download_item.h" +#include "components/download/public/common/download_url_parameters.h" +#include "url/gurl.h" + +namespace download { + +// Contains information to reconstruct an interrupted download item for +// resumption. +struct InProgressInfo { + public: + InProgressInfo(); + InProgressInfo(const InProgressInfo& other); + ~InProgressInfo(); + + bool operator==(const InProgressInfo& other) const; + + // request info ------------------------------------------------------------ + + // The url chain. + std::vector<GURL> url_chain; + + // If the entity body of unsuccessful HTTP response, like HTTP 404, will be + // downloaded. + bool fetch_error_body = false; + + // Request header key/value pairs that will be added to the download HTTP + // request. + DownloadUrlParameters::RequestHeadersType request_headers; + + // response info ----------------------------------------------------------- + + // Contents of most recently seen ETag header. + std::string etag; + + // Contents of most recently seen Last-Modified header. + std::string last_modified; + + // The total number of bytes in the download. + int64_t total_bytes = 0; + + // destination info -------------------------------------------------------- + + // The current path to the download (potentially different from final if + // download is in progress or interrupted). + base::FilePath current_path; + + // The target path where the download will go when it's complete. + base::FilePath target_path; + + // The number of bytes received (so far). + int64_t received_bytes = 0; + + // The time when the download completed. + base::Time end_time; + + // Data slices that have been downloaded so far. The slices must be ordered + // by their offset. + std::vector<DownloadItem::ReceivedSlice> received_slices; + + // Hash of the downloaded content. + std::string hash; + + // state info -------------------------------------------------------------- + + // Whether this download is transient. Transient items are cleaned up after + // completion and not shown in the UI. + bool transient = false; + + // The current state of the download. + DownloadItem::DownloadState state = DownloadItem::DownloadState::IN_PROGRESS; + + // Whether and how the download is dangerous. + DownloadDangerType danger_type = + DownloadDangerType::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS; + + // The reason the download was interrupted, if state == kStateInterrupted. + DownloadInterruptReason interrupt_reason = DOWNLOAD_INTERRUPT_REASON_NONE; + + // Whether this download is paused. + bool paused = false; + + // Whether the download is initiated on a metered network + bool metered = false; + + // Represents the origin information for this download. Used by offline pages. + std::string request_origin; + + // Count for how many (extra) bytes were used (including resumption). + int64_t bytes_wasted = 0; +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_IN_PROGRESS_INFO_H_ diff --git a/chromium/components/download/downloader/in_progress/proto/download_entry.proto b/chromium/components/download/downloader/in_progress/proto/download_entry.proto index 776b00600a4..ddf18d4a196 100644 --- a/chromium/components/download/downloader/in_progress/proto/download_entry.proto +++ b/chromium/components/download/downloader/in_progress/proto/download_entry.proto @@ -15,6 +15,13 @@ message HttpRequestHeader { optional string value = 2; } +// Slice information for parallel downloading. +message ReceivedSlice { + optional int64 offset = 1; + optional int64 received_bytes = 2; + optional bool finished = 3; +} + // Stores various in-progress metadata related to a download. message DownloadEntry { optional string guid = 1; @@ -30,3 +37,48 @@ message DownloadEntry { message DownloadEntries { repeated DownloadEntry entries = 1; } + +// Information for ukm reporting +message UkmInfo { + optional DownloadSource download_source = 1; + optional int64 ukm_download_id = 2; +} + +// Information about an in progress download. +message InProgressInfo { + repeated string url_chain = 1; + optional bool fetch_error_body = 2; + repeated HttpRequestHeader request_headers = 3; + optional string etag = 4; + optional string last_modified = 5; + optional int64 total_bytes = 6; + optional bytes current_path = 7; // Serialized pickles to support string16 + optional bytes target_path = 8; // Serialized pickles to support string16 + optional int64 received_bytes = 9; + optional int64 end_time = 10; + repeated ReceivedSlice received_slices = 11; + optional string hash = 12; + optional bool transient = 13; + optional int32 state = 14; + optional int32 danger_type = 15; + optional int32 interrupt_reason = 16; + optional bool paused = 17; + optional bool metered = 18; + optional string request_origin = 19; + optional int64 bytes_wasted = 20; +} + +// Stores various in-progress metadata related to a download. +// WIP and will replace DownloadEntry. +message DownloadInfo { + optional string guid = 1; + optional UkmInfo ukm_info = 2; + optional InProgressInfo in_progress_info = 3; +} + +// database entry for download information. +message DownloadDBEntry { + optional string id = 1; + // Add field for offline page download. + oneof entry { DownloadInfo download_info = 2; } +}
\ No newline at end of file diff --git a/chromium/components/download/downloader/in_progress/ukm_info.cc b/chromium/components/download/downloader/in_progress/ukm_info.cc new file mode 100644 index 00000000000..a02851403ac --- /dev/null +++ b/chromium/components/download/downloader/in_progress/ukm_info.cc @@ -0,0 +1,23 @@ +// Copyright 2018 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/ukm_info.h" + +namespace download { + +UkmInfo::UkmInfo() = default; + +UkmInfo::UkmInfo(const UkmInfo& other) = default; + +UkmInfo::UkmInfo(DownloadSource download_source, int64_t ukm_download_id) + : download_source(download_source), ukm_download_id(ukm_download_id) {} + +UkmInfo::~UkmInfo() = default; + +bool UkmInfo::operator==(const UkmInfo& other) const { + return download_source == other.download_source && + ukm_download_id == other.ukm_download_id; +} + +} // namespace download diff --git a/chromium/components/download/downloader/in_progress/ukm_info.h b/chromium/components/download/downloader/in_progress/ukm_info.h new file mode 100644 index 00000000000..ac3c73ff17d --- /dev/null +++ b/chromium/components/download/downloader/in_progress/ukm_info.h @@ -0,0 +1,34 @@ +// Copyright 2018 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_DOWNLOADER_IN_PROGRESS_UKM_INFO_H_ +#define COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_UKM_INFO_H_ + +#include <stdint.h> + +#include "components/download/public/common/download_source.h" + +namespace download { + +// Contains information for UKM reporting. +struct UkmInfo { + public: + UkmInfo(); + UkmInfo(DownloadSource download_source, int64_t ukm_download_id); + UkmInfo(const UkmInfo& other); + ~UkmInfo(); + + bool operator==(const UkmInfo& other) const; + + // The source that triggered the download. + DownloadSource download_source = DownloadSource::UNKNOWN; + + // Unique ID that tracks the download UKM entry, where 0 means the + // download_id is not yet initialized. + uint64_t ukm_download_id = 0; +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_DOWNLOADER_IN_PROGRESS_UKM_INFO_H_ diff --git a/chromium/components/download/internal/background_service/blob_task_proxy.cc b/chromium/components/download/internal/background_service/blob_task_proxy.cc index ab525937720..4c0813902d0 100644 --- a/chromium/components/download/internal/background_service/blob_task_proxy.cc +++ b/chromium/components/download/internal/background_service/blob_task_proxy.cc @@ -61,7 +61,7 @@ void BlobTaskProxy::SaveAsBlobOnIO(std::unique_ptr<std::string> data, // Build blob data. This has to do a copy into blob's internal storage. std::string blob_uuid = base::GenerateGUID(); auto builder = std::make_unique<storage::BlobDataBuilder>(blob_uuid); - builder->AppendData(*data.get()); + builder->AppendData(*data); blob_data_handle_ = blob_storage_context_->AddFinishedBlob(std::move(builder)); diff --git a/chromium/components/download/internal/background_service/controller_impl.cc b/chromium/components/download/internal/background_service/controller_impl.cc index 5b602187936..2150eca0b17 100644 --- a/chromium/components/download/internal/background_service/controller_impl.cc +++ b/chromium/components/download/internal/background_service/controller_impl.cc @@ -1051,7 +1051,7 @@ void ControllerImpl::KillTimedOutUploads() { void ControllerImpl::NotifyClientsOfStartup(bool state_lost) { auto categorized = util::MapEntriesToMetadataForClients( - clients_->GetRegisteredClients(), model_->PeekEntries()); + clients_->GetRegisteredClients(), model_->PeekEntries(), driver_.get()); for (auto client_id : clients_->GetRegisteredClients()) { base::ThreadTaskRunnerHandle::Get()->PostTask( diff --git a/chromium/components/download/internal/background_service/controller_impl_unittest.cc b/chromium/components/download/internal/background_service/controller_impl_unittest.cc index 66d8ff523da..1e4f96bab34 100644 --- a/chromium/components/download/internal/background_service/controller_impl_unittest.cc +++ b/chromium/components/download/internal/background_service/controller_impl_unittest.cc @@ -437,8 +437,8 @@ TEST_F(DownloadServiceControllerImplTest, SuccessfulInitWithExistingDownload) { std::vector<Entry> entries = {entry1, entry2, entry3}; std::vector<DownloadMetaData> expected_downloads = { - util::BuildDownloadMetaData(&entry1), - util::BuildDownloadMetaData(&entry2)}; + util::BuildDownloadMetaData(&entry1, driver_), + util::BuildDownloadMetaData(&entry2, driver_)}; EXPECT_CALL(*client_, OnServiceInitialized(false, testing::UnorderedElementsAreArray( diff --git a/chromium/components/download/internal/background_service/entry_utils.cc b/chromium/components/download/internal/background_service/entry_utils.cc index 8738cb02350..4402d4c9647 100644 --- a/chromium/components/download/internal/background_service/entry_utils.cc +++ b/chromium/components/download/internal/background_service/entry_utils.cc @@ -6,6 +6,7 @@ #include <algorithm> +#include "components/download/internal/background_service/download_driver.h" #include "components/download/internal/background_service/entry.h" #include "components/download/public/background_service/download_metadata.h" @@ -24,7 +25,8 @@ uint32_t GetNumberOfLiveEntriesForClient(DownloadClient client, std::map<DownloadClient, std::vector<DownloadMetaData>> MapEntriesToMetadataForClients(const std::set<DownloadClient>& clients, - const std::vector<Entry*>& entries) { + const std::vector<Entry*>& entries, + DownloadDriver* driver) { std::map<DownloadClient, std::vector<DownloadMetaData>> categorized; for (auto* entry : entries) { @@ -32,7 +34,7 @@ MapEntriesToMetadataForClients(const std::set<DownloadClient>& clients, if (clients.find(client) == clients.end()) client = DownloadClient::INVALID; - categorized[client].push_back(BuildDownloadMetaData(entry)); + categorized[client].push_back(BuildDownloadMetaData(entry, driver)); } return categorized; @@ -86,14 +88,22 @@ bool EntryBetterThan(const Entry& lhs, const Entry& rhs) { return lhs.create_time < rhs.create_time; } -DownloadMetaData BuildDownloadMetaData(Entry* entry) { +DownloadMetaData BuildDownloadMetaData(Entry* entry, DownloadDriver* driver) { DCHECK(entry); DownloadMetaData meta_data; meta_data.guid = entry->guid; if (entry->state == Entry::State::COMPLETE) { meta_data.completion_info = CompletionInfo(entry->target_file_path, entry->bytes_downloaded); + // If the download is completed, the |current_size| needs to pull from entry + // since the history db record has been deleted. + meta_data.current_size = entry->bytes_downloaded; + return meta_data; } + + auto driver_entry = driver->Find(entry->guid); + if (driver_entry) + meta_data.current_size = driver_entry->bytes_downloaded; return meta_data; } diff --git a/chromium/components/download/internal/background_service/entry_utils.h b/chromium/components/download/internal/background_service/entry_utils.h index 410383c50b6..a519d759e24 100644 --- a/chromium/components/download/internal/background_service/entry_utils.h +++ b/chromium/components/download/internal/background_service/entry_utils.h @@ -17,6 +17,7 @@ namespace download { +class DownloadDriver; struct DownloadMetaData; struct Entry; @@ -35,7 +36,8 @@ uint32_t GetNumberOfLiveEntriesForClient(DownloadClient client, // not be included. std::map<DownloadClient, std::vector<DownloadMetaData>> MapEntriesToMetadataForClients(const std::set<DownloadClient>& clients, - const std::vector<Entry*>& entries); + const std::vector<Entry*>& entries, + DownloadDriver* driver); // Gets the least strict scheduling criteria from |entries|, the criteria is // used to schedule platform background tasks. @@ -46,8 +48,9 @@ Criteria GetSchedulingCriteria(const Model::EntryList& entries, // |rhs| based on their priority and cancel time. bool EntryBetterThan(const Entry& lhs, const Entry& rhs); -// Builds a download meta data based on |entry|. -DownloadMetaData BuildDownloadMetaData(Entry* entry); +// Builds a download meta data based on |entry| and the states from |driver| in +// the history db. +DownloadMetaData BuildDownloadMetaData(Entry* entry, DownloadDriver* driver); } // namespace util } // namespace download diff --git a/chromium/components/download/internal/background_service/entry_utils_unittest.cc b/chromium/components/download/internal/background_service/entry_utils_unittest.cc index fda16a7f2a8..75f36afc8be 100644 --- a/chromium/components/download/internal/background_service/entry_utils_unittest.cc +++ b/chromium/components/download/internal/background_service/entry_utils_unittest.cc @@ -7,6 +7,7 @@ #include <algorithm> #include "components/download/internal/background_service/test/entry_utils.h" +#include "components/download/internal/background_service/test/test_download_driver.h" #include "components/download/public/background_service/clients.h" #include "components/download/public/background_service/download_metadata.h" #include "testing/gtest/include/gtest/gtest.h" @@ -35,21 +36,23 @@ TEST(DownloadServiceEntryUtilsTest, MapEntriesToClients) { Entry entry5 = test::BuildBasicEntry(Entry::State::AVAILABLE); std::vector<Entry*> entries = {&entry1, &entry2, &entry3, &entry4, &entry5}; + test::TestDownloadDriver driver; std::vector<DownloadMetaData> expected_list = { - util::BuildDownloadMetaData(&entry1), - util::BuildDownloadMetaData(&entry2), - util::BuildDownloadMetaData(&entry3), - util::BuildDownloadMetaData(&entry4), - util::BuildDownloadMetaData(&entry5)}; + util::BuildDownloadMetaData(&entry1, &driver), + util::BuildDownloadMetaData(&entry2, &driver), + util::BuildDownloadMetaData(&entry3, &driver), + util::BuildDownloadMetaData(&entry4, &driver), + util::BuildDownloadMetaData(&entry5, &driver)}; std::vector<DownloadMetaData> expected_pruned_list = { - util::BuildDownloadMetaData(&entry1), - util::BuildDownloadMetaData(&entry2), - util::BuildDownloadMetaData(&entry3), - util::BuildDownloadMetaData(&entry5)}; + util::BuildDownloadMetaData(&entry1, &driver), + util::BuildDownloadMetaData(&entry2, &driver), + util::BuildDownloadMetaData(&entry3, &driver), + util::BuildDownloadMetaData(&entry5, &driver)}; + // If DownloadClient::TEST isn't a valid Client, all of the associated entries // should move to the DownloadClient::INVALID bucket. - auto mapped1 = - util::MapEntriesToMetadataForClients(std::set<DownloadClient>(), entries); + auto mapped1 = util::MapEntriesToMetadataForClients( + std::set<DownloadClient>(), entries, &driver); EXPECT_EQ(1U, mapped1.size()); EXPECT_NE(mapped1.end(), mapped1.find(DownloadClient::INVALID)); EXPECT_EQ(mapped1.end(), mapped1.find(DownloadClient::TEST)); @@ -62,7 +65,8 @@ TEST(DownloadServiceEntryUtilsTest, MapEntriesToClients) { // If DownloadClient::TEST is a valid Client, it should have the associated // entries. std::set<DownloadClient> clients = {DownloadClient::TEST}; - auto mapped2 = util::MapEntriesToMetadataForClients(clients, entries); + auto mapped2 = + util::MapEntriesToMetadataForClients(clients, entries, &driver); EXPECT_EQ(1U, mapped2.size()); EXPECT_NE(mapped2.end(), mapped2.find(DownloadClient::TEST)); EXPECT_EQ(mapped2.end(), mapped2.find(DownloadClient::INVALID)); @@ -136,15 +140,22 @@ TEST(DownloadServiceEntryUtilsTest, BuildDownloadMetaData) { Entry entry = test::BuildBasicEntry(Entry::State::PAUSED); entry.target_file_path = base::FilePath::FromUTF8Unsafe("123"); entry.bytes_downloaded = 200u; - auto meta_data = util::BuildDownloadMetaData(&entry); + test::TestDownloadDriver driver; + + auto meta_data = util::BuildDownloadMetaData(&entry, &driver); EXPECT_EQ(entry.guid, meta_data.guid); + // Incomplete downloads don't copy the following data. EXPECT_FALSE(meta_data.completion_info.has_value()); + // Current size is always pulled from driver, and we don't persist the current + // size in each OnDownloadUpdated call in DownloadDriver. + EXPECT_EQ(0u, meta_data.current_size); + entry = test::BuildBasicEntry(Entry::State::COMPLETE); entry.target_file_path = base::FilePath::FromUTF8Unsafe("123"); entry.bytes_downloaded = 100u; - meta_data = util::BuildDownloadMetaData(&entry); + meta_data = util::BuildDownloadMetaData(&entry, &driver); EXPECT_EQ(entry.guid, meta_data.guid); EXPECT_TRUE(meta_data.completion_info.has_value()); EXPECT_EQ(entry.target_file_path, meta_data.completion_info->path); @@ -152,4 +163,21 @@ TEST(DownloadServiceEntryUtilsTest, BuildDownloadMetaData) { meta_data.completion_info->bytes_downloaded); } +// Test to verify the current size is correctly built into the meta data. +TEST(DownloadServiceEntryUtilsTest, BuildDownloadMetaDataCurrentSize) { + Entry entry = test::BuildBasicEntry(Entry::State::PAUSED); + entry.target_file_path = base::FilePath::FromUTF8Unsafe("123"); + entry.bytes_downloaded = 0u; + + test::TestDownloadDriver driver; + DriverEntry driver_entry; + driver_entry.guid = entry.guid; + driver_entry.bytes_downloaded = 100u; + driver.AddTestData({driver_entry}); + + auto meta_data = util::BuildDownloadMetaData(&entry, &driver); + meta_data = util::BuildDownloadMetaData(&entry, &driver); + EXPECT_EQ(driver_entry.bytes_downloaded, meta_data.current_size); +} + } // namespace download diff --git a/chromium/components/download/internal/background_service/in_memory_download_driver.cc b/chromium/components/download/internal/background_service/in_memory_download_driver.cc index 99ba0970550..50eeda0460c 100644 --- a/chromium/components/download/internal/background_service/in_memory_download_driver.cc +++ b/chromium/components/download/internal/background_service/in_memory_download_driver.cc @@ -131,7 +131,7 @@ base::Optional<DriverEntry> InMemoryDownloadDriver::Find( base::Optional<DriverEntry> entry; auto it = downloads_.find(guid); if (it != downloads_.end()) - entry = CreateDriverEntry(*it->second.get()); + entry = CreateDriverEntry(*it->second); return entry; } diff --git a/chromium/components/download/internal/background_service/in_memory_download_driver.h b/chromium/components/download/internal/background_service/in_memory_download_driver.h index b1594999c72..c28df4c386e 100644 --- a/chromium/components/download/internal/background_service/in_memory_download_driver.h +++ b/chromium/components/download/internal/background_service/in_memory_download_driver.h @@ -11,6 +11,7 @@ #include <memory> #include "base/macros.h" +#include "base/single_thread_task_runner.h" #include "components/download/internal/background_service/in_memory_download.h" #include "services/network/public/mojom/url_loader_factory.mojom.h" diff --git a/chromium/components/download/internal/background_service/in_memory_download_unittest.cc b/chromium/components/download/internal/background_service/in_memory_download_unittest.cc index c89ae3a0cdc..e04ec21317e 100644 --- a/chromium/components/download/internal/background_service/in_memory_download_unittest.cc +++ b/chromium/components/download/internal/background_service/in_memory_download_unittest.cc @@ -44,7 +44,7 @@ class MockDelegate : public InMemoryDownload::Delegate { // InMemoryDownload::Delegate implementation. MOCK_METHOD1(OnDownloadProgress, void(InMemoryDownload*)); - void OnDownloadComplete(InMemoryDownload* download) { + void OnDownloadComplete(InMemoryDownload* download) override { if (run_loop_.running()) run_loop_.Quit(); } diff --git a/chromium/components/download/internal/background_service/scheduler/device_status_listener_unittest.cc b/chromium/components/download/internal/background_service/scheduler/device_status_listener_unittest.cc index 18ab129af56..74ceabe5538 100644 --- a/chromium/components/download/internal/background_service/scheduler/device_status_listener_unittest.cc +++ b/chromium/components/download/internal/background_service/scheduler/device_status_listener_unittest.cc @@ -34,8 +34,7 @@ MATCHER_P(BatteryStatusEqual, value, "") { class TestNetworkChangeNotifier : public net::NetworkChangeNotifier { public: TestNetworkChangeNotifier() - : net::NetworkChangeNotifier(), - conn_type_(ConnectionType::CONNECTION_UNKNOWN) {} + : conn_type_(ConnectionType::CONNECTION_UNKNOWN) {} // net::NetworkChangeNotifier implementation. ConnectionType GetCurrentConnectionType() const override { @@ -129,7 +128,7 @@ class DeviceStatusListenerTest : public testing::Test { // Simulates a network change call, the event will be sent to client // immediately. void ChangeNetworkTypeImmediately(ConnectionType type) { - DCHECK(listener_.get()); + DCHECK(listener_); static_cast<NetworkStatusListener::Observer*>(listener_.get()) ->OnNetworkChanged(type); } diff --git a/chromium/components/download/internal/common/BUILD.gn b/chromium/components/download/internal/common/BUILD.gn index bb3a9299559..42ed3007fc8 100644 --- a/chromium/components/download/internal/common/BUILD.gn +++ b/chromium/components/download/internal/common/BUILD.gn @@ -14,7 +14,6 @@ source_set("internal") { sources = [ "base_file.cc", - "base_file_posix.cc", "base_file_win.cc", "download_create_info.cc", "download_file_factory.cc", @@ -35,12 +34,14 @@ source_set("internal") { "download_utils.cc", "download_worker.cc", "download_worker.h", + "in_progress_download_manager.cc", "parallel_download_job.cc", "parallel_download_job.h", "parallel_download_utils.cc", "parallel_download_utils.h", "rate_estimator.cc", "resource_downloader.cc", + "resource_downloader.h", "save_package_download_job.cc", "save_package_download_job.h", "stream_handle_input_stream.cc", @@ -61,6 +62,10 @@ source_set("internal") { "//services/metrics/public/cpp:ukm_builders", "//services/network/public/cpp", ] + + if (is_posix || is_fuchsia) { + sources += [ "base_file_posix.cc" ] + } } # tests need to access both public and internal sources. So in the component diff --git a/chromium/components/download/internal/common/DEPS b/chromium/components/download/internal/common/DEPS index 65c70055f64..a5df1addd5a 100644 --- a/chromium/components/download/internal/common/DEPS +++ b/chromium/components/download/internal/common/DEPS @@ -6,10 +6,12 @@ include_rules = [ "+components/ukm/test_ukm_recorder.h", "+crypto", "+mojo/public/c/system", + "+mojo/public/cpp/bindings", "+net/base/filename_util.h", "+net/base/load_flags.h", "+net/base/io_buffer.h", "+net/base/net_errors.h", + "+net/cert/cert_status_flags.h", "+net/http/http_content_disposition.h", "+net/http/http_request_headers.h", "+net/http/http_response_headers.h", @@ -18,4 +20,5 @@ include_rules = [ "+net/traffic_annotation/network_traffic_annotation.h", "+services/metrics/public/cpp", "+services/network/public/cpp", + "+services/network/public/mojom", ] diff --git a/chromium/components/download/internal/common/base_file.cc b/chromium/components/download/internal/common/base_file.cc index fce4f343809..03416fb14a8 100644 --- a/chromium/components/download/internal/common/base_file.cc +++ b/chromium/components/download/internal/common/base_file.cc @@ -13,6 +13,7 @@ #include "base/format_macros.h" #include "base/logging.h" #include "base/macros.h" +#include "base/numerics/safe_conversions.h" #include "base/pickle.h" #include "base/strings/stringprintf.h" #include "base/threading/thread_restrictions.h" @@ -137,14 +138,6 @@ DownloadInterruptReason BaseFile::WriteDataToFile(int64_t offset, // belong to the same download will be grouped together. CONDITIONAL_TRACE( NESTABLE_ASYNC_BEGIN0("download", "DownloadFileWrite", download_id_)); - int write_result = file_.Write(offset, data, data_len); - DCHECK_NE(0, write_result); - - // Report errors on file writes. - if (write_result < 0) - return LogSystemError("Write", logging::GetLastSystemErrorCode()); - - DCHECK_EQ(static_cast<size_t>(write_result), data_len); if (bytes_so_far_ != offset) { // A hole is created in the file. @@ -152,7 +145,28 @@ DownloadInterruptReason BaseFile::WriteDataToFile(int64_t offset, secure_hash_.reset(); } - bytes_so_far_ += data_len; + // Writes to the file. + int64_t len = base::saturated_cast<int64_t>(data_len); + const char* current_data = data; + int64_t current_offset = offset; + while (len > 0) { + // |write_result| may be less than |len|, and return an error on the next + // write call when the disk is unavaliable. + int write_result = file_.Write(current_offset, current_data, len); + DCHECK_NE(0, write_result); + + // Report errors on file writes. + if (write_result < 0) + return LogSystemError("Write", logging::GetLastSystemErrorCode()); + + // Update status. + DCHECK_LE(write_result, len); + len -= write_result; + current_data += write_result; + current_offset += write_result; + bytes_so_far_ += write_result; + } + CONDITIONAL_TRACE(NESTABLE_ASYNC_END1("download", "DownloadFileWrite", download_id_, "bytes", data_len)); diff --git a/chromium/components/download/internal/common/base_file_win.cc b/chromium/components/download/internal/common/base_file_win.cc index df78b6956d5..11796902e9a 100644 --- a/chromium/components/download/internal/common/base_file_win.cc +++ b/chromium/components/download/internal/common/base_file_win.cc @@ -257,25 +257,22 @@ DownloadInterruptReason MapShFileOperationCodes(int code) { if (result == DOWNLOAD_INTERRUPT_REASON_FILE_FAILED) { UMA_HISTOGRAM_CUSTOM_ENUMERATION( "Download.MapWinShErrorFileFailed", code, - base::CustomHistogram::ArrayToCustomRanges( - kAllSpecialShFileOperationCodes, - arraysize(kAllSpecialShFileOperationCodes))); + base::CustomHistogram::ArrayToCustomEnumRanges( + kAllSpecialShFileOperationCodes)); } if (result == DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED) { UMA_HISTOGRAM_CUSTOM_ENUMERATION( "Download.MapWinShErrorAccessDenied", code, - base::CustomHistogram::ArrayToCustomRanges( - kAllSpecialShFileOperationCodes, - arraysize(kAllSpecialShFileOperationCodes))); + base::CustomHistogram::ArrayToCustomEnumRanges( + kAllSpecialShFileOperationCodes)); } if (result == DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR) { UMA_HISTOGRAM_CUSTOM_ENUMERATION( "Download.MapWinShErrorTransientError", code, - base::CustomHistogram::ArrayToCustomRanges( - kAllSpecialShFileOperationCodes, - arraysize(kAllSpecialShFileOperationCodes))); + base::CustomHistogram::ArrayToCustomEnumRanges( + kAllSpecialShFileOperationCodes)); } if (result != DOWNLOAD_INTERRUPT_REASON_NONE) @@ -302,7 +299,7 @@ DownloadInterruptReason BaseFile::MoveFileAndAdjustPermissions( source.append(1, L'\0'); target.append(1, L'\0'); - SHFILEOPSTRUCT move_info = {0}; + SHFILEOPSTRUCT move_info = {nullptr}; move_info.wFunc = FO_MOVE; move_info.pFrom = source.c_str(); move_info.pTo = target.c_str(); diff --git a/chromium/components/download/internal/common/download_file_impl.cc b/chromium/components/download/internal/common/download_file_impl.cc index 7acae0764dd..05d8f7c5c19 100644 --- a/chromium/components/download/internal/common/download_file_impl.cc +++ b/chromium/components/download/internal/common/download_file_impl.cc @@ -9,7 +9,7 @@ #include "base/bind.h" #include "base/files/file_util.h" -#include "base/message_loop/message_loop.h" +#include "base/message_loop/message_loop_current.h" #include "base/strings/stringprintf.h" #include "base/threading/sequenced_task_runner_handle.h" #include "base/time/time.h" @@ -147,7 +147,7 @@ DownloadFileImpl::DownloadFileImpl( bytes_seen_without_parallel_streams_(0), is_paused_(false), download_id_(download_id), - main_task_runner_(base::MessageLoop::current()->task_runner()), + main_task_runner_(base::MessageLoopCurrent::Get()->task_runner()), observer_(observer), weak_factory_(this) { TRACE_EVENT_INSTANT0("download", "DownloadFileCreated", @@ -229,7 +229,7 @@ void DownloadFileImpl::AddInputStream(std::unique_ptr<InputStream> stream, CancelRequest(offset); return; } - + DCHECK(source_streams_.find(offset) == source_streams_.end()); source_streams_[offset] = std::make_unique<SourceStream>(offset, length, std::move(stream)); OnSourceStreamAdded(source_streams_[offset].get()); @@ -504,7 +504,7 @@ void DownloadFileImpl::StreamActive(SourceStream* source_stream, DCHECK_GE(incoming_data_size, bytes_to_write); reason = WriteDataToFile( source_stream->offset() + source_stream->bytes_written(), - incoming_data.get()->data(), bytes_to_write); + incoming_data->data(), bytes_to_write); disk_writes_time_ += (base::TimeTicks::Now() - write_start); bytes_seen_ += bytes_to_write; total_incoming_data_size += bytes_to_write; diff --git a/chromium/components/download/internal/common/download_file_unittest.cc b/chromium/components/download/internal/common/download_file_unittest.cc index 91c9d212691..f0f2204d6a5 100644 --- a/chromium/components/download/internal/common/download_file_unittest.cc +++ b/chromium/components/download/internal/common/download_file_unittest.cc @@ -201,7 +201,7 @@ class DownloadFileTest : public testing::Test { bool calculate_hash, const DownloadItem::ReceivedSlices& received_slices) { // There can be only one. - DCHECK(!download_file_.get()); + DCHECK(!download_file_); input_stream_ = new StrictMock<MockInputStream>(); @@ -721,8 +721,8 @@ void TestRenameCompletionCallback(const base::Closure& closure, // succeed. // // Note that there is only one queue of tasks to run, and that is in the tests' -// base::MessageLoop::current(). Each RunLoop processes that queue until it sees -// a QuitClosure() targeted at itself, at which point it stops processing. +// base::MessageLoopCurrent::Get(). Each RunLoop processes that queue until it +// sees a QuitClosure() targeted at itself, at which point it stops processing. TEST_P(DownloadFileTestWithRename, RenameWithErrorRetry) { ASSERT_TRUE(CreateDownloadFile(0, true)); base::FilePath initial_path(download_file_->FullPath()); @@ -1042,8 +1042,8 @@ TEST_F(DownloadFileTest, SecondStreamStartingOffsetAlreadyWritten) { .RetiresOnSaturation(); download_file_->AddInputStream( - std::unique_ptr<MockInputStream>(additional_streams_[0]), 0, - DownloadSaveInfo::kLengthFullContent); + std::unique_ptr<MockInputStream>(additional_streams_[0]), + strlen(kTestData1), DownloadSaveInfo::kLengthFullContent); // The stream should get terminated and reset the callback. EXPECT_TRUE(sink_callback_.is_null()); diff --git a/chromium/components/download/internal/common/download_item_impl.cc b/chromium/components/download/internal/common/download_item_impl.cc index 7181a2ca5c8..817deacd108 100644 --- a/chromium/components/download/internal/common/download_item_impl.cc +++ b/chromium/components/download/internal/common/download_item_impl.cc @@ -147,6 +147,8 @@ std::string GetDownloadDangerNames(DownloadDangerType type) { return "DANGEROUS_HOST"; case DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED: return "POTENTIALLY_UNWANTED"; + case DOWNLOAD_DANGER_TYPE_WHITELISTED_BY_POLICY: + return "WHITELISTED_BY_POLICY"; default: NOTREACHED(); return "UNKNOWN_DANGER_TYPE"; @@ -418,7 +420,7 @@ DownloadItemImpl::~DownloadItemImpl() { // Should always have been nuked before now, at worst in // DownloadManager shutdown. - DCHECK(!download_file_.get()); + DCHECK(!download_file_); CHECK(!is_updating_observers_); for (auto& observer : observers_) @@ -957,6 +959,11 @@ bool DownloadItemImpl::IsTransient() const { return transient_; } +bool DownloadItemImpl::IsParallelDownload() const { + bool is_parallelizable = job_ ? job_->IsParallelizable() : false; + return is_parallelizable && download::IsParallelDownloadEnabled(); +} + void DownloadItemImpl::OnContentCheckCompleted(DownloadDangerType danger_type, DownloadInterruptReason reason) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -1040,7 +1047,7 @@ std::string DownloadItemImpl::DebugString(bool verbose) const { IsPaused() ? 'T' : 'F', DebugResumeModeString(GetResumeMode()), auto_resume_count_, GetDangerType(), AllDataSaved() ? 'T' : 'F', GetLastModifiedTime().c_str(), GetETag().c_str(), - download_file_.get() ? "true" : "false", url_list.c_str(), + download_file_ ? "true" : "false", url_list.c_str(), GetFullPath().value().c_str(), GetTargetFilePath().value().c_str(), GetReferrerUrl().spec().c_str(), GetSiteUrl().spec().c_str()); } else { @@ -1383,17 +1390,18 @@ void DownloadItemImpl::Start( std::unique_ptr<DownloadFile> file, std::unique_ptr<DownloadRequestHandleInterface> req_handle, const DownloadCreateInfo& new_create_info, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, net::URLRequestContextGetter* url_request_context_getter) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - DCHECK(!download_file_.get()); + DCHECK(!download_file_); DVLOG(20) << __func__ << "() this=" << DebugString(true); RecordDownloadCountWithSource(START_COUNT, download_source_); download_file_ = std::move(file); job_ = DownloadJobFactory::CreateJob( this, std::move(req_handle), new_create_info, false, - std::move(shared_url_loader_factory), url_request_context_getter); + std::move(url_loader_factory_getter), url_request_context_getter); if (job_->IsParallelizable()) { RecordParallelizableDownloadCount(START_COUNT, IsParallelDownloadEnabled()); } @@ -1422,23 +1430,20 @@ void DownloadItemImpl::Start( // If a resumption attempted failed, or if the download was DOA, then the // download should go back to being interrupted. if (new_create_info.result != DOWNLOAD_INTERRUPT_REASON_NONE) { - DCHECK(!download_file_.get()); + DCHECK(!download_file_); // Download requests that are interrupted by Start() should result in a // DownloadCreateInfo with an intact DownloadSaveInfo. DCHECK(new_create_info.save_info); - int64_t offset = new_create_info.save_info->offset; std::unique_ptr<crypto::SecureHash> hash_state = new_create_info.save_info->hash_state ? new_create_info.save_info->hash_state->Clone() : nullptr; - destination_info_.received_bytes = offset; hash_state_ = std::move(hash_state); destination_info_.hash.clear(); deferred_interrupt_reason_ = new_create_info.result; - received_slices_.clear(); TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL); DetermineDownloadTarget(); return; @@ -1473,9 +1478,8 @@ void DownloadItemImpl::Start( if (state_ == RESUMING_INTERNAL) UpdateValidatorsOnResumption(new_create_info); - // If the download is not parallel download during resumption, clear the - // |received_slices_|. - if (!job_->IsParallelizable() && !received_slices_.empty()) { + // If the download is not parallel, clear the |received_slices_|. + if (!received_slices_.empty() && !job_->IsParallelizable()) { destination_info_.received_bytes = GetMaxContiguousDataBlockSizeFromBeginning(received_slices_); received_slices_.clear(); @@ -1715,7 +1719,7 @@ void DownloadItemImpl::OnDownloadCompleting() { DCHECK(!GetTargetFilePath().empty()); DCHECK(!IsDangerous()); - DCHECK(download_file_.get()); + DCHECK(download_file_); // Unilaterally rename; even if it already has the right name, // we need theannotation. DownloadFile::RenameCompletionCallback callback = @@ -1727,7 +1731,9 @@ void DownloadItemImpl::OnDownloadCompleting() { base::Unretained(download_file_.get()), GetTargetFilePath(), delegate_->GetApplicationClientIdForFileScanning(), - GetURL(), GetReferrerUrl(), std::move(callback))); + delegate_->IsOffTheRecord() ? GURL() : GetURL(), + delegate_->IsOffTheRecord() ? GURL() : GetReferrerUrl(), + std::move(callback))); } void DownloadItemImpl::OnDownloadRenamedToFinalName( @@ -2229,7 +2235,8 @@ void DownloadItemImpl::SetDangerType(DownloadDangerType danger_type) { if ((danger_type_ == DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS || danger_type_ == DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE || danger_type_ == DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT || - danger_type_ == DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT) && + danger_type_ == DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT || + danger_type_ == DOWNLOAD_DANGER_TYPE_WHITELISTED_BY_POLICY) && (danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST || danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_URL || danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT || @@ -2341,6 +2348,9 @@ void DownloadItemImpl::ResumeInterruptedDownload( for (const auto& header : request_headers_) { download_params->add_request_header(header.first, header.second); } + // The offset is calculated after decompression, so the range request cannot + // involve any compression, + download_params->add_request_header("Accept-Encoding", "identity"); auto entry = delegate_->GetInProgressEntry(this); if (entry) diff --git a/chromium/components/download/internal/common/download_item_impl_unittest.cc b/chromium/components/download/internal/common/download_item_impl_unittest.cc index 8515eb822f6..1503ec33b07 100644 --- a/chromium/components/download/internal/common/download_item_impl_unittest.cc +++ b/chromium/components/download/internal/common/download_item_impl_unittest.cc @@ -66,7 +66,7 @@ base::HistogramBase::Sample ToHistogramSample(T t) { class MockDelegate : public DownloadItemImplDelegate { public: - MockDelegate() : DownloadItemImplDelegate() { SetDefaultExpectations(); } + MockDelegate() { SetDefaultExpectations(); } MOCK_METHOD2(DetermineDownloadTarget, void(DownloadItemImpl*, @@ -89,6 +89,7 @@ class MockDelegate : public DownloadItemImplDelegate { MOCK_METHOD1(DownloadOpened, void(DownloadItemImpl*)); MOCK_METHOD1(DownloadRemoved, void(DownloadItemImpl*)); MOCK_CONST_METHOD1(AssertStateConsistent, void(DownloadItemImpl*)); + MOCK_CONST_METHOD0(IsOffTheRecord, bool()); void VerifyAndClearExpectations() { ::testing::Mock::VerifyAndClearExpectations(this); @@ -101,6 +102,7 @@ class MockDelegate : public DownloadItemImplDelegate { EXPECT_CALL(*this, ShouldOpenFileBasedOnExtension(_)) .WillRepeatedly(Return(false)); EXPECT_CALL(*this, ShouldOpenDownload(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(*this, IsOffTheRecord()).WillRepeatedly(Return(false)); } }; @@ -2010,6 +2012,48 @@ TEST_F(DownloadItemTest, StealInterruptedNonContinuableDangerousDownload) { EXPECT_TRUE(returned_path.empty()); } +// Tests that for an incognito download, the target file is annotated with an +// empty source URL. +TEST_F(DownloadItemTest, AnnotationWithEmptyURLInIncognito) { + // Non-incognito case + DownloadItemImpl* item = CreateDownloadItem(); + MockDownloadFile* download_file = + DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS); + // Target file should be annotated with the source URL. + EXPECT_CALL(*download_file, + RenameAndAnnotate(_, _, create_info()->url(), _, _)) + .WillOnce(ScheduleRenameAndAnnotateCallback( + DOWNLOAD_INTERRUPT_REASON_NONE, base::FilePath(kDummyTargetPath), + base::ThreadTaskRunnerHandle::Get())); + EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _)) + .WillOnce(Return(true)); + EXPECT_CALL(*download_file, FullPath()) + .WillOnce(ReturnRefOfCopy(base::FilePath())); + EXPECT_CALL(*download_file, Detach()); + item->DestinationObserverAsWeakPtr()->DestinationCompleted( + 0, std::unique_ptr<crypto::SecureHash>()); + task_environment_.RunUntilIdle(); + + // Incognito case + item = CreateDownloadItem(); + download_file = + DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS); + // Target file should be annotated with an empty URL. + EXPECT_CALL(*download_file, RenameAndAnnotate(_, _, GURL(), _, _)) + .WillOnce(ScheduleRenameAndAnnotateCallback( + DOWNLOAD_INTERRUPT_REASON_NONE, base::FilePath(kDummyTargetPath), + base::ThreadTaskRunnerHandle::Get())); + EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _)) + .WillOnce(Return(true)); + EXPECT_CALL(*mock_delegate(), IsOffTheRecord()).WillRepeatedly(Return(true)); + EXPECT_CALL(*download_file, FullPath()) + .WillOnce(ReturnRefOfCopy(base::FilePath())); + EXPECT_CALL(*download_file, Detach()); + item->DestinationObserverAsWeakPtr()->DestinationCompleted( + 0, std::unique_ptr<crypto::SecureHash>()); + task_environment_.RunUntilIdle(); +} + namespace { // The DownloadItemDestinationUpdateRaceTest fixture (defined below) is used to @@ -2191,8 +2235,7 @@ class DownloadItemDestinationUpdateRaceTest public ::testing::WithParamInterface<EventList> { public: DownloadItemDestinationUpdateRaceTest() - : DownloadItemTest(), - item_(CreateDownloadItem()), + : item_(CreateDownloadItem()), file_(new ::testing::StrictMock<MockDownloadFile>()), request_handle_(new ::testing::StrictMock<MockRequestHandle>()) { DCHECK_EQ(GetParam().size(), static_cast<unsigned>(kEventCount)); diff --git a/chromium/components/download/internal/common/download_job_factory.cc b/chromium/components/download/internal/common/download_job_factory.cc index b2fda04191a..c9cffbebc1b 100644 --- a/chromium/components/download/internal/common/download_job_factory.cc +++ b/chromium/components/download/internal/common/download_job_factory.cc @@ -12,7 +12,7 @@ #include "components/download/internal/common/save_package_download_job.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/download_stats.h" -#include "services/network/public/cpp/shared_url_loader_factory.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" namespace download { @@ -44,10 +44,12 @@ bool IsParallelizableDownload(const DownloadCreateInfo& create_info, net::HttpResponseInfo::CONNECTION_INFO_HTTP1_1; bool http_get_method = create_info.method == "GET" && create_info.url().SchemeIsHTTPOrHTTPS(); - + bool partial_response_success = + download_item->GetReceivedSlices().empty() || create_info.offset != 0; bool is_parallelizable = has_strong_validator && create_info.accept_range && has_content_length && satisfy_min_file_size && - satisfy_connection_type && http_get_method; + satisfy_connection_type && http_get_method && + partial_response_success; if (!IsParallelDownloadEnabled()) return is_parallelizable; @@ -93,7 +95,8 @@ std::unique_ptr<DownloadJob> DownloadJobFactory::CreateJob( std::unique_ptr<DownloadRequestHandleInterface> req_handle, const DownloadCreateInfo& create_info, bool is_save_package_download, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, net::URLRequestContextGetter* url_request_context_getter) { if (is_save_package_download) { return std::make_unique<SavePackageDownloadJob>(download_item, @@ -105,7 +108,7 @@ std::unique_ptr<DownloadJob> DownloadJobFactory::CreateJob( if (IsParallelDownloadEnabled() && is_parallelizable) { return std::make_unique<ParallelDownloadJob>( download_item, std::move(req_handle), create_info, - std::move(shared_url_loader_factory), url_request_context_getter); + std::move(url_loader_factory_getter), url_request_context_getter); } // An ordinary download job. diff --git a/chromium/components/download/internal/common/download_stats.cc b/chromium/components/download/internal/common/download_stats.cc index 829b99ed9d6..fb06e932988 100644 --- a/chromium/components/download/internal/common/download_stats.cc +++ b/chromium/components/download/internal/common/download_stats.cc @@ -377,8 +377,7 @@ void RecordDownloadInterrupted(DownloadInterruptReason reason, } std::vector<base::HistogramBase::Sample> samples = - base::CustomHistogram::ArrayToCustomRanges( - kAllInterruptReasonCodes, arraysize(kAllInterruptReasonCodes)); + base::CustomHistogram::ArrayToCustomEnumRanges(kAllInterruptReasonCodes); UMA_HISTOGRAM_CUSTOM_ENUMERATION("Download.InterruptedReason", reason, samples); diff --git a/chromium/components/download/internal/common/download_task_runner.cc b/chromium/components/download/internal/common/download_task_runner.cc index 0cc99355e32..55db38f64b6 100644 --- a/chromium/components/download/internal/common/download_task_runner.cc +++ b/chromium/components/download/internal/common/download_task_runner.cc @@ -6,6 +6,7 @@ #include "base/lazy_instance.h" #include "base/no_destructor.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/lock.h" #include "base/task_scheduler/lazy_task_runner.h" #include "build/build_config.h" diff --git a/chromium/components/download/internal/common/download_utils.cc b/chromium/components/download/internal/common/download_utils.cc index 224894906f4..cc9f500041b 100644 --- a/chromium/components/download/internal/common/download_utils.cc +++ b/chromium/components/download/internal/common/download_utils.cc @@ -5,6 +5,7 @@ #include "components/download/public/common/download_utils.h" #include "base/format_macros.h" +#include "base/rand_util.h" #include "base/strings/stringprintf.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_interrupt_reasons_utils.h" @@ -347,4 +348,24 @@ std::unique_ptr<net::HttpRequestHeaders> GetAdditionalRequestHeaders( return headers; } +DownloadEntry CreateDownloadEntryFromItem( + const DownloadItem& item, + const std::string& request_origin, + DownloadSource download_source, + bool fetch_error_body, + const DownloadUrlParameters::RequestHeadersType& request_headers) { + return DownloadEntry(item.GetGuid(), request_origin, download_source, + fetch_error_body, request_headers, + GetUniqueDownloadId()); +} + +uint64_t GetUniqueDownloadId() { + // Get a new UKM download_id that is not 0. + uint64_t download_id = 0; + do { + download_id = base::RandUint64(); + } while (download_id == 0); + return download_id; +} + } // namespace download diff --git a/chromium/components/download/internal/common/download_worker.cc b/chromium/components/download/internal/common/download_worker.cc index c05b1f8a0c2..b6f3aa58879 100644 --- a/chromium/components/download/internal/common/download_worker.cc +++ b/chromium/components/download/internal/common/download_worker.cc @@ -4,13 +4,13 @@ #include "components/download/internal/common/download_worker.h" -#include "base/message_loop/message_loop.h" +#include "components/download/internal/common/resource_downloader.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_interrupt_reasons.h" #include "components/download/public/common/download_task_runner.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" #include "components/download/public/common/download_utils.h" #include "components/download/public/common/input_stream.h" -#include "components/download/public/common/resource_downloader.h" #include "components/download/public/common/url_download_handler_factory.h" #include "services/network/public/cpp/features.h" #include "services/network/public/cpp/shared_url_loader_factory.h" @@ -43,10 +43,11 @@ class CompletedInputStream : public InputStream { void CreateUrlDownloadHandler( std::unique_ptr<DownloadUrlParameters> params, base::WeakPtr<UrlDownloadHandler::Delegate> delegate, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) { auto downloader = UrlDownloadHandlerFactory::Create( - std::move(params), delegate, std::move(shared_url_loader_factory), + std::move(params), delegate, std::move(url_loader_factory_getter), task_runner); task_runner->PostTask( FROM_HERE, @@ -74,11 +75,12 @@ DownloadWorker::~DownloadWorker() = default; void DownloadWorker::SendRequest( std::unique_ptr<DownloadUrlParameters> params, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory) { + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter) { GetIOTaskRunner()->PostTask( FROM_HERE, base::BindOnce(&CreateUrlDownloadHandler, std::move(params), weak_factory_.GetWeakPtr(), - std::move(shared_url_loader_factory), + std::move(url_loader_factory_getter), base::ThreadTaskRunnerHandle::Get())); } @@ -104,7 +106,8 @@ void DownloadWorker::Cancel(bool user_cancel) { void DownloadWorker::OnUrlDownloadStarted( std::unique_ptr<DownloadCreateInfo> create_info, std::unique_ptr<InputStream> input_stream, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const DownloadUrlParameters::OnStartedCallback& callback) { // |callback| is not used in subsequent requests. DCHECK(callback.is_null()); @@ -134,7 +137,8 @@ void DownloadWorker::OnUrlDownloadStarted( Pause(); } - delegate_->OnInputStreamReady(this, std::move(input_stream)); + delegate_->OnInputStreamReady(this, std::move(input_stream), + std::move(create_info)); } void DownloadWorker::OnUrlDownloadStopped(UrlDownloadHandler* downloader) { diff --git a/chromium/components/download/internal/common/download_worker.h b/chromium/components/download/internal/common/download_worker.h index 8996f082d12..8c712607093 100644 --- a/chromium/components/download/internal/common/download_worker.h +++ b/chromium/components/download/internal/common/download_worker.h @@ -14,11 +14,8 @@ #include "components/download/public/common/download_url_parameters.h" #include "components/download/public/common/url_download_handler.h" -namespace network { -class SharedURLLoaderFactory; -} - namespace download { +class DownloadURLLoaderFactoryGetter; // Helper class used to send subsequent range requests to fetch slices of the // file after handling response of the original non-range request. @@ -34,7 +31,8 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker // destination file. virtual void OnInputStreamReady( DownloadWorker* worker, - std::unique_ptr<InputStream> input_stream) = 0; + std::unique_ptr<InputStream> input_stream, + std::unique_ptr<DownloadCreateInfo> download_create_info) = 0; }; DownloadWorker(DownloadWorker::Delegate* delegate, @@ -46,9 +44,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker int64_t length() const { return length_; } // Send network request to ask for a download. - void SendRequest( - std::unique_ptr<DownloadUrlParameters> params, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory); + void SendRequest(std::unique_ptr<DownloadUrlParameters> params, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter); // Download operations. void Pause(); @@ -60,7 +58,8 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker void OnUrlDownloadStarted( std::unique_ptr<DownloadCreateInfo> create_info, std::unique_ptr<InputStream> input_stream, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const DownloadUrlParameters::OnStartedCallback& callback) override; void OnUrlDownloadStopped(UrlDownloadHandler* downloader) override; void OnUrlDownloadHandlerCreated( diff --git a/chromium/components/download/internal/common/in_progress_download_manager.cc b/chromium/components/download/internal/common/in_progress_download_manager.cc new file mode 100644 index 00000000000..9ed647a5b05 --- /dev/null +++ b/chromium/components/download/internal/common/in_progress_download_manager.cc @@ -0,0 +1,402 @@ +// Copyright 2018 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/public/common/in_progress_download_manager.h" + +#include "base/optional.h" +#include "base/task_scheduler/post_task.h" +#include "base/threading/thread_task_runner_handle.h" +#include "components/download/downloader/in_progress/in_progress_cache_impl.h" +#include "components/download/internal/common/resource_downloader.h" +#include "components/download/public/common/download_file.h" +#include "components/download/public/common/download_item_impl.h" +#include "components/download/public/common/download_stats.h" +#include "components/download/public/common/download_task_runner.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" +#include "components/download/public/common/download_url_parameters.h" +#include "components/download/public/common/download_utils.h" +#include "components/download/public/common/input_stream.h" +#include "services/network/public/cpp/resource_request.h" +#include "services/network/public/cpp/resource_response.h" + +namespace download { + +namespace { + +void OnUrlDownloadHandlerCreated( + UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader, + base::WeakPtr<InProgressDownloadManager> download_manager, + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) { + main_task_runner->PostTask( + FROM_HERE, + base::BindOnce(&UrlDownloadHandler::Delegate::OnUrlDownloadHandlerCreated, + download_manager, std::move(downloader))); +} + +void BeginResourceDownload( + std::unique_ptr<DownloadUrlParameters> params, + std::unique_ptr<network::ResourceRequest> request, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + uint32_t download_id, + base::WeakPtr<InProgressDownloadManager> download_manager, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) { + DCHECK(GetIOTaskRunner()->BelongsToCurrentThread()); + UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader( + ResourceDownloader::BeginDownload( + download_manager, std::move(params), std::move(request), + std::move(url_loader_factory_getter), site_url, tab_url, + tab_referrer_url, download_id, false, main_task_runner) + .release(), + base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); + + OnUrlDownloadHandlerCreated(std::move(downloader), download_manager, + main_task_runner); +} + +void CreateDownloadHandlerForNavigation( + base::WeakPtr<InProgressDownloadManager> download_manager, + std::unique_ptr<network::ResourceRequest> resource_request, + int render_process_id, + int render_frame_id, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + std::vector<GURL> url_chain, + scoped_refptr<network::ResourceResponse> response, + net::CertStatus cert_status, + network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) { + DCHECK(GetIOTaskRunner()->BelongsToCurrentThread()); + UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader( + ResourceDownloader::InterceptNavigationResponse( + download_manager, std::move(resource_request), render_process_id, + render_frame_id, site_url, tab_url, tab_referrer_url, + std::move(url_chain), std::move(response), std::move(cert_status), + std::move(url_loader_client_endpoints), + std::move(url_loader_factory_getter), main_task_runner) + .release(), + base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); + + OnUrlDownloadHandlerCreated(std::move(downloader), download_manager, + main_task_runner); +} + +// Responsible for persisting the in-progress metadata associated with a +// download. +class InProgressDownloadObserver : public DownloadItem::Observer { + public: + explicit InProgressDownloadObserver(InProgressCache* in_progress_cache); + ~InProgressDownloadObserver() override; + + private: + // DownloadItem::Observer + void OnDownloadUpdated(DownloadItem* download) override; + void OnDownloadRemoved(DownloadItem* download) override; + + // The persistent cache to store in-progress metadata. + InProgressCache* in_progress_cache_; + + DISALLOW_COPY_AND_ASSIGN(InProgressDownloadObserver); +}; + +InProgressDownloadObserver::InProgressDownloadObserver( + InProgressCache* in_progress_cache) + : in_progress_cache_(in_progress_cache) {} + +InProgressDownloadObserver::~InProgressDownloadObserver() = default; + +void InProgressDownloadObserver::OnDownloadUpdated(DownloadItem* download) { + // TODO(crbug.com/778425): Properly handle fail/resume/retry for downloads + // that are in the INTERRUPTED state for a long time. + if (!in_progress_cache_) + return; + + switch (download->GetState()) { + case DownloadItem::DownloadState::COMPLETE: + // Intentional fallthrough. + case DownloadItem::DownloadState::CANCELLED: + if (in_progress_cache_) + in_progress_cache_->RemoveEntry(download->GetGuid()); + break; + + case DownloadItem::DownloadState::INTERRUPTED: + // Intentional fallthrough. + case DownloadItem::DownloadState::IN_PROGRESS: { + // Make sure the entry exists in the cache. + base::Optional<DownloadEntry> entry_opt = + in_progress_cache_->RetrieveEntry(download->GetGuid()); + DownloadEntry entry; + if (!entry_opt.has_value()) { + entry = CreateDownloadEntryFromItem( + *download, std::string(), /* request_origin */ + DownloadSource::UNKNOWN, false, /* fetch_error_body */ + DownloadUrlParameters::RequestHeadersType()); + in_progress_cache_->AddOrReplaceEntry(entry); + break; + } + entry = entry_opt.value(); + break; + } + + default: + break; + } +} + +void InProgressDownloadObserver::OnDownloadRemoved(DownloadItem* download) { + if (!in_progress_cache_) + return; + + in_progress_cache_->RemoveEntry(download->GetGuid()); +} + +} // namespace + +InProgressDownloadManager::InProgressDownloadManager( + Delegate* delegate, + const IsOriginSecureCallback& is_origin_secure_cb) + : delegate_(delegate), + file_factory_(new DownloadFileFactory()), + is_origin_secure_cb_(is_origin_secure_cb), + weak_factory_(this) {} + +InProgressDownloadManager::~InProgressDownloadManager() = default; + +void InProgressDownloadManager::OnUrlDownloadStarted( + std::unique_ptr<DownloadCreateInfo> download_create_info, + std::unique_ptr<InputStream> input_stream, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + const DownloadUrlParameters::OnStartedCallback& callback) { + StartDownload(std::move(download_create_info), std::move(input_stream), + std::move(url_loader_factory_getter), callback); +} + +void InProgressDownloadManager::OnUrlDownloadStopped( + UrlDownloadHandler* downloader) { + for (auto ptr = url_download_handlers_.begin(); + ptr != url_download_handlers_.end(); ++ptr) { + if (ptr->get() == downloader) { + url_download_handlers_.erase(ptr); + return; + } + } +} + +void InProgressDownloadManager::OnUrlDownloadHandlerCreated( + UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader) { + if (downloader) + url_download_handlers_.push_back(std::move(downloader)); +} + +void InProgressDownloadManager::BeginDownload( + std::unique_ptr<DownloadUrlParameters> params, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + uint32_t download_id, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url) { + std::unique_ptr<network::ResourceRequest> request = + CreateResourceRequest(params.get()); + GetIOTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce(&BeginResourceDownload, std::move(params), + std::move(request), std::move(url_loader_factory_getter), + download_id, weak_factory_.GetWeakPtr(), site_url, tab_url, + tab_referrer_url, base::ThreadTaskRunnerHandle::Get())); +} + +void InProgressDownloadManager::InterceptDownloadFromNavigation( + std::unique_ptr<network::ResourceRequest> resource_request, + int render_process_id, + int render_frame_id, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + std::vector<GURL> url_chain, + scoped_refptr<network::ResourceResponse> response, + net::CertStatus cert_status, + network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter) { + GetIOTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce(&CreateDownloadHandlerForNavigation, + weak_factory_.GetWeakPtr(), std::move(resource_request), + render_process_id, render_frame_id, site_url, tab_url, + tab_referrer_url, std::move(url_chain), + std::move(response), std::move(cert_status), + std::move(url_loader_client_endpoints), + std::move(url_loader_factory_getter), + base::ThreadTaskRunnerHandle::Get())); +} + +void InProgressDownloadManager::Initialize( + const base::FilePath& metadata_cache_dir, + base::OnceClosure callback) { + download_metadata_cache_ = std::make_unique<InProgressCacheImpl>( + metadata_cache_dir.empty() ? base::FilePath() : + metadata_cache_dir.Append(kDownloadMetadataStoreFilename), + base::CreateSequencedTaskRunnerWithTraits( + {base::MayBlock(), base::TaskPriority::BACKGROUND, + base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})); + + download_metadata_cache_->Initialize(std::move(callback)); +} + +void InProgressDownloadManager::ShutDown() { + url_download_handlers_.clear(); +} + +void InProgressDownloadManager::ResumeInterruptedDownload( + std::unique_ptr<DownloadUrlParameters> params, + uint32_t id, + const GURL& site_url) {} + +base::Optional<DownloadEntry> InProgressDownloadManager::GetInProgressEntry( + DownloadItemImpl* download) { + if (!download || !download_metadata_cache_) + return base::Optional<DownloadEntry>(); + + return download_metadata_cache_->RetrieveEntry(download->GetGuid()); +} + +void InProgressDownloadManager::ReportBytesWasted(DownloadItemImpl* download) { + if (!download_metadata_cache_) + return; + base::Optional<DownloadEntry> entry_opt = + download_metadata_cache_->RetrieveEntry(download->GetGuid()); + if (entry_opt.has_value()) { + DownloadEntry entry = entry_opt.value(); + entry.bytes_wasted = download->GetBytesWasted(); + download_metadata_cache_->AddOrReplaceEntry(entry); + } +} + +void InProgressDownloadManager::StartDownload( + std::unique_ptr<DownloadCreateInfo> info, + std::unique_ptr<InputStream> stream, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + const DownloadUrlParameters::OnStartedCallback& on_started) { + DCHECK(info); + + uint32_t download_id = info->download_id; + bool new_download = (download_id == DownloadItem::kInvalidId); + if (new_download && info->result == DOWNLOAD_INTERRUPT_REASON_NONE) { + if (delegate_ && delegate_->InterceptDownload(*info)) { + GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release()); + return; + } + } + + // |stream| is only non-null if the download request was successful. + DCHECK( + (info->result == DOWNLOAD_INTERRUPT_REASON_NONE && !stream->IsEmpty()) || + (info->result != DOWNLOAD_INTERRUPT_REASON_NONE && stream->IsEmpty())); + DVLOG(20) << __func__ + << "() result=" << DownloadInterruptReasonToString(info->result); + + GURL url = info->url(); + std::vector<GURL> url_chain = info->url_chain; + std::string mime_type = info->mime_type; + + if (new_download) { + RecordDownloadConnectionSecurity(info->url(), info->url_chain); + RecordDownloadContentTypeSecurity(info->url(), info->url_chain, + info->mime_type, is_origin_secure_cb_); + } + + base::RepeatingCallback<void(uint32_t)> got_id(base::BindRepeating( + &InProgressDownloadManager::StartDownloadWithId, + weak_factory_.GetWeakPtr(), base::Passed(&info), base::Passed(&stream), + std::move(url_loader_factory_getter), on_started, new_download)); + if (new_download) { + // TODO(qinmin): use GUID as the key for downloads history table so we don't + // rely on the delegate to provide the next download ID. + if (delegate_) + delegate_->GetNextId(std::move(got_id)); + } else { + std::move(got_id).Run(download_id); + } +} + +void InProgressDownloadManager::StartDownloadWithId( + std::unique_ptr<DownloadCreateInfo> info, + std::unique_ptr<InputStream> stream, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + const DownloadUrlParameters::OnStartedCallback& on_started, + bool new_download, + uint32_t id) { + DCHECK_NE(DownloadItem::kInvalidId, id); + DownloadItemImpl* download = + delegate_ ? delegate_->GetDownloadItem(id, new_download, *info) : nullptr; + + if (!download) { + // If the download is no longer known to the DownloadManager, then it was + // removed after it was resumed. Ignore. If the download is cancelled + // while resuming, then also ignore the request. + if (info->request_handle) + info->request_handle->CancelRequest(true); + if (!on_started.is_null()) + on_started.Run(nullptr, DOWNLOAD_INTERRUPT_REASON_USER_CANCELED); + // The ByteStreamReader lives and dies on the download sequence. + if (info->result == DOWNLOAD_INTERRUPT_REASON_NONE) + GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release()); + return; + } + + base::FilePath default_download_directory; + if (delegate_) + default_download_directory = delegate_->GetDefaultDownloadDirectory(); + + if (download_metadata_cache_) { + base::Optional<DownloadEntry> entry_opt = + download_metadata_cache_->RetrieveEntry(download->GetGuid()); + if (!entry_opt.has_value()) { + download_metadata_cache_->AddOrReplaceEntry(CreateDownloadEntryFromItem( + *download, info->request_origin, info->download_source, + info->fetch_error_body, info->request_headers)); + } + } + + if (!in_progress_download_observer_) { + in_progress_download_observer_ = + std::make_unique<InProgressDownloadObserver>( + download_metadata_cache_.get()); + } + // May already observe this item, remove observer first. + download->RemoveObserver(in_progress_download_observer_.get()); + download->AddObserver(in_progress_download_observer_.get()); + + std::unique_ptr<DownloadFile> download_file; + if (info->result == DOWNLOAD_INTERRUPT_REASON_NONE) { + DCHECK(stream); + download_file.reset(file_factory_->CreateFile( + std::move(info->save_info), default_download_directory, + std::move(stream), id, download->DestinationObserverAsWeakPtr())); + } + // It is important to leave info->save_info intact in the case of an interrupt + // so that the DownloadItem can salvage what it can out of a failed + // resumption attempt. + + download->Start( + std::move(download_file), std::move(info->request_handle), *info, + std::move(url_loader_factory_getter), + delegate_ ? delegate_->GetURLRequestContextGetter(*info) : nullptr); + + // For interrupted downloads, Start() will transition the state to + // IN_PROGRESS and consumers will be notified via OnDownloadUpdated(). + // For new downloads, we notify here, rather than earlier, so that + // the download_file is bound to download and all the usual + // setters (e.g. Cancel) work. + if (new_download && delegate_) + delegate_->OnNewDownloadStarted(download); + + if (!on_started.is_null()) + on_started.Run(download, DOWNLOAD_INTERRUPT_REASON_NONE); +} + +} // namespace download diff --git a/chromium/components/download/internal/common/parallel_download_job.cc b/chromium/components/download/internal/common/parallel_download_job.cc index e139a42c314..e72f9af89c0 100644 --- a/chromium/components/download/internal/common/parallel_download_job.cc +++ b/chromium/components/download/internal/common/parallel_download_job.cc @@ -12,6 +12,7 @@ #include "components/download/internal/common/parallel_download_utils.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_stats.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" #include "net/traffic_annotation/network_traffic_annotation.h" namespace download { @@ -23,7 +24,8 @@ ParallelDownloadJob::ParallelDownloadJob( DownloadItem* download_item, std::unique_ptr<DownloadRequestHandleInterface> request_handle, const DownloadCreateInfo& create_info, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, net::URLRequestContextGetter* url_request_context_getter) : DownloadJobImpl(download_item, std::move(request_handle), true), initial_request_offset_(create_info.offset), @@ -31,7 +33,7 @@ ParallelDownloadJob::ParallelDownloadJob( content_length_(create_info.total_bytes), requests_sent_(false), is_canceled_(false), - shared_url_loader_factory_(std::move(shared_url_loader_factory)), + url_loader_factory_getter_(std::move(url_loader_factory_getter)), url_request_context_getter_(url_request_context_getter) {} ParallelDownloadJob::~ParallelDownloadJob() = default; @@ -121,9 +123,14 @@ void ParallelDownloadJob::BuildParallelRequestAfterDelay() { void ParallelDownloadJob::OnInputStreamReady( DownloadWorker* worker, - std::unique_ptr<InputStream> input_stream) { - bool success = DownloadJob::AddInputStream( - std::move(input_stream), worker->offset(), worker->length()); + std::unique_ptr<InputStream> input_stream, + std::unique_ptr<DownloadCreateInfo> download_create_info) { + // If server returns a wrong range, abort the parallel request. + bool success = download_create_info->offset == worker->offset(); + if (success) { + success = DownloadJob::AddInputStream(std::move(input_stream), + worker->offset(), worker->length()); + } RecordParallelDownloadAddStreamSuccess(success); // Destroy the request if the sink is gone. @@ -286,7 +293,7 @@ void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) { download_params->set_referrer_policy(net::URLRequest::NEVER_CLEAR_REFERRER); // Send the request. - worker->SendRequest(std::move(download_params), shared_url_loader_factory_); + worker->SendRequest(std::move(download_params), url_loader_factory_getter_); DCHECK(workers_.find(offset) == workers_.end()); workers_[offset] = std::move(worker); } diff --git a/chromium/components/download/internal/common/parallel_download_job.h b/chromium/components/download/internal/common/parallel_download_job.h index c37fb07eb96..a6803e22ee4 100644 --- a/chromium/components/download/internal/common/parallel_download_job.h +++ b/chromium/components/download/internal/common/parallel_download_job.h @@ -35,7 +35,8 @@ class COMPONENTS_DOWNLOAD_EXPORT ParallelDownloadJob DownloadItem* download_item, std::unique_ptr<DownloadRequestHandleInterface> request_handle, const DownloadCreateInfo& create_info, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, net::URLRequestContextGetter* url_request_context_getter); ~ParallelDownloadJob() override; @@ -67,8 +68,10 @@ class COMPONENTS_DOWNLOAD_EXPORT ParallelDownloadJob friend class ParallelDownloadJobTest; // DownloadWorker::Delegate implementation. - void OnInputStreamReady(DownloadWorker* worker, - std::unique_ptr<InputStream> input_stream) override; + void OnInputStreamReady( + DownloadWorker* worker, + std::unique_ptr<InputStream> input_stream, + std::unique_ptr<DownloadCreateInfo> download_create_info) override; // Build parallel requests after a delay, to effectively measure the single // stream bandwidth. @@ -110,8 +113,9 @@ class COMPONENTS_DOWNLOAD_EXPORT ParallelDownloadJob // If the download progress is canceled. bool is_canceled_; - // SharedURLLoaderFactory to issue network requests with network service - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_; + // URLLoaderFactory getter to issue network requests with network service + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter_; // URLRequestContextGetter for issueing network requests when network service // is disabled. diff --git a/chromium/components/download/internal/common/parallel_download_job_unittest.cc b/chromium/components/download/internal/common/parallel_download_job_unittest.cc index eb09f931707..61761c49fb4 100644 --- a/chromium/components/download/internal/common/parallel_download_job_unittest.cc +++ b/chromium/components/download/internal/common/parallel_download_job_unittest.cc @@ -94,8 +94,10 @@ class ParallelDownloadJobForTest : public ParallelDownloadJob { return min_remaining_time_; } - void OnInputStreamReady(DownloadWorker* worker, - std::unique_ptr<InputStream> input_stream) override { + void OnInputStreamReady( + DownloadWorker* worker, + std::unique_ptr<InputStream> input_stream, + std::unique_ptr<DownloadCreateInfo> download_create_info) override { CountOnInputStreamReady(); } @@ -429,7 +431,7 @@ TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeByteStreamReady) { for (auto& worker : job_->workers()) { std::unique_ptr<MockDownloadRequestHandle> mock_handle = std::make_unique<MockDownloadRequestHandle>(); - EXPECT_CALL(*mock_handle.get(), CancelRequest(_)); + EXPECT_CALL(*mock_handle, CancelRequest(_)); MakeWorkerReady(worker.second.get(), std::move(mock_handle)); } @@ -451,10 +453,10 @@ TEST_F(ParallelDownloadJobTest, EarlyPauseBeforeByteStreamReady) { EXPECT_TRUE(job_->is_paused()); for (auto& worker : job_->workers()) { - EXPECT_CALL(*job_.get(), CountOnInputStreamReady()); + EXPECT_CALL(*job_, CountOnInputStreamReady()); std::unique_ptr<MockDownloadRequestHandle> mock_handle = std::make_unique<MockDownloadRequestHandle>(); - EXPECT_CALL(*mock_handle.get(), PauseRequest()); + EXPECT_CALL(*mock_handle, PauseRequest()); MakeWorkerReady(worker.second.get(), std::move(mock_handle)); } @@ -517,7 +519,7 @@ TEST_F(ParallelDownloadJobTest, InterruptOnStartup) { job_->MakeFileInitialized(callback.Get(), DOWNLOAD_INTERRUPT_REASON_NONE); // Simulate and inject an error from IO thread after file initialized. - EXPECT_CALL(*download_item_.get(), GetState()) + EXPECT_CALL(*download_item_, GetState()) .WillRepeatedly(Return(DownloadItem::DownloadState::INTERRUPTED)); // Because of the error, no parallel requests are built. diff --git a/chromium/components/download/internal/common/resource_downloader.cc b/chromium/components/download/internal/common/resource_downloader.cc index ab961f310e3..988cf5cef88 100644 --- a/chromium/components/download/internal/common/resource_downloader.cc +++ b/chromium/components/download/internal/common/resource_downloader.cc @@ -2,11 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/download/public/common/resource_downloader.h" +#include "components/download/internal/common/resource_downloader.h" #include <memory> -#include "base/strings/utf_string_conversions.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" #include "components/download/public/common/stream_handle_input_stream.h" #include "services/network/public/cpp/shared_url_loader_factory.h" @@ -60,7 +60,8 @@ std::unique_ptr<ResourceDownloader> ResourceDownloader::BeginDownload( base::WeakPtr<UrlDownloadHandler::Delegate> delegate, std::unique_ptr<DownloadUrlParameters> params, std::unique_ptr<network::ResourceRequest> request, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const GURL& site_url, const GURL& tab_url, const GURL& tab_referrer_url, @@ -71,7 +72,7 @@ std::unique_ptr<ResourceDownloader> ResourceDownloader::BeginDownload( delegate, std::move(request), params->render_process_host_id(), params->render_frame_host_routing_id(), site_url, tab_url, tab_referrer_url, download_id, task_runner, - std::move(shared_url_loader_factory)); + std::move(url_loader_factory_getter)); downloader->Start(std::move(params), is_parallel_request); return downloader; @@ -88,18 +89,18 @@ ResourceDownloader::InterceptNavigationResponse( const GURL& tab_url, const GURL& tab_referrer_url, std::vector<GURL> url_chain, - const base::Optional<std::string>& suggested_filename, const scoped_refptr<network::ResourceResponse>& response, net::CertStatus cert_status, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) { auto downloader = std::make_unique<ResourceDownloader>( delegate, std::move(resource_request), render_process_id, render_frame_id, site_url, tab_url, tab_referrer_url, download::DownloadItem::kInvalidId, - task_runner, std::move(shared_url_loader_factory)); + task_runner, std::move(url_loader_factory_getter)); downloader->InterceptResponse(std::move(response), std::move(url_chain), - suggested_filename, cert_status, + cert_status, std::move(url_loader_client_endpoints)); return downloader; } @@ -114,7 +115,8 @@ ResourceDownloader::ResourceDownloader( const GURL& tab_referrer_url, uint32_t download_id, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory) + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter) : delegate_(delegate), resource_request_(std::move(resource_request)), download_id_(download_id), @@ -124,7 +126,7 @@ ResourceDownloader::ResourceDownloader( tab_url_(tab_url), tab_referrer_url_(tab_referrer_url), delegate_task_runner_(task_runner), - shared_url_loader_factory_(std::move(shared_url_loader_factory)), + url_loader_factory_getter_(std::move(url_loader_factory_getter)), weak_ptr_factory_(this) {} ResourceDownloader::~ResourceDownloader() = default; @@ -154,7 +156,7 @@ void ResourceDownloader::Start( // Set up the URLLoader network::mojom::URLLoaderRequest url_loader_request = mojo::MakeRequest(&url_loader_); - shared_url_loader_factory_->CreateLoaderAndStart( + url_loader_factory_getter_->GetURLLoaderFactory()->CreateLoaderAndStart( std::move(url_loader_request), 0, // routing_id 0, // request_id @@ -169,18 +171,14 @@ void ResourceDownloader::Start( void ResourceDownloader::InterceptResponse( const scoped_refptr<network::ResourceResponse>& response, std::vector<GURL> url_chain, - const base::Optional<std::string>& suggested_filename, net::CertStatus cert_status, network::mojom::URLLoaderClientEndpointsPtr endpoints) { // Set the URLLoader. url_loader_.Bind(std::move(endpoints->url_loader)); // Create the new URLLoaderClient that will intercept the navigation. - auto save_info = std::make_unique<DownloadSaveInfo>(); - if (suggested_filename.has_value()) - save_info->suggested_name = base::UTF8ToUTF16(suggested_filename.value()); url_loader_client_ = std::make_unique<DownloadResponseHandler>( - resource_request_.get(), this, std::move(save_info), + resource_request_.get(), this, std::make_unique<DownloadSaveInfo>(), false, /* is_parallel_request */ false, /* is_transient */ false, /* fetch_error_body */ @@ -216,11 +214,11 @@ void ResourceDownloader::OnResponseStarted( &UrlDownloadHandler::Delegate::OnUrlDownloadStarted, delegate_, std::move(download_create_info), std::make_unique<StreamHandleInputStream>(std::move(stream_handle)), - std::move(shared_url_loader_factory_), callback_)); + std::move(url_loader_factory_getter_), callback_)); } void ResourceDownloader::OnReceiveRedirect() { - url_loader_->FollowRedirect(); + url_loader_->FollowRedirect(base::nullopt); } } // namespace download diff --git a/chromium/components/download/public/common/resource_downloader.h b/chromium/components/download/internal/common/resource_downloader.h index 9106f98d3dd..ce6eef63834 100644 --- a/chromium/components/download/public/common/resource_downloader.h +++ b/chromium/components/download/internal/common/resource_downloader.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef COMPONENTS_DOWNLOAD_PUBLIC_COMMON_RESOURCE_DOWNLOADER_H_ -#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_RESOURCE_DOWNLOADER_H_ +#ifndef COMPONENTS_DOWNLOAD_INTERNAL_COMMON_RESOURCE_DOWNLOADER_H_ +#define COMPONENTS_DOWNLOAD_INTERNAL_COMMON_RESOURCE_DOWNLOADER_H_ #include "components/download/public/common/download_export.h" #include "components/download/public/common/download_response_handler.h" @@ -13,11 +13,8 @@ #include "services/network/public/cpp/resource_request.h" #include "services/network/public/mojom/url_loader.mojom.h" -namespace network { -class SharedURLLoaderFactory; -} - namespace download { +class DownloadURLLoaderFactoryGetter; // Class for handing the download of a url. class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader @@ -29,7 +26,8 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, std::unique_ptr<download::DownloadUrlParameters> download_url_parameters, std::unique_ptr<network::ResourceRequest> request, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const GURL& site_url, const GURL& tab_url, const GURL& tab_referrer_url, @@ -49,11 +47,11 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader const GURL& tab_url, const GURL& tab_referrer_url, std::vector<GURL> url_chain, - const base::Optional<std::string>& suggested_filename, const scoped_refptr<network::ResourceResponse>& response, net::CertStatus cert_status, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); ResourceDownloader( @@ -66,7 +64,8 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader const GURL& tab_referrer_url, uint32_t download_id, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory); + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter); ~ResourceDownloader() override; // download::DownloadResponseHandler::Delegate @@ -85,7 +84,6 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader void InterceptResponse( const scoped_refptr<network::ResourceResponse>& response, std::vector<GURL> url_chain, - const base::Optional<std::string>& suggested_filename, net::CertStatus cert_status, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints); @@ -133,8 +131,9 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader // TaskRunner to post callbacks to the |delegate_| scoped_refptr<base::SingleThreadTaskRunner> delegate_task_runner_; - // URLLoaderFactory for issueing network requests. - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_; + // URLLoaderFactory getter for issueing network requests. + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter_; base::WeakPtrFactory<ResourceDownloader> weak_ptr_factory_; @@ -143,4 +142,4 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader } // namespace download -#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_RESOURCE_DOWNLOADER_H_ +#endif // COMPONENTS_DOWNLOAD_INTERNAL_COMMON_RESOURCE_DOWNLOADER_H_ diff --git a/chromium/components/download/internal/common/url_download_handler_factory.cc b/chromium/components/download/internal/common/url_download_handler_factory.cc index fec5bf41cd0..625f6da4ffd 100644 --- a/chromium/components/download/internal/common/url_download_handler_factory.cc +++ b/chromium/components/download/internal/common/url_download_handler_factory.cc @@ -6,9 +6,10 @@ #include "base/no_destructor.h" #include "base/synchronization/lock.h" +#include "components/download/internal/common/resource_downloader.h" #include "components/download/public/common/download_item.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" #include "components/download/public/common/download_utils.h" -#include "components/download/public/common/resource_downloader.h" #include "services/network/public/cpp/shared_url_loader_factory.h" namespace download { @@ -25,14 +26,15 @@ class DefaultUrlDownloadHandlerFactory : public UrlDownloadHandlerFactory { UrlDownloadHandler::UniqueUrlDownloadHandlerPtr CreateUrlDownloadHandler( std::unique_ptr<download::DownloadUrlParameters> params, base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) override { std::unique_ptr<network::ResourceRequest> request = CreateResourceRequest(params.get()); return UrlDownloadHandler::UniqueUrlDownloadHandlerPtr( download::ResourceDownloader::BeginDownload( delegate, std::move(params), std::move(request), - std::move(shared_url_loader_factory), GURL(), GURL(), GURL(), + std::move(url_loader_factory_getter), GURL(), GURL(), GURL(), download::DownloadItem::kInvalidId, true, task_runner) .release(), base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); @@ -57,13 +59,14 @@ UrlDownloadHandler::UniqueUrlDownloadHandlerPtr UrlDownloadHandlerFactory::Create( std::unique_ptr<download::DownloadUrlParameters> params, base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) { base::AutoLock auto_lock(GetURLDownloadHandlerFactoryLock()); if (!g_url_download_handler_factory) g_url_download_handler_factory = new DefaultUrlDownloadHandlerFactory(); return g_url_download_handler_factory->CreateUrlDownloadHandler( - std::move(params), delegate, std::move(shared_url_loader_factory), + std::move(params), delegate, std::move(url_loader_factory_getter), task_runner); } diff --git a/chromium/components/download/public/background_service/download_metadata.cc b/chromium/components/download/public/background_service/download_metadata.cc index 22eb64c2c37..9beca125590 100644 --- a/chromium/components/download/public/background_service/download_metadata.cc +++ b/chromium/components/download/public/background_service/download_metadata.cc @@ -19,12 +19,13 @@ bool CompletionInfo::operator==(const CompletionInfo& other) const { return path == other.path && bytes_downloaded == other.bytes_downloaded; } -DownloadMetaData::DownloadMetaData() = default; +DownloadMetaData::DownloadMetaData() : current_size(0u) {} DownloadMetaData::DownloadMetaData(const DownloadMetaData& other) = default; bool DownloadMetaData::operator==(const DownloadMetaData& other) const { - return guid == other.guid && completion_info == other.completion_info; + return guid == other.guid && current_size == other.current_size && + completion_info == other.completion_info; } DownloadMetaData::~DownloadMetaData() = default; diff --git a/chromium/components/download/public/background_service/download_metadata.h b/chromium/components/download/public/background_service/download_metadata.h index b786ba19962..68a507fc2db 100644 --- a/chromium/components/download/public/background_service/download_metadata.h +++ b/chromium/components/download/public/background_service/download_metadata.h @@ -35,6 +35,10 @@ struct DownloadMetaData { // The GUID of the download. std::string guid; + // Data that has been fetched. Can be used to get the current size of + // uncompleted download. + uint64_t current_size; + // Info about successfully completed download, or null for in-progress // download. Failed download will not be persisted and exposed as meta data. base::Optional<CompletionInfo> completion_info; diff --git a/chromium/components/download/public/common/BUILD.gn b/chromium/components/download/public/common/BUILD.gn index f11815d9c84..16f35f36623 100644 --- a/chromium/components/download/public/common/BUILD.gn +++ b/chromium/components/download/public/common/BUILD.gn @@ -48,11 +48,11 @@ component("public") { "download_url_parameters.cc", "download_url_parameters.h", "download_utils.h", + "in_progress_download_manager.h", "input_stream.cc", "input_stream.h", "parallel_download_configs.h", "rate_estimator.h", - "resource_downloader.h", "resume_mode.h", "stream_handle_input_stream.h", "url_download_handler_factory.h", @@ -104,10 +104,6 @@ mojom("interfaces") { "download_stream.mojom", ] - public_deps = [ - "//mojo/common:common_custom_types", - ] - export_class_attribute = "COMPONENTS_DOWNLOAD_EXPORT" export_define = "COMPONENTS_DOWNLOAD_IMPLEMENTATION=1" export_header = "components/download/public/common/download_export.h" diff --git a/chromium/components/download/public/common/download_danger_type.h b/chromium/components/download/public/common/download_danger_type.h index c97bd08eebc..e9e8952413b 100644 --- a/chromium/components/download/public/common/download_danger_type.h +++ b/chromium/components/download/public/common/download_danger_type.h @@ -15,34 +15,37 @@ enum DownloadDangerType { // A dangerous file to the system (e.g.: a pdf or extension from // places other than gallery). - DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE, + DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE = 1, // Safebrowsing download service shows this URL leads to malicious file // download. - DOWNLOAD_DANGER_TYPE_DANGEROUS_URL, + DOWNLOAD_DANGER_TYPE_DANGEROUS_URL = 2, // SafeBrowsing download service shows this file content as being malicious. - DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT, + DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT = 3, // The content of this download may be malicious (e.g., extension is exe but // SafeBrowsing has not finished checking the content). - DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT, + DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT = 4, // SafeBrowsing download service checked the contents of the download, but // didn't have enough data to determine whether it was malicious. - DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT, + DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT = 5, // The download was evaluated to be one of the other types of danger, // but the user told us to go ahead anyway. - DOWNLOAD_DANGER_TYPE_USER_VALIDATED, + DOWNLOAD_DANGER_TYPE_USER_VALIDATED = 6, // SafeBrowsing download service checked the contents of the download and // didn't have data on this specific file, but the file was served from a host // known to serve mostly malicious content. - DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST, + DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST = 7, // Applications and extensions that modify browser and/or computer settings - DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED, + DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED = 8, + + // Download URL whitelisted by enterprise policy. + DOWNLOAD_DANGER_TYPE_WHITELISTED_BY_POLICY = 9, // Memory space for histograms is determined by the max. // ALWAYS ADD NEW VALUES BEFORE THIS ONE. diff --git a/chromium/components/download/public/common/download_item.h b/chromium/components/download/public/common/download_item.h index 3520400f3a6..225d60ce645 100644 --- a/chromium/components/download/public/common/download_item.h +++ b/chromium/components/download/public/common/download_item.h @@ -112,7 +112,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItem : public base::SupportsUserData { // parallel downloading is enabled. Slices should have different offsets // so that they don't overlap. |finished| will be marked as true when the // download stream is successfully completed. - struct COMPONENTS_DOWNLOAD_EXPORT ReceivedSlice { + struct ReceivedSlice { ReceivedSlice(int64_t offset, int64_t received_bytes) : offset(offset), received_bytes(received_bytes), finished(false) {} @@ -432,6 +432,11 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItem : public base::SupportsUserData { // for target file path determination. virtual bool IsTransient() const = 0; + // Returns whether the download item corresponds to a parallel download. This + // usually means parallel download has been enabled and the download job is + // parallelizable. + virtual bool IsParallelDownload() const = 0; + // External state transitions/setters ---------------------------------------- // TODO(rdsmith): These should all be removed; the download item should diff --git a/chromium/components/download/public/common/download_item_impl.h b/chromium/components/download/public/common/download_item_impl.h index a92093e5497..bb8ae8623e1 100644 --- a/chromium/components/download/public/common/download_item_impl.h +++ b/chromium/components/download/public/common/download_item_impl.h @@ -23,9 +23,9 @@ #include "components/download/public/common/download_interrupt_reasons.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/download_request_handle_interface.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" #include "components/download/public/common/download_url_parameters.h" #include "components/download/public/common/resume_mode.h" -#include "services/network/public/cpp/shared_url_loader_factory.h" #include "url/gurl.h" namespace net { @@ -33,7 +33,6 @@ class URLRequestContextGetter; } namespace download { - class DownloadFile; class DownloadItemImplDelegate; class DownloadJob; @@ -275,6 +274,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl bool GetOpened() const override; base::Time GetLastAccessTime() const override; bool IsTransient() const override; + bool IsParallelDownload() const override; void OnContentCheckCompleted(DownloadDangerType danger_type, DownloadInterruptReason reason) override; void SetOpenWhenComplete(bool open) override; @@ -298,12 +298,12 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl // download resumption request. // TODO(qinmin): Remove |url_request_context_getter| once network service is // enabled. - virtual void Start( - std::unique_ptr<DownloadFile> download_file, - std::unique_ptr<DownloadRequestHandleInterface> req_handle, - const DownloadCreateInfo& new_create_info, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, - net::URLRequestContextGetter* url_request_context_getter); + virtual void Start(std::unique_ptr<DownloadFile> download_file, + std::unique_ptr<DownloadRequestHandleInterface> req_handle, + const DownloadCreateInfo& new_create_info, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, + net::URLRequestContextGetter* url_request_context_getter); // Needed because of intertwining with DownloadManagerImpl ------------------- diff --git a/chromium/components/download/public/common/download_job_factory.h b/chromium/components/download/public/common/download_job_factory.h index ba90f668d11..cd44f441cad 100644 --- a/chromium/components/download/public/common/download_job_factory.h +++ b/chromium/components/download/public/common/download_job_factory.h @@ -15,15 +15,12 @@ namespace net { class URLRequestContextGetter; } -namespace network { -class SharedURLLoaderFactory; -} // namespace network - namespace download { class DownloadItem; class DownloadJob; class DownloadRequestHandleInterface; +class DownloadURLLoaderFactoryGetter; // Factory class to create different kinds of DownloadJob. class COMPONENTS_DOWNLOAD_EXPORT DownloadJobFactory { @@ -33,7 +30,8 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadJobFactory { std::unique_ptr<DownloadRequestHandleInterface> req_handle, const DownloadCreateInfo& create_info, bool is_save_package_download, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, net::URLRequestContextGetter* url_request_context_getter); private: diff --git a/chromium/components/download/public/common/download_stats.h b/chromium/components/download/public/common/download_stats.h index 251dfa8e99a..15652249738 100644 --- a/chromium/components/download/public/common/download_stats.h +++ b/chromium/components/download/public/common/download_stats.h @@ -136,6 +136,10 @@ enum DownloadCountTypes { // Count of attempts for auto download resumption. AUTO_RESUMPTION_COUNT, + // Count of download attempts that are dropped due to content settings or + // request limiter before DownloadItem is created. + DOWNLOAD_DROPPED_COUNT, + DOWNLOAD_COUNT_TYPES_LAST_ENTRY }; diff --git a/chromium/components/download/public/common/download_utils.h b/chromium/components/download/public/common/download_utils.h index 9fc9c5a5e39..c3e4b9f246b 100644 --- a/chromium/components/download/public/common/download_utils.h +++ b/chromium/components/download/public/common/download_utils.h @@ -5,8 +5,11 @@ #ifndef COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_UTILS_H_ #define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_UTILS_H_ +#include "components/download/downloader/in_progress/download_entry.h" #include "components/download/public/common/download_export.h" #include "components/download/public/common/download_interrupt_reasons.h" +#include "components/download/public/common/download_item.h" +#include "components/download/public/common/download_source.h" #include "net/base/net_errors.h" #include "net/cert/cert_status_flags.h" #include "net/http/http_response_headers.h" @@ -57,6 +60,16 @@ COMPONENTS_DOWNLOAD_EXPORT int GetLoadFlags(DownloadUrlParameters* params, COMPONENTS_DOWNLOAD_EXPORT std::unique_ptr<net::HttpRequestHeaders> GetAdditionalRequestHeaders(DownloadUrlParameters* params); +// Helper functions for DownloadItem -> DownloadEntry for InProgressCache. +COMPONENTS_DOWNLOAD_EXPORT DownloadEntry CreateDownloadEntryFromItem( + const DownloadItem& item, + const std::string& request_origin, + DownloadSource download_source, + bool fetch_error_body, + const DownloadUrlParameters::RequestHeadersType& request_headers); + +COMPONENTS_DOWNLOAD_EXPORT uint64_t GetUniqueDownloadId(); + } // namespace download #endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_UTILS_H_ diff --git a/chromium/components/download/public/common/in_progress_download_manager.h b/chromium/components/download/public/common/in_progress_download_manager.h new file mode 100644 index 00000000000..9dba8c67b1d --- /dev/null +++ b/chromium/components/download/public/common/in_progress_download_manager.h @@ -0,0 +1,171 @@ +// Copyright 2018 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_PUBLIC_COMMON_IN_PROGRESS_DOWNLOAD_MANAGER_H_ +#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_IN_PROGRESS_DOWNLOAD_MANAGER_H_ + +#include <memory> +#include <vector> + +#include "base/memory/scoped_refptr.h" +#include "base/memory/weak_ptr.h" +#include "base/optional.h" +#include "components/download/public/common/download_export.h" +#include "components/download/public/common/download_file_factory.h" +#include "components/download/public/common/download_item_impl_delegate.h" +#include "components/download/public/common/url_download_handler.h" +#include "url/gurl.h" + +namespace net { +class URLRequestContextGetter; +} + +namespace network { +struct ResourceResponse; +} + +namespace download { + +class DownloadURLLoaderFactoryGetter; +class DownloadUrlParameters; +class InProgressCache; + +// Manager for handling all active downloads. +class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager + : public UrlDownloadHandler::Delegate, + public DownloadItemImplDelegate { + public: + using DownloadIdCallback = base::RepeatingCallback<void(uint32_t)>; + + // Class to be notified when download starts/stops. + class COMPONENTS_DOWNLOAD_EXPORT Delegate { + public: + // Intercepts the download to another system if applicable. Returns true if + // the download was intercepted. + virtual bool InterceptDownload( + const DownloadCreateInfo& download_create_info) = 0; + + // Called to get an ID for a new download. |callback| may be called + // synchronously. + virtual void GetNextId(const DownloadIdCallback& callback) = 0; + + // Gets the default download directory. + virtual base::FilePath GetDefaultDownloadDirectory() = 0; + + // Gets the download item for the given id. + // TODO(qinmin): remove this method and let InProgressDownloadManager + // create the DownloadItem from in-progress cache. + virtual DownloadItemImpl* GetDownloadItem( + uint32_t id, + bool new_download, + const DownloadCreateInfo& download_create_info) = 0; + + // Gets the URLRequestContextGetter for sending requests. + // TODO(qinmin): remove this once network service is fully enabled. + virtual net::URLRequestContextGetter* GetURLRequestContextGetter( + const DownloadCreateInfo& download_create_info) = 0; + + // Called when a new download is started. + virtual void OnNewDownloadStarted(DownloadItem* download) = 0; + }; + + using IsOriginSecureCallback = base::RepeatingCallback<bool(const GURL&)>; + InProgressDownloadManager(Delegate* delegate, + const IsOriginSecureCallback& is_origin_secure_cb); + ~InProgressDownloadManager() override; + // Called to start a download. + void BeginDownload( + std::unique_ptr<DownloadUrlParameters> params, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + uint32_t download_id, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url); + + // Intercepts a download from navigation. + void InterceptDownloadFromNavigation( + std::unique_ptr<network::ResourceRequest> resource_request, + int render_process_id, + int render_frame_id, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + std::vector<GURL> url_chain, + scoped_refptr<network::ResourceResponse> response, + net::CertStatus cert_status, + network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter); + + void Initialize(const base::FilePath& metadata_cache_dir, + base::OnceClosure callback); + + void StartDownload( + std::unique_ptr<DownloadCreateInfo> info, + std::unique_ptr<InputStream> stream, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + const DownloadUrlParameters::OnStartedCallback& on_started); + + // Shutting down the manager and stop all downloads. + void ShutDown(); + + // DownloadItemImplDelegate implementations. + void ResumeInterruptedDownload(std::unique_ptr<DownloadUrlParameters> params, + uint32_t id, + const GURL& site_url) override; + base::Optional<DownloadEntry> GetInProgressEntry( + DownloadItemImpl* download) override; + void ReportBytesWasted(DownloadItemImpl* download) override; + + void set_file_factory(std::unique_ptr<DownloadFileFactory> file_factory) { + file_factory_ = std::move(file_factory); + } + DownloadFileFactory* file_factory() { return file_factory_.get(); } + + private: + // UrlDownloadHandler::Delegate implementations. + void OnUrlDownloadStarted( + std::unique_ptr<DownloadCreateInfo> download_create_info, + std::unique_ptr<InputStream> input_stream, + scoped_refptr<DownloadURLLoaderFactoryGetter> shared_url_loader_factory, + const DownloadUrlParameters::OnStartedCallback& callback) override; + void OnUrlDownloadStopped(UrlDownloadHandler* downloader) override; + void OnUrlDownloadHandlerCreated( + UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader) override; + + // Start a download with given ID. + void StartDownloadWithId( + std::unique_ptr<DownloadCreateInfo> info, + std::unique_ptr<InputStream> stream, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + const DownloadUrlParameters::OnStartedCallback& on_started, + bool new_download, + uint32_t id); + + // Active download handlers. + std::vector<UrlDownloadHandler::UniqueUrlDownloadHandlerPtr> + url_download_handlers_; + + // Delegate to provide information to create a new download. Can be null. + Delegate* delegate_; + + // Factory for the creation of download files. + std::unique_ptr<DownloadFileFactory> file_factory_; + + // Cache for storing metadata about in progress downloads. + std::unique_ptr<InProgressCache> download_metadata_cache_; + + // listens to information about in-progress download items. + std::unique_ptr<DownloadItem::Observer> in_progress_download_observer_; + + // callback to check if an origin is secure. + IsOriginSecureCallback is_origin_secure_cb_; + + base::WeakPtrFactory<InProgressDownloadManager> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(InProgressDownloadManager); +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_IN_PROGRESS_DOWNLOAD_MANAGER_H_ diff --git a/chromium/components/download/public/common/mock_download_file.h b/chromium/components/download/public/common/mock_download_file.h index 8f5bec44208..7684ee89a6e 100644 --- a/chromium/components/download/public/common/mock_download_file.h +++ b/chromium/components/download/public/common/mock_download_file.h @@ -23,7 +23,7 @@ namespace download { class MockDownloadFile : public DownloadFile { public: MockDownloadFile(); - virtual ~MockDownloadFile(); + ~MockDownloadFile() override; // DownloadFile functions. // Using the legacy workaround for move-only types in mock methods. diff --git a/chromium/components/download/public/common/mock_download_item.h b/chromium/components/download/public/common/mock_download_item.h index ddb4684f746..d079bdc2b95 100644 --- a/chromium/components/download/public/common/mock_download_item.h +++ b/chromium/components/download/public/common/mock_download_item.h @@ -106,6 +106,7 @@ class MockDownloadItem : public DownloadItem { MOCK_CONST_METHOD0(GetOpened, bool()); MOCK_CONST_METHOD0(GetLastAccessTime, base::Time()); MOCK_CONST_METHOD0(IsTransient, bool()); + MOCK_CONST_METHOD0(IsParallelDownload, bool()); MOCK_METHOD2(OnContentCheckCompleted, void(DownloadDangerType, DownloadInterruptReason)); MOCK_METHOD1(SetOpenWhenComplete, void(bool)); diff --git a/chromium/components/download/public/common/mock_download_item_impl.h b/chromium/components/download/public/common/mock_download_item_impl.h index 752eda539db..7bbb9589d24 100644 --- a/chromium/components/download/public/common/mock_download_item_impl.h +++ b/chromium/components/download/public/common/mock_download_item_impl.h @@ -53,7 +53,8 @@ class MockDownloadItemImpl : public DownloadItemImpl { std::unique_ptr<DownloadFile> download_file, std::unique_ptr<DownloadRequestHandleInterface> req_handle, const DownloadCreateInfo& create_info, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, net::URLRequestContextGetter* url_request_context_getter) override { MockStart(download_file.get(), req_handle.get()); } diff --git a/chromium/components/download/public/common/url_download_handler.h b/chromium/components/download/public/common/url_download_handler.h index 5779dd135a6..ce059d92313 100644 --- a/chromium/components/download/public/common/url_download_handler.h +++ b/chromium/components/download/public/common/url_download_handler.h @@ -6,11 +6,13 @@ #define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_DOWNLOAD_HANDLER_H_ #include "components/download/public/common/download_export.h" +#include "components/download/public/common/download_url_loader_factory_getter.h" #include "components/download/public/common/download_url_parameters.h" #include "services/network/public/cpp/shared_url_loader_factory.h" namespace download { struct DownloadCreateInfo; +class DownloadURLLoaderFactoryGetter; class InputStream; // Class for handling the download of a url. Implemented by child classes. @@ -24,8 +26,7 @@ class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadHandler { virtual void OnUrlDownloadStarted( std::unique_ptr<DownloadCreateInfo> download_create_info, std::unique_ptr<InputStream> input_stream, - scoped_refptr<network::SharedURLLoaderFactory> - shared_url_loader_factory, + scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, const DownloadUrlParameters::OnStartedCallback& callback) = 0; // Called after the connection is cancelled or finished. diff --git a/chromium/components/download/public/common/url_download_handler_factory.h b/chromium/components/download/public/common/url_download_handler_factory.h index c43719488b1..c09c37af0cc 100644 --- a/chromium/components/download/public/common/url_download_handler_factory.h +++ b/chromium/components/download/public/common/url_download_handler_factory.h @@ -7,11 +7,8 @@ #include "components/download/public/common/url_download_handler.h" -namespace network { -class SharedURLLoaderFactory; -} - namespace download { +class DownloadURLLoaderFactoryGetter; class DownloadUrlParameters; // Class for handling the creation of a URLDownloadHandler. This is used to @@ -28,7 +25,8 @@ class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadHandlerFactory { static UrlDownloadHandler::UniqueUrlDownloadHandlerPtr Create( std::unique_ptr<download::DownloadUrlParameters> params, base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); virtual ~UrlDownloadHandlerFactory(); @@ -42,7 +40,8 @@ class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadHandlerFactory { CreateUrlDownloadHandler( std::unique_ptr<download::DownloadUrlParameters> params, base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory, + scoped_refptr<download::DownloadURLLoaderFactoryGetter> + url_loader_factory_getter, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) = 0; }; diff --git a/chromium/components/download/quarantine/quarantine.h b/chromium/components/download/quarantine/quarantine.h index cdd296d8f5e..01efc346a18 100644 --- a/chromium/components/download/quarantine/quarantine.h +++ b/chromium/components/download/quarantine/quarantine.h @@ -66,8 +66,9 @@ enum class QuarantineFileResult { // // Parameters: // |file| : Final name of the file. -// |source_url|: URL from which the file content was downloaded. -// |referrer_url|: Referring URL. +// |source_url|: URL from which the file content was downloaded. This is empty +// for off-the-record download. +// |referrer_url|: Referring URL. This is empty for off-the-record download. // |client_guid|: Only used on Windows. Identifies the client application // that downloaded the file. QuarantineFileResult QuarantineFile(const base::FilePath& file, diff --git a/chromium/components/download/quarantine/quarantine_win.cc b/chromium/components/download/quarantine/quarantine_win.cc index 948c59682a6..f240414d7a6 100644 --- a/chromium/components/download/quarantine/quarantine_win.cc +++ b/chromium/components/download/quarantine/quarantine_win.cc @@ -74,7 +74,8 @@ bool ZoneIdentifierPresentForFile(const base::FilePath& path) { std::vector<char> zone_identifier_contents_buffer(32); DWORD actual_length = 0; if (!ReadFile(file.Get(), &zone_identifier_contents_buffer.front(), - zone_identifier_contents_buffer.size(), &actual_length, NULL)) + zone_identifier_contents_buffer.size(), &actual_length, + nullptr)) return false; zone_identifier_contents_buffer.resize(actual_length); @@ -227,9 +228,9 @@ bool InvokeAttachmentServices(const base::FilePath& full_path, return false; } - // The source URL could be empty if it was not a valid URL or was not HTTP/S. - // If so, user "about:internet" as a fallback URL. The latter is known to - // reliably map to the Internet zone. + // The source URL could be empty if it was not a valid URL, or was not HTTP/S, + // or the download was off-the-record. If so, user "about:internet" as a + // fallback URL. The latter is known to reliably map to the Internet zone. // // In addition, URLs that are longer than INTERNET_MAX_URL_LENGTH are also // known to cause problems for URLMon. Hence also use "about:internet" in |