diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-01-20 13:40:20 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-01-22 12:41:23 +0000 |
commit | 7961cea6d1041e3e454dae6a1da660b453efd238 (patch) | |
tree | c0eeb4a9ff9ba32986289c1653d9608e53ccb444 /chromium/components/download | |
parent | b7034d0803538058e5c9d904ef03cf5eab34f6ef (diff) | |
download | qtwebengine-chromium-7961cea6d1041e3e454dae6a1da660b453efd238.tar.gz |
BASELINE: Update Chromium to 78.0.3904.130
Change-Id: If185e0c0061b3437531c97c9c8c78f239352a68b
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/components/download')
75 files changed, 637 insertions, 1068 deletions
diff --git a/chromium/components/download/content/factory/BUILD.gn b/chromium/components/download/content/factory/BUILD.gn index d9eb53fa253..5d754f7bd5f 100644 --- a/chromium/components/download/content/factory/BUILD.gn +++ b/chromium/components/download/content/factory/BUILD.gn @@ -22,7 +22,6 @@ source_set("factory") { "//components/download/network", "//components/keyed_service/content", "//components/leveldb_proto", - "//components/leveldb_proto/content:factory", "//content/public/browser", "//net", "//url", diff --git a/chromium/components/download/content/factory/download_service_factory_helper.cc b/chromium/components/download/content/factory/download_service_factory_helper.cc index 3bd089b9563..7be7ce8f4cb 100644 --- a/chromium/components/download/content/factory/download_service_factory_helper.cc +++ b/chromium/components/download/content/factory/download_service_factory_helper.cc @@ -25,7 +25,6 @@ #include "components/download/internal/background_service/scheduler/scheduler_impl.h" #include "components/download/public/common/simple_download_manager_coordinator.h" #include "components/download/public/task/empty_task_scheduler.h" -#include "components/leveldb_proto/content/proto_database_provider_factory.h" #include "components/leveldb_proto/public/proto_database_provider.h" #include "services/network/public/cpp/shared_url_loader_factory.h" @@ -103,6 +102,7 @@ std::unique_ptr<DownloadService> BuildDownloadService( network::NetworkConnectionTracker* network_connection_tracker, const base::FilePath& storage_dir, SimpleDownloadManagerCoordinator* download_manager_coordinator, + leveldb_proto::ProtoDatabaseProvider* proto_db_provider, const scoped_refptr<base::SequencedTaskRunner>& background_task_runner, std::unique_ptr<TaskScheduler> task_scheduler) { auto config = Configuration::CreateFromFinch(); @@ -112,10 +112,7 @@ std::unique_ptr<DownloadService> BuildDownloadService( auto entry_db_storage_dir = storage_dir.Append(kEntryDBStorageDir); - leveldb_proto::ProtoDatabaseProvider* db_provider = - leveldb_proto::ProtoDatabaseProviderFactory::GetForKey( - simple_factory_key); - auto entry_db = db_provider->GetDB<protodb::Entry>( + auto entry_db = proto_db_provider->GetDB<protodb::Entry>( leveldb_proto::ProtoDbType::DOWNLOAD_STORE, entry_db_storage_dir, background_task_runner); auto store = std::make_unique<DownloadStore>(std::move(entry_db)); diff --git a/chromium/components/download/content/factory/download_service_factory_helper.h b/chromium/components/download/content/factory/download_service_factory_helper.h index 4d4d4e24f90..d7384dcae2f 100644 --- a/chromium/components/download/content/factory/download_service_factory_helper.h +++ b/chromium/components/download/content/factory/download_service_factory_helper.h @@ -21,6 +21,10 @@ class NetworkConnectionTracker; class SharedURLLoaderFactory; } // namespace network +namespace leveldb_proto { +class ProtoDatabaseProvider; +} // namespace leveldb_proto + namespace download { class DownloadService; @@ -43,6 +47,7 @@ std::unique_ptr<DownloadService> BuildDownloadService( network::NetworkConnectionTracker* network_connection_tracker, const base::FilePath& storage_dir, SimpleDownloadManagerCoordinator* download_manager_coordinator, + leveldb_proto::ProtoDatabaseProvider* proto_db_provider, const scoped_refptr<base::SequencedTaskRunner>& background_task_runner, std::unique_ptr<TaskScheduler> task_scheduler); diff --git a/chromium/components/download/database/download_db_impl.cc b/chromium/components/download/database/download_db_impl.cc index b1a5ca60889..2e728e8fe1c 100644 --- a/chromium/components/download/database/download_db_impl.cc +++ b/chromium/components/download/database/download_db_impl.cc @@ -20,7 +20,6 @@ namespace { const int kMaxNumInitializeAttempts = 3; -const char kDatabaseClientName[] = "DownloadDB"; using ProtoKeyVector = std::vector<std::string>; using ProtoEntryVector = std::vector<download_pb::DownloadDBEntry>; using ProtoKeyEntryVector = @@ -46,30 +45,27 @@ void OnUpdateDone(bool success) { } // namespace -DownloadDBImpl::DownloadDBImpl(DownloadNamespace download_namespace, - const base::FilePath& database_dir) - : DownloadDBImpl( - download_namespace, - database_dir, - leveldb_proto::ProtoDatabaseProvider::CreateUniqueDB< - download_pb::DownloadDBEntry>( - base::CreateSequencedTaskRunnerWithTraits( - {base::MayBlock(), - // USER_VISIBLE because it is required to display - // chrome://downloads. https://crbug.com/976223 - base::TaskPriority::USER_VISIBLE, - base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}))) {} - DownloadDBImpl::DownloadDBImpl( DownloadNamespace download_namespace, const base::FilePath& database_dir, + leveldb_proto::ProtoDatabaseProvider* db_provider) + : download_namespace_(download_namespace) { + DCHECK(!database_dir.empty()); + db_ = db_provider->GetDB<download_pb::DownloadDBEntry>( + leveldb_proto::ProtoDbType::DOWNLOAD_DB, database_dir, + base::CreateSequencedTaskRunnerWithTraits( + {base::ThreadPool(), base::MayBlock(), + // USER_VISIBLE because it is required to display chrome://downloads. + // https://crbug.com/976223 + base::TaskPriority::USER_VISIBLE, + base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})); +} + +DownloadDBImpl::DownloadDBImpl( + DownloadNamespace download_namespace, std::unique_ptr<leveldb_proto::ProtoDatabase<download_pb::DownloadDBEntry>> db) - : database_dir_(database_dir), - db_(std::move(db)), - is_initialized_(false), - download_namespace_(download_namespace), - num_initialize_attempts_(0) {} + : db_(std::move(db)), download_namespace_(download_namespace) {} DownloadDBImpl::~DownloadDBImpl() = default; @@ -84,7 +80,7 @@ void DownloadDBImpl::Initialize(DownloadDBCallback callback) { leveldb_env::Options options = leveldb_proto::CreateSimpleOptions(); options.reuse_logs = false; options.write_buffer_size = 64 << 10; // 64 KiB - db_->Init(kDatabaseClientName, database_dir_, options, + db_->Init(options, base::BindOnce(&DownloadDBImpl::OnDatabaseInitialized, weak_factory_.GetWeakPtr(), std::move(callback))); } @@ -153,8 +149,10 @@ void DownloadDBImpl::OnAllEntriesLoaded( std::move(callback).Run(success, std::move(result)); } -void DownloadDBImpl::OnDatabaseInitialized(DownloadDBCallback callback, - bool success) { +void DownloadDBImpl::OnDatabaseInitialized( + DownloadDBCallback callback, + leveldb_proto::Enums::InitStatus status) { + bool success = status == leveldb_proto::Enums::InitStatus::kOK; if (!success) { DestroyAndReinitialize(std::move(callback)); return; diff --git a/chromium/components/download/database/download_db_impl.h b/chromium/components/download/database/download_db_impl.h index a1a6b65b781..2a85c7cb564 100644 --- a/chromium/components/download/database/download_db_impl.h +++ b/chromium/components/download/database/download_db_impl.h @@ -18,16 +18,20 @@ namespace download_pb { class DownloadDBEntry; } +namespace leveldb_proto { +class ProtoDatabaseProvider; +} // namespace leveldb_proto + namespace download { // A protodb Implementation of DownloadDB. class DownloadDBImpl : public DownloadDB { public: DownloadDBImpl(DownloadNamespace download_namespace, - const base::FilePath& database_dir); + const base::FilePath& database_dir, + leveldb_proto::ProtoDatabaseProvider* db_provider); DownloadDBImpl( DownloadNamespace download_namespace, - const base::FilePath& database_dir, std::unique_ptr< leveldb_proto::ProtoDatabase<download_pb::DownloadDBEntry>> db); ~DownloadDBImpl() override; @@ -51,7 +55,8 @@ class DownloadDBImpl : public DownloadDB { std::string GetEntryKey(const std::string& guid) const; // Called when database is initialized. - void OnDatabaseInitialized(DownloadDBCallback callback, bool success); + void OnDatabaseInitialized(DownloadDBCallback callback, + leveldb_proto::Enums::InitStatus status); // Called when database is destroyed. void OnDatabaseDestroyed(DownloadDBCallback callback, bool success); @@ -65,21 +70,18 @@ class DownloadDBImpl : public DownloadDB { bool success, std::unique_ptr<std::vector<download_pb::DownloadDBEntry>> entries); - // Directory to store |db_|. - base::FilePath database_dir_; - // Proto db for storing all the entries. std::unique_ptr<leveldb_proto::ProtoDatabase<download_pb::DownloadDBEntry>> db_; // Whether the object is initialized. - bool is_initialized_; + bool is_initialized_ = false; // Namespace of this db. DownloadNamespace download_namespace_; // Number of initialize attempts. - int num_initialize_attempts_; + int num_initialize_attempts_ = 0; base::WeakPtrFactory<DownloadDBImpl> weak_factory_{this}; diff --git a/chromium/components/download/database/download_db_impl_unittest.cc b/chromium/components/download/database/download_db_impl_unittest.cc index cd582a2908f..4df08226694 100644 --- a/chromium/components/download/database/download_db_impl_unittest.cc +++ b/chromium/components/download/database/download_db_impl_unittest.cc @@ -50,8 +50,7 @@ class DownloadDBTest : public testing::Test { &db_entries_); db_ = db.get(); download_db_.reset(new DownloadDBImpl( - DownloadNamespace::NAMESPACE_BROWSER_DOWNLOAD, - base::FilePath(FILE_PATH_LITERAL("/test/db/fakepath")), std::move(db))); + DownloadNamespace::NAMESPACE_BROWSER_DOWNLOAD, std::move(db))); } void InitCallback(bool success) { init_success_ = success; } @@ -99,7 +98,7 @@ TEST_F(DownloadDBTest, InitializeSucceeded) { download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); ASSERT_TRUE(IsInitialized()); ASSERT_TRUE(init_success_); @@ -111,7 +110,7 @@ TEST_F(DownloadDBTest, InitializeFailed) { download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(false); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kError); ASSERT_FALSE(IsInitialized()); ASSERT_FALSE(init_success_); @@ -122,7 +121,7 @@ TEST_F(DownloadDBTest, LoadEntries) { CreateDatabase(); download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); ASSERT_TRUE(IsInitialized()); std::vector<DownloadDBEntry> loaded_entries; @@ -142,7 +141,7 @@ TEST_F(DownloadDBTest, AddEntry) { CreateDatabase(); download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); ASSERT_TRUE(IsInitialized()); DownloadDBEntry entry = CreateDownloadDBEntry(); @@ -173,7 +172,7 @@ TEST_F(DownloadDBTest, ReplaceEntry) { CreateDatabase(); download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); ASSERT_TRUE(IsInitialized()); InProgressInfo in_progress_info; @@ -210,7 +209,7 @@ TEST_F(DownloadDBTest, Remove) { CreateDatabase(); download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); ASSERT_TRUE(IsInitialized()); download_db_->Remove(first.GetGuid()); @@ -231,7 +230,7 @@ TEST_F(DownloadDBTest, DestroyAndReinitialize) { CreateDatabase(); download_db_->Initialize( base::BindOnce(&DownloadDBTest::InitCallback, base::Unretained(this))); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); ASSERT_TRUE(IsInitialized()); std::vector<DownloadDBEntry> loaded_entries; diff --git a/chromium/components/download/internal/background_service/blob_task_proxy.h b/chromium/components/download/internal/background_service/blob_task_proxy.h index 2714fb74b4b..7de3466b4b0 100644 --- a/chromium/components/download/internal/background_service/blob_task_proxy.h +++ b/chromium/components/download/internal/background_service/blob_task_proxy.h @@ -12,7 +12,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/single_thread_task_runner.h" -#include "storage/common/blob_storage/blob_storage_constants.h" +#include "storage/browser/blob/blob_storage_constants.h" namespace storage { class BlobDataHandle; diff --git a/chromium/components/download/internal/background_service/controller_impl.cc b/chromium/components/download/internal/background_service/controller_impl.cc index 2469409cbfc..3c02debb23e 100644 --- a/chromium/components/download/internal/background_service/controller_impl.cc +++ b/chromium/components/download/internal/background_service/controller_impl.cc @@ -212,6 +212,8 @@ void ControllerImpl::StartDownload(const DownloadParams& params) { void ControllerImpl::PauseDownload(const std::string& guid) { DCHECK(controller_state_ == State::READY || controller_state_ == State::UNAVAILABLE); + stats::LogServiceApiAction(GetOwnerOfDownload(guid), + stats::ServiceApiAction::PAUSE_DOWNLOAD); if (controller_state_ != State::READY) return; @@ -234,6 +236,8 @@ void ControllerImpl::PauseDownload(const std::string& guid) { void ControllerImpl::ResumeDownload(const std::string& guid) { DCHECK(controller_state_ == State::READY || controller_state_ == State::UNAVAILABLE); + stats::LogServiceApiAction(GetOwnerOfDownload(guid), + stats::ServiceApiAction::RESUME_DOWNLOAD); if (controller_state_ != State::READY) return; @@ -252,6 +256,8 @@ void ControllerImpl::ResumeDownload(const std::string& guid) { void ControllerImpl::CancelDownload(const std::string& guid) { DCHECK(controller_state_ == State::READY || controller_state_ == State::UNAVAILABLE); + stats::LogServiceApiAction(GetOwnerOfDownload(guid), + stats::ServiceApiAction::CANCEL_DOWNLOAD); if (controller_state_ != State::READY) return; @@ -274,6 +280,8 @@ void ControllerImpl::ChangeDownloadCriteria(const std::string& guid, const SchedulingParams& params) { DCHECK(controller_state_ == State::READY || controller_state_ == State::UNAVAILABLE); + stats::LogServiceApiAction(GetOwnerOfDownload(guid), + stats::ServiceApiAction::CHANGE_CRITERIA); if (controller_state_ != State::READY) return; diff --git a/chromium/components/download/internal/background_service/download_service_impl.cc b/chromium/components/download/internal/background_service/download_service_impl.cc index 6a30111d255..f41e6d7f9bb 100644 --- a/chromium/components/download/internal/background_service/download_service_impl.cc +++ b/chromium/components/download/internal/background_service/download_service_impl.cc @@ -89,8 +89,6 @@ void DownloadServiceImpl::StartDownload(const DownloadParams& download_params) { } void DownloadServiceImpl::PauseDownload(const std::string& guid) { - stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), - stats::ServiceApiAction::PAUSE_DOWNLOAD); if (startup_completed_) { controller_->PauseDownload(guid); } else { @@ -100,8 +98,6 @@ void DownloadServiceImpl::PauseDownload(const std::string& guid) { } void DownloadServiceImpl::ResumeDownload(const std::string& guid) { - stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), - stats::ServiceApiAction::RESUME_DOWNLOAD); if (startup_completed_) { controller_->ResumeDownload(guid); } else { @@ -112,8 +108,6 @@ void DownloadServiceImpl::ResumeDownload(const std::string& guid) { } void DownloadServiceImpl::CancelDownload(const std::string& guid) { - stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), - stats::ServiceApiAction::CANCEL_DOWNLOAD); if (startup_completed_) { controller_->CancelDownload(guid); } else { @@ -126,8 +120,6 @@ void DownloadServiceImpl::CancelDownload(const std::string& guid) { void DownloadServiceImpl::ChangeDownloadCriteria( const std::string& guid, const SchedulingParams& params) { - stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), - stats::ServiceApiAction::CHANGE_CRITERIA); if (startup_completed_) { controller_->ChangeDownloadCriteria(guid, params); } else { 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 19a7e6d353f..5c4b9dfffc7 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 @@ -6,10 +6,10 @@ #include "base/bind.h" #include "base/guid.h" -#include "base/message_loop/message_loop.h" +#include "base/message_loop/message_pump_type.h" #include "base/run_loop.h" #include "base/test/bind_test_util.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "base/threading/thread.h" #include "net/base/io_buffer.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" @@ -89,7 +89,7 @@ class InMemoryDownloadTest : public testing::Test { void SetUp() override { io_thread_.reset(new base::Thread("Network and Blob IO thread")); - base::Thread::Options options(base::MessageLoop::TYPE_IO, 0); + base::Thread::Options options(base::MessagePumpType::IO, 0); io_thread_->StartWithOptions(options); base::RunLoop loop; @@ -172,7 +172,7 @@ class InMemoryDownloadTest : public testing::Test { std::unique_ptr<base::Thread> io_thread_; // Created before other objects to provide test environment. - base::test::ScopedTaskEnvironment scoped_task_environment_; + base::test::TaskEnvironment task_environment_; std::unique_ptr<InMemoryDownloadImpl> download_; std::unique_ptr<NiceMock<MockDelegate>> mock_delegate_; 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 e3da016383e..d6ad0b4b9a7 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 @@ -8,7 +8,7 @@ #include "base/run_loop.h" #include "base/test/power_monitor_test_base.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "components/download/internal/background_service/scheduler/battery_status_listener_impl.h" #include "components/download/network/network_status_listener_impl.h" #include "services/network/test/test_network_connection_tracker.h" @@ -146,7 +146,7 @@ class DeviceStatusListenerTest : public testing::Test { MockObserver mock_observer_; // Needed for network change notifier and power monitor. - base::test::ScopedTaskEnvironment task_environment_; + base::test::TaskEnvironment task_environment_; base::PowerMonitorTestSource* power_source_; TestBatteryStatusListener* test_battery_listener_; }; diff --git a/chromium/components/download/internal/background_service/stats.cc b/chromium/components/download/internal/background_service/stats.cc index 282cf52abfd..ac937aba898 100644 --- a/chromium/components/download/internal/background_service/stats.cc +++ b/chromium/components/download/internal/background_service/stats.cc @@ -15,10 +15,6 @@ namespace download { namespace stats { namespace { -// The maximum tracked file size in KB, larger files will fall into overflow -// bucket. -const int64_t kMaxFileSizeKB = 4 * 1024 * 1024; /* 4GB */ - // Enum used by UMA metrics to track various reasons of pausing a download. enum class PauseReason { // The download was paused. The reason can be anything. @@ -104,35 +100,6 @@ std::string ClientToHistogramSuffix(DownloadClient client) { return std::string(); } -// Converts CompletionType to histogram suffix. -// Should maps to suffix string in histograms.xml. -std::string CompletionTypeToHistogramSuffix(CompletionType type) { - switch (type) { - case CompletionType::SUCCEED: - return "Succeed"; - case CompletionType::FAIL: - return "Fail"; - case CompletionType::ABORT: - return "Abort"; - case CompletionType::TIMEOUT: - return "Timeout"; - case CompletionType::UNKNOWN: - return "Unknown"; - case CompletionType::CANCEL: - return "Cancel"; - case CompletionType::OUT_OF_RETRIES: - return "OutOfRetries"; - case CompletionType::OUT_OF_RESUMPTIONS: - return "OutOfResumptions"; - case CompletionType::UPLOAD_TIMEOUT: - return "UploadTimeout"; - case CompletionType::COUNT: - NOTREACHED(); - } - NOTREACHED(); - return std::string(); -} - // Converts FileCleanupReason to histogram suffix. // Should maps to suffix string in histograms.xml. std::string FileCleanupReasonToHistogramSuffix(FileCleanupReason reason) { @@ -234,14 +201,6 @@ void LogDownloadCompletion(CompletionType type, uint64_t file_size_bytes) { // TODO(xingliu): Use DownloadItem::GetStartTime and DownloadItem::GetEndTime // to record the completion time to histogram "Download.Service.Finish.Time". // Also propagates and records the mime type here. - - // Records the file size. - std::string name("Download.Service.Finish.FileSize"); - uint64_t file_size_kb = file_size_bytes / 1024; - base::UmaHistogramCustomCounts(name, file_size_kb, 1, kMaxFileSizeKB, 50); - - name.append(".").append(CompletionTypeToHistogramSuffix(type)); - base::UmaHistogramCustomCounts(name, file_size_kb, 1, kMaxFileSizeKB, 50); } void LogDownloadPauseReason(const DownloadBlockageStatus& blockage_status, diff --git a/chromium/components/download/internal/common/BUILD.gn b/chromium/components/download/internal/common/BUILD.gn index 63db61acc4b..4959f0a026c 100644 --- a/chromium/components/download/internal/common/BUILD.gn +++ b/chromium/components/download/internal/common/BUILD.gn @@ -39,8 +39,6 @@ source_set("internal") { "download_stats.cc", "download_task_runner.cc", "download_ukm_helper.cc", - "download_url_loader_factory_getter.cc", - "download_url_loader_factory_getter_impl.cc", "download_utils.cc", "download_worker.cc", "download_worker.h", @@ -59,9 +57,12 @@ source_set("internal") { "stream_handle_input_stream.cc", "url_download_handler_factory.cc", "url_download_request_handle.cc", + "url_loader_factory_provider.cc", ] public_deps = [ + "//components/services/quarantine/public/mojom", + "//mojo/public/cpp/bindings", "//services/network/public/mojom", ] diff --git a/chromium/components/download/internal/common/android/java/src/org/chromium/components/download/DownloadCollectionBridge.java b/chromium/components/download/internal/common/android/java/src/org/chromium/components/download/DownloadCollectionBridge.java index cdf3e44e5bc..e710401f2f1 100644 --- a/chromium/components/download/internal/common/android/java/src/org/chromium/components/download/DownloadCollectionBridge.java +++ b/chromium/components/download/internal/common/android/java/src/org/chromium/components/download/DownloadCollectionBridge.java @@ -73,7 +73,7 @@ public class DownloadCollectionBridge { * @param filePath File path of the download. * @return True if the download needs to be published, or false otherwise. */ - protected boolean needToPublishDownload(final String filePath) { + public boolean needToPublishDownload(final String filePath) { return false; } @@ -116,10 +116,12 @@ public class DownloadCollectionBridge { } /** - * @return whether a download with the file name exists. + * Gets the content URI of the download that has the given file name. + * @param pendingUri name of the file. + * @return Uri of the download with the given display name. */ - protected boolean checkFileNameExists(final String fileName) { - return false; + public Uri getDownloadUriForFileName(final String fileName) { + return null; } /** @@ -251,7 +253,8 @@ public class DownloadCollectionBridge { */ @CalledByNative private static boolean fileNameExists(final String fileName) { - return getDownloadCollectionBridge().checkFileNameExists(fileName); + Uri uri = getDownloadCollectionBridge().getDownloadUriForFileName(fileName); + return uri != null; } /** diff --git a/chromium/components/download/internal/common/base_file.cc b/chromium/components/download/internal/common/base_file.cc index 9497a10ae44..5f955ab6340 100644 --- a/chromium/components/download/internal/common/base_file.cc +++ b/chromium/components/download/internal/common/base_file.cc @@ -25,7 +25,6 @@ #include "components/download/public/common/download_stats.h" #include "components/download/quarantine/quarantine.h" #include "crypto/secure_hash.h" -#include "services/service_manager/public/cpp/connector.h" #if defined(OS_WIN) #include "components/services/quarantine/public/cpp/quarantine_features_win.h" @@ -651,10 +650,10 @@ void BaseFile::AnnotateWithSourceInformation( const std::string& client_guid, const GURL& source_url, const GURL& referrer_url, - std::unique_ptr<service_manager::Connector> connector, + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine, OnAnnotationDoneCallback on_annotation_done_callback) { GURL authority_url = GetEffectiveAuthorityURL(source_url, referrer_url); - if (!connector) { + if (!remote_quarantine) { #if defined(OS_WIN) QuarantineFileResult result = quarantine::SetInternetZoneIdentifierDirectly( full_path_, authority_url, referrer_url); @@ -664,12 +663,11 @@ void BaseFile::AnnotateWithSourceInformation( std::move(on_annotation_done_callback) .Run(QuarantineFileResultToReason(result)); } else { - connector->BindInterface(quarantine::mojom::kServiceName, - mojo::MakeRequest(&quarantine_service_)); + quarantine_service_.Bind(std::move(remote_quarantine)); on_annotation_done_callback_ = std::move(on_annotation_done_callback); - quarantine_service_.set_connection_error_handler(base::BindOnce( + quarantine_service_.set_disconnect_handler(base::BindOnce( &BaseFile::OnQuarantineServiceError, weak_factory_.GetWeakPtr(), authority_url, referrer_url)); diff --git a/chromium/components/download/internal/common/download_db_cache.cc b/chromium/components/download/internal/common/download_db_cache.cc index e9f8afffb41..fc6c029813d 100644 --- a/chromium/components/download/internal/common/download_db_cache.cc +++ b/chromium/components/download/internal/common/download_db_cache.cc @@ -73,6 +73,9 @@ void CleanUpInProgressEntry(DownloadDBEntry* entry) { in_progress_info->state = DownloadItem::DownloadState::INTERRUPTED; in_progress_info->interrupt_reason = download::DOWNLOAD_INTERRUPT_REASON_CRASH; + // We should not trust the hash value for crashed in-progress download, as + // hash is not calculated for when download is in progress. + in_progress_info->hash = std::string(); } } diff --git a/chromium/components/download/internal/common/download_db_cache_unittest.cc b/chromium/components/download/internal/common/download_db_cache_unittest.cc index e39763bc48b..02c322875ae 100644 --- a/chromium/components/download/internal/common/download_db_cache_unittest.cc +++ b/chromium/components/download/internal/common/download_db_cache_unittest.cc @@ -9,7 +9,7 @@ #include "base/bind.h" #include "base/guid.h" #include "base/run_loop.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "base/test/test_mock_time_task_runner.h" #include "components/download/database/download_db_conversions.h" #include "components/download/database/download_db_entry.h" @@ -37,6 +37,7 @@ DownloadDBEntry CreateDownloadDBEntry() { download_info.guid = base::GenerateGUID(); static int id = 0; download_info.id = ++id; + download_info.in_progress_info->hash = "abc"; entry.download_info = download_info; return entry; } @@ -53,6 +54,7 @@ void CleanUpInProgressEntry(DownloadDBEntry* entry) { entry->download_info->in_progress_info->state = DownloadItem::INTERRUPTED; entry->download_info->in_progress_info->interrupt_reason = DOWNLOAD_INTERRUPT_REASON_CRASH; + entry->download_info->in_progress_info->hash = std::string(); } } // namespace @@ -70,8 +72,7 @@ class DownloadDBCacheTest : public testing::Test { &db_entries_); db_ = db.get(); auto download_db = std::make_unique<DownloadDBImpl>( - DownloadNamespace::NAMESPACE_BROWSER_DOWNLOAD, - base::FilePath(FILE_PATH_LITERAL("/test/db/fakepath")), std::move(db)); + DownloadNamespace::NAMESPACE_BROWSER_DOWNLOAD, std::move(db)); db_cache_ = std::make_unique<DownloadDBCache>(std::move(download_db)); db_cache_->SetTimerTaskRunnerForTesting(task_runner_); } @@ -108,7 +109,7 @@ class DownloadDBCacheTest : public testing::Test { leveldb_proto::test::FakeDB<download_pb::DownloadDBEntry>* db_; std::unique_ptr<DownloadDBCache> db_cache_; scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; - base::test::ScopedTaskEnvironment scoped_task_environment_; + base::test::TaskEnvironment task_environment_; DISALLOW_COPY_AND_ASSIGN(DownloadDBCacheTest); }; @@ -119,7 +120,7 @@ TEST_F(DownloadDBCacheTest, InitializeAndRetrieve) { db_cache_->Initialize( base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this), &loaded_entries)); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); db_->LoadCallback(true); ASSERT_EQ(loaded_entries.size(), 2u); @@ -141,7 +142,7 @@ TEST_F(DownloadDBCacheTest, AddNewEntry) { db_cache_->Initialize( base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this), &loaded_entries)); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); db_->LoadCallback(true); ASSERT_EQ(loaded_entries.size(), 2u); @@ -166,7 +167,7 @@ TEST_F(DownloadDBCacheTest, ModifyExistingEntry) { db_cache_->Initialize( base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this), &loaded_entries)); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); db_->LoadCallback(true); ASSERT_EQ(loaded_entries.size(), 2u); @@ -216,7 +217,7 @@ TEST_F(DownloadDBCacheTest, FilePathChange) { db_cache_->Initialize( base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this), &loaded_entries)); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); db_->LoadCallback(true); ASSERT_EQ(loaded_entries.size(), 1u); ASSERT_EQ(loaded_entries[0].download_info->in_progress_info->current_path, @@ -245,7 +246,7 @@ TEST_F(DownloadDBCacheTest, RemoveEntry) { db_cache_->Initialize( base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this), &loaded_entries)); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); db_->LoadCallback(true); ASSERT_EQ(loaded_entries.size(), 2u); @@ -272,7 +273,7 @@ TEST_F(DownloadDBCacheTest, RemoveWhileModifyExistingEntry) { db_cache_->Initialize( base::BindOnce(&DownloadDBCacheTest::InitCallback, base::Unretained(this), &loaded_entries)); - db_->InitCallback(true); + db_->InitStatusCallback(leveldb_proto::Enums::InitStatus::kOK); db_->LoadCallback(true); ASSERT_EQ(loaded_entries.size(), 2u); // Let the DBCache to cache the entry first. diff --git a/chromium/components/download/internal/common/download_file_impl.cc b/chromium/components/download/internal/common/download_file_impl.cc index f0ebeb52667..ce58672bd20 100644 --- a/chromium/components/download/internal/common/download_file_impl.cc +++ b/chromium/components/download/internal/common/download_file_impl.cc @@ -27,8 +27,6 @@ #include "crypto/sha2.h" #include "mojo/public/c/system/types.h" #include "net/base/io_buffer.h" -#include "services/network/public/cpp/features.h" -#include "services/service_manager/public/cpp/connector.h" #if defined(OS_ANDROID) #include "base/android/content_uri_utils.h" @@ -357,14 +355,14 @@ void DownloadFileImpl::RenameAndAnnotate( const std::string& client_guid, const GURL& source_url, const GURL& referrer_url, - std::unique_ptr<service_manager::Connector> connector, + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine, const RenameCompletionCallback& callback) { std::unique_ptr<RenameParameters> parameters(new RenameParameters( ANNOTATE_WITH_SOURCE_INFORMATION, full_path, callback)); parameters->client_guid = client_guid; parameters->source_url = source_url; parameters->referrer_url = referrer_url; - parameters->connector = std::move(connector); + parameters->remote_quarantine = std::move(remote_quarantine); RenameWithRetryInternal(std::move(parameters)); } @@ -484,7 +482,7 @@ void DownloadFileImpl::RenameWithRetryInternal( download::features::kPreventDownloadsWithSamePath)) { file_.AnnotateWithSourceInformation( parameters->client_guid, parameters->source_url, - parameters->referrer_url, std::move(parameters->connector), + parameters->referrer_url, std::move(parameters->remote_quarantine), base::BindOnce(&DownloadFileImpl::OnRenameComplete, weak_factory_.GetWeakPtr(), new_path, parameters->completion_callback)); @@ -557,6 +555,8 @@ void DownloadFileImpl::Pause() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); is_paused_ = true; record_stream_bandwidth_ = false; + for (auto& stream : source_streams_) + stream.second->ClearDataReadyCallback(); } void DownloadFileImpl::Resume() { @@ -564,13 +564,10 @@ void DownloadFileImpl::Resume() { DCHECK(is_paused_); is_paused_ = false; - if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) - return; - for (auto& stream : source_streams_) { SourceStream* source_stream = stream.second.get(); if (!source_stream->is_finished()) { - StreamActive(source_stream, MOJO_RESULT_OK); + ActivateStream(source_stream); } } } @@ -578,8 +575,7 @@ void DownloadFileImpl::Resume() { void DownloadFileImpl::StreamActive(SourceStream* source_stream, MojoResult result) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - if (base::FeatureList::IsEnabled(network::features::kNetworkService) && - is_paused_) + if (is_paused_) return; base::TimeTicks start(base::TimeTicks::Now()); @@ -728,15 +724,19 @@ void DownloadFileImpl::RegisterAndActivateStream(SourceStream* source_stream) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); source_stream->Initialize(); - source_stream->RegisterDataReadyCallback( - base::Bind(&DownloadFileImpl::StreamActive, weak_factory_.GetWeakPtr(), - source_stream)); // Truncate |source_stream|'s length if necessary. for (const auto& received_slice : received_slices_) { source_stream->TruncateLengthWithWrittenDataBlock( received_slice.offset, received_slice.received_bytes); } num_active_streams_++; + ActivateStream(source_stream); +} + +void DownloadFileImpl::ActivateStream(SourceStream* source_stream) { + source_stream->RegisterDataReadyCallback( + base::Bind(&DownloadFileImpl::StreamActive, weak_factory_.GetWeakPtr(), + source_stream)); StreamActive(source_stream, MOJO_RESULT_OK); } diff --git a/chromium/components/download/internal/common/download_file_unittest.cc b/chromium/components/download/internal/common/download_file_unittest.cc index 41fd65fe478..e36751b69f4 100644 --- a/chromium/components/download/internal/common/download_file_unittest.cc +++ b/chromium/components/download/internal/common/download_file_unittest.cc @@ -16,7 +16,7 @@ #include "base/run_loop.h" #include "base/single_thread_task_runner.h" #include "base/strings/string_number_conversions.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "base/test/test_file_util.h" #include "base/threading/thread_task_runner_handle.h" #include "build/build_config.h" @@ -26,7 +26,6 @@ #include "components/download/public/common/download_interrupt_reasons.h" #include "components/download/public/common/mock_input_stream.h" #include "net/base/net_errors.h" -#include "services/service_manager/public/cpp/connector.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -397,7 +396,7 @@ class DownloadFileTest : public testing::Test { case RENAME_AND_ANNOTATE: download_file_->RenameAndAnnotate( full_path, "12345678-ABCD-1234-DCBA-123456789ABC", GURL(), GURL(), - nullptr, completion_callback); + mojo::NullRemote(), completion_callback); break; } } @@ -501,7 +500,7 @@ class DownloadFileTest : public testing::Test { closure.Run(); } - base::test::ScopedTaskEnvironment scoped_task_environment_; + base::test::TaskEnvironment task_environment_; }; // DownloadFile::RenameAndAnnotate and DownloadFile::RenameAndUniquify have a diff --git a/chromium/components/download/internal/common/download_item_impl.cc b/chromium/components/download/internal/common/download_item_impl.cc index 386660c5ee9..f24ef713bf7 100644 --- a/chromium/components/download/internal/common/download_item_impl.cc +++ b/chromium/components/download/internal/common/download_item_impl.cc @@ -412,7 +412,7 @@ DownloadItemImpl::DownloadItemImpl( const base::FilePath& path, const GURL& url, const std::string& mime_type, - std::unique_ptr<DownloadRequestHandleInterface> request_handle) + DownloadJob::CancelRequestCallback cancel_request_callback) : request_info_(url), guid_(base::GenerateGUID()), download_id_(download_id), @@ -423,9 +423,9 @@ DownloadItemImpl::DownloadItemImpl( delegate_(delegate), destination_info_(path, path, 0, false, std::string(), base::Time()), is_updating_observers_(false) { - job_ = DownloadJobFactory::CreateJob(this, std::move(request_handle), - DownloadCreateInfo(), true, nullptr, - nullptr, nullptr); + job_ = DownloadJobFactory::CreateJob( + this, std::move(cancel_request_callback), DownloadCreateInfo(), true, + URLLoaderFactoryProvider::GetNullPtr(), nullptr); delegate_->Attach(); Init(true /* actively downloading */, TYPE_SAVE_PAGE_AS); } @@ -1415,11 +1415,10 @@ void DownloadItemImpl::Init(bool active, // We're starting the download. void DownloadItemImpl::Start( std::unique_ptr<DownloadFile> file, - std::unique_ptr<DownloadRequestHandleInterface> req_handle, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadCreateInfo& new_create_info, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter) { + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK(!download_file_); DVLOG(20) << __func__ << "() this=" << DebugString(true); @@ -1427,8 +1426,8 @@ void DownloadItemImpl::Start( download_file_ = std::move(file); job_ = DownloadJobFactory::CreateJob( - this, std::move(req_handle), new_create_info, false, - std::move(url_loader_factory_getter), url_request_context_getter, + this, std::move(cancel_request_callback), new_create_info, false, + std::move(url_loader_factory_provider), delegate_ ? delegate_->GetServiceManagerConnector() : nullptr); if (job_->IsParallelizable()) { RecordParallelizableDownloadCount(START_COUNT, IsParallelDownloadEnabled()); @@ -1788,12 +1787,10 @@ void DownloadItemImpl::OnDownloadCompleting() { } #endif // defined(OS_ANDROID) - std::unique_ptr<service_manager::Connector> new_connector; - service_manager::Connector* connector = - delegate_->GetServiceManagerConnector(); - if (connector) - new_connector = connector->Clone(); - + mojo::PendingRemote<quarantine::mojom::Quarantine> quarantine; + auto quarantine_callback = delegate_->GetQuarantineConnectionCallback(); + if (quarantine_callback) + quarantine_callback.Run(quarantine.InitWithNewPipeAndPassReceiver()); GetDownloadTaskRunner()->PostTask( FROM_HERE, base::BindOnce(&DownloadFile::RenameAndAnnotate, @@ -1802,7 +1799,7 @@ void DownloadItemImpl::OnDownloadCompleting() { delegate_->GetApplicationClientIdForFileScanning(), delegate_->IsOffTheRecord() ? GURL() : GetURL(), delegate_->IsOffTheRecord() ? GURL() : GetReferrerUrl(), - std::move(new_connector), std::move(callback))); + std::move(quarantine), std::move(callback))); } void DownloadItemImpl::OnDownloadRenamedToFinalName( @@ -2036,14 +2033,6 @@ void DownloadItemImpl::InterruptWithPartialState( job_->Cancel(false); if (IsCancellation(reason)) { - if (IsDangerous()) { - RecordDangerousDownloadDiscard( - reason == DOWNLOAD_INTERRUPT_REASON_USER_CANCELED - ? DOWNLOAD_DISCARD_DUE_TO_USER_ACTION - : DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN, - GetDangerType(), GetTargetFilePath()); - } - RecordDownloadCountWithSource(CANCELLED_COUNT, download_source_); if (job_ && job_->IsParallelizable()) { RecordParallelizableDownloadCount(CANCELLED_COUNT, @@ -2056,8 +2045,7 @@ void DownloadItemImpl::InterruptWithPartialState( RecordDownloadInterrupted(reason, GetReceivedBytes(), total_bytes_, job_ && job_->IsParallelizable(), - IsParallelDownloadEnabled(), download_source_, - received_bytes_at_length_mismatch_ > 0); + IsParallelDownloadEnabled(), download_source_); base::TimeDelta time_since_start = base::Time::Now() - GetStartTime(); int resulting_file_size = GetReceivedBytes(); @@ -2300,19 +2288,6 @@ void DownloadItemImpl::SetDangerType(DownloadDangerType danger_type) { TRACE_EVENT_SCOPE_THREAD, "danger_type", GetDownloadDangerNames(danger_type).c_str()); } - // Only record the Malicious UMA stat if it's going from {not malicious} -> - // {malicious}. - 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_WHITELISTED_BY_POLICY) && - (danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST || - danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_URL || - danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT || - danger_type == DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED)) { - RecordMaliciousDownloadClassified(danger_type); - } danger_type_ = danger_type; } @@ -2420,16 +2395,23 @@ void DownloadItemImpl::ResumeInterruptedDownload( download_params->set_hash_of_partial_file(GetHash()); download_params->set_hash_state(std::move(hash_state_)); download_params->set_guid(guid_); - if (!HasStrongValidators() && download_params->offset() > 0 && + if (!HasStrongValidators() && base::FeatureList::IsEnabled( features::kAllowDownloadResumptionWithoutStrongValidators)) { - download_params->set_use_if_range(false); - download_params->set_file_offset(download_params->offset()); int64_t validation_length = GetDownloadValidationLengthConfig(); - download_params->set_offset(download_params->offset() > validation_length - ? download_params->offset() - - validation_length - : 0); + if (download_params->offset() > validation_length) { + // There is enough data for validation, set the file_offset so + // DownloadFileImpl will validate the data between offset to + // file_offset. + download_params->set_use_if_range(false); + download_params->set_file_offset(download_params->offset()); + download_params->set_offset(download_params->offset() - + validation_length); + } else { + // There is not enough data for validation, simply overwrites the + // existing data from the beginning. + download_params->set_offset(0); + } } // TODO(xingliu): Read |fetch_error_body| and |request_headers_| from the @@ -2449,13 +2431,6 @@ void DownloadItemImpl::ResumeInterruptedDownload( download_params->set_referrer_policy(net::URLRequest::NEVER_CLEAR_REFERRER); download_params->set_follow_cross_origin_redirects(false); - // If the interruption was caused by content length mismatch, ignore it during - // resumption. - if (last_reason_ == - DOWNLOAD_INTERRUPT_REASON_SERVER_CONTENT_LENGTH_MISMATCH) { - download_params->set_ignore_content_length_mismatch(true); - } - TransitionTo(RESUMING_INTERNAL); RecordDownloadCountWithSource(source == ResumptionRequestSource::USER ? MANUAL_RESUMPTION_COUNT diff --git a/chromium/components/download/internal/common/download_item_impl_delegate.cc b/chromium/components/download/internal/common/download_item_impl_delegate.cc index f5dff20abdc..11a8f2691cc 100644 --- a/chromium/components/download/internal/common/download_item_impl_delegate.cc +++ b/chromium/components/download/internal/common/download_item_impl_delegate.cc @@ -108,4 +108,9 @@ DownloadItemImplDelegate::GetServiceManagerConnector() { return nullptr; } +QuarantineConnectionCallback +DownloadItemImplDelegate::GetQuarantineConnectionCallback() { + return base::NullCallback(); +} + } // namespace download 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 93a09d6a13e..7bb6147522d 100644 --- a/chromium/components/download/internal/common/download_item_impl_unittest.cc +++ b/chromium/components/download/internal/common/download_item_impl_unittest.cc @@ -19,14 +19,13 @@ #include "base/files/file_util.h" #include "base/memory/ptr_util.h" #include "base/test/metrics/histogram_tester.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "base/threading/thread.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_destination_observer.h" #include "components/download/public/common/download_file_factory.h" #include "components/download/public/common/download_interrupt_reasons.h" #include "components/download/public/common/download_item_impl_delegate.h" -#include "components/download/public/common/download_request_handle_interface.h" #include "components/download/public/common/download_url_parameters.h" #include "components/download/public/common/mock_download_file.h" #include "components/download/public/common/mock_download_item.h" @@ -102,13 +101,6 @@ class MockDelegate : public DownloadItemImplDelegate { } }; -class MockRequestHandle : public DownloadRequestHandleInterface { - public: - MOCK_METHOD0(PauseRequest, void()); - MOCK_METHOD0(ResumeRequest, void()); - MOCK_METHOD1(CancelRequest, void(bool)); -}; - class TestDownloadItemObserver : public DownloadItem::Observer { public: explicit TestDownloadItemObserver(DownloadItem* item) @@ -240,8 +232,8 @@ class DownloadItemTest : public testing::Test { public: DownloadItemTest() : task_environment_( - base::test::ScopedTaskEnvironment::MainThreadType::UI, - base::test::ScopedTaskEnvironment::ThreadPoolExecutionMode::QUEUED), + base::test::TaskEnvironment::MainThreadType::UI, + base::test::TaskEnvironment::ThreadPoolExecutionMode::QUEUED), next_download_id_(DownloadItem::kInvalidId + 1) { create_info_.reset(new DownloadCreateInfo()); create_info_->save_info = @@ -297,10 +289,8 @@ class DownloadItemTest : public testing::Test { .WillRepeatedly(ReturnRefOfCopy(base::FilePath())); } - std::unique_ptr<MockRequestHandle> request_handle = - std::make_unique<NiceMock<MockRequestHandle>>(); - item->Start(std::move(download_file), std::move(request_handle), - *create_info_, nullptr, nullptr); + item->Start(std::move(download_file), base::DoNothing(), *create_info_, + URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); // So that we don't have a function writing to a stack variable @@ -383,13 +373,18 @@ class DownloadItemTest : public testing::Test { DownloadCreateInfo* create_info() { return create_info_.get(); } - base::test::ScopedTaskEnvironment task_environment_; + void CancelRequest(bool user_cancel) { canceled_ = true; } + + bool canceled() { return canceled_; } + + base::test::TaskEnvironment task_environment_; private: StrictMock<MockDelegate> mock_delegate_; std::map<DownloadItem*, std::unique_ptr<DownloadItem>> allocated_downloads_; std::unique_ptr<DownloadCreateInfo> create_info_; uint32_t next_download_id_ = DownloadItem::kInvalidId + 1; + bool canceled_ = false; }; // Tests to ensure calls that change a DownloadItem generate an update @@ -577,13 +572,11 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) { TestDownloadItemObserver observer(item); MockDownloadFile* mock_download_file(new MockDownloadFile); std::unique_ptr<DownloadFile> download_file(mock_download_file); - std::unique_ptr<DownloadRequestHandleInterface> request_handle( - new NiceMock<MockRequestHandle>); EXPECT_CALL(*mock_download_file, Initialize(_, _, _, _)); EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _)); - item->Start(std::move(download_file), std::move(request_handle), - *create_info(), nullptr, nullptr); + item->Start(std::move(download_file), base::DoNothing(), *create_info(), + URLLoaderFactoryProvider::GetNullPtr()); item->Pause(); ASSERT_TRUE(observer.CheckAndResetDownloadUpdated()); @@ -804,7 +797,6 @@ TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) { TestDownloadItemObserver observer(item); MockDownloadFile* mock_download_file_ref = nullptr; std::unique_ptr<MockDownloadFile> mock_download_file; - std::unique_ptr<MockRequestHandle> mock_request_handle; DownloadItemImplDelegate::DownloadTargetCallback callback; EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _)) @@ -823,15 +815,14 @@ TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) { mock_download_file = std::make_unique<NiceMock<MockDownloadFile>>(); mock_download_file_ref = mock_download_file.get(); - mock_request_handle = std::make_unique<NiceMock<MockRequestHandle>>(); ON_CALL(*mock_download_file_ref, FullPath()) .WillByDefault(ReturnRefOfCopy(base::FilePath())); // Copied key parts of DoIntermediateRename & CallDownloadItemStart // to allow for holding onto the request handle. - item->Start(std::move(mock_download_file), std::move(mock_request_handle), - *create_info(), nullptr, nullptr); + item->Start(std::move(mock_download_file), base::DoNothing(), + *create_info(), URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); base::FilePath target_path(kDummyTargetPath); @@ -1161,11 +1152,9 @@ TEST_F(DownloadItemTest, Start) { std::unique_ptr<DownloadFile> download_file(mock_download_file); DownloadItemImpl* item = CreateDownloadItem(); EXPECT_CALL(*mock_download_file, Initialize(_, _, _, _)); - std::unique_ptr<DownloadRequestHandleInterface> request_handle( - new NiceMock<MockRequestHandle>); EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _)); - item->Start(std::move(download_file), std::move(request_handle), - *create_info(), nullptr, nullptr); + item->Start(std::move(download_file), base::DoNothing(), *create_info(), + URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS); @@ -1176,12 +1165,9 @@ TEST_F(DownloadItemTest, Start) { TEST_F(DownloadItemTest, InitDownloadFileFails) { DownloadItemImpl* item = CreateDownloadItem(); std::unique_ptr<MockDownloadFile> file = std::make_unique<MockDownloadFile>(); - std::unique_ptr<MockRequestHandle> request_handle = - std::make_unique<MockRequestHandle>(); base::HistogramTester histogram_tester; EXPECT_CALL(*file, Cancel()); - EXPECT_CALL(*request_handle, CancelRequest(_)); EXPECT_CALL(*file, Initialize(_, _, _, _)) .WillOnce(ScheduleCallbackWithParams( DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED, 0, @@ -1191,8 +1177,10 @@ TEST_F(DownloadItemTest, InitDownloadFileFails) { EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _)) .WillOnce(SaveArg<1>(&download_target_callback)); - item->Start(std::move(file), std::move(request_handle), *create_info(), - nullptr, nullptr); + item->Start( + std::move(file), + base::Bind(&DownloadItemTest::CancelRequest, base::Unretained(this)), + *create_info(), URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); download_target_callback.Run(base::FilePath(kDummyTargetPath), @@ -1207,6 +1195,7 @@ TEST_F(DownloadItemTest, InitDownloadFileFails) { item->GetLastReason()); EXPECT_FALSE(item->GetTargetFilePath().empty()); EXPECT_TRUE(item->GetFullPath().empty()); + EXPECT_TRUE(canceled()); histogram_tester.ExpectBucketCount( "Download.InterruptedReason", ToHistogramSample<DownloadInterruptReason>( @@ -1224,12 +1213,11 @@ TEST_F(DownloadItemTest, StartFailedDownload) { // DownloadFile and DownloadRequestHandleInterface objects aren't created for // failed downloads. std::unique_ptr<DownloadFile> null_download_file; - std::unique_ptr<DownloadRequestHandleInterface> null_request_handle; DownloadItemImplDelegate::DownloadTargetCallback download_target_callback; EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _)) .WillOnce(SaveArg<1>(&download_target_callback)); - item->Start(std::move(null_download_file), std::move(null_request_handle), - *create_info(), nullptr, nullptr); + item->Start(std::move(null_download_file), base::DoNothing(), *create_info(), + URLLoaderFactoryProvider::GetNullPtr()); EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState()); task_environment_.RunUntilIdle(); @@ -2266,8 +2254,7 @@ class DownloadItemDestinationUpdateRaceTest public: DownloadItemDestinationUpdateRaceTest() : item_(CreateDownloadItem()), - file_(new ::testing::StrictMock<MockDownloadFile>()), - request_handle_(new ::testing::StrictMock<MockRequestHandle>()) { + file_(new ::testing::StrictMock<MockDownloadFile>()) { DCHECK_EQ(GetParam().size(), static_cast<unsigned>(kEventCount)); } @@ -2298,7 +2285,6 @@ class DownloadItemDestinationUpdateRaceTest DownloadItemImpl* item_; std::unique_ptr<MockDownloadFile> file_; - std::unique_ptr<MockRequestHandle> request_handle_; base::queue<base::Closure> successful_update_events_; base::queue<base::Closure> failing_update_events_; @@ -2320,13 +2306,14 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) { // Expect that the download file and the request will be cancelled as a // result. EXPECT_CALL(*file_, Cancel()); - EXPECT_CALL(*request_handle_, CancelRequest(_)); DownloadFile::InitializeCallback initialize_callback; EXPECT_CALL(*file_, Initialize(_, _, _, _)) .WillOnce(SaveArg<0>(&initialize_callback)); - item_->Start(std::move(file_), std::move(request_handle_), *create_info(), - nullptr, nullptr); + item_->Start( + std::move(file_), + base::Bind(&DownloadItemTest::CancelRequest, base::Unretained(this)), + *create_info(), URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); base::WeakPtr<DownloadDestinationObserver> destination_observer = @@ -2351,6 +2338,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) { DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, base::FilePath(), DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_EQ(DownloadItem::CANCELLED, item_->GetState()); + EXPECT_TRUE(canceled()); task_environment_.RunUntilIdle(); } @@ -2359,7 +2347,6 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) { // Expect that the download file and the request will be cancelled as a // result. EXPECT_CALL(*file_, Cancel()); - EXPECT_CALL(*request_handle_, CancelRequest(_)); // Intermediate rename loop is not used immediately, but let's set up the // DownloadFile expectations since we are about to transfer its ownership to @@ -2372,8 +2359,10 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) { EXPECT_CALL(*file_, Initialize(_, _, _, _)) .WillOnce(SaveArg<0>(&initialize_callback)); - item_->Start(std::move(file_), std::move(request_handle_), *create_info(), - nullptr, nullptr); + item_->Start( + std::move(file_), + base::Bind(&DownloadItemTest::CancelRequest, base::Unretained(this)), + *create_info(), URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); base::WeakPtr<DownloadDestinationObserver> destination_observer = @@ -2409,6 +2398,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) { task_environment_.RunUntilIdle(); EXPECT_EQ(DownloadItem::INTERRUPTED, item_->GetState()); + EXPECT_TRUE(canceled()); } // Run through the DII workflow. Download file initialization, target @@ -2419,7 +2409,6 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) { // DownloadFile will Detach()). It depends on the list of observations that // are given to us. EXPECT_CALL(*file_, Cancel()).Times(::testing::AnyNumber()); - EXPECT_CALL(*request_handle_, CancelRequest(_)).Times(::testing::AnyNumber()); EXPECT_CALL(*file_, Detach()).Times(::testing::AnyNumber()); EXPECT_CALL(*file_, FullPath()) @@ -2436,8 +2425,8 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) { EXPECT_CALL(*file_, Initialize(_, _, _, _)) .WillOnce(SaveArg<0>(&initialize_callback)); - item_->Start(std::move(file_), std::move(request_handle_), *create_info(), - nullptr, nullptr); + item_->Start(std::move(file_), base::DoNothing(), *create_info(), + URLLoaderFactoryProvider::GetNullPtr()); task_environment_.RunUntilIdle(); base::WeakPtr<DownloadDestinationObserver> destination_observer = diff --git a/chromium/components/download/internal/common/download_job.cc b/chromium/components/download/internal/common/download_job.cc index f69b10805a7..04861abe4a3 100644 --- a/chromium/components/download/internal/common/download_job.cc +++ b/chromium/components/download/internal/common/download_job.cc @@ -11,18 +11,17 @@ namespace download { -DownloadJob::DownloadJob( - DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle) +DownloadJob::DownloadJob(DownloadItem* download_item, + CancelRequestCallback cancel_request_callback) : download_item_(download_item), - request_handle_(std::move(request_handle)), + cancel_request_callback_(std::move(cancel_request_callback)), is_paused_(false) {} DownloadJob::~DownloadJob() = default; void DownloadJob::Cancel(bool user_cancel) { - if (request_handle_) - request_handle_->CancelRequest(user_cancel); + if (cancel_request_callback_) + std::move(cancel_request_callback_).Run(user_cancel); } void DownloadJob::Pause() { @@ -36,8 +35,6 @@ void DownloadJob::Pause() { // Safe because we control download file lifetime. base::Unretained(download_file))); } - if (request_handle_) - request_handle_->PauseRequest(); } void DownloadJob::Resume(bool resume_request) { @@ -53,9 +50,6 @@ void DownloadJob::Resume(bool resume_request) { // Safe because we control download file lifetime. base::Unretained(download_file))); } - - if (request_handle_) - request_handle_->ResumeRequest(); } void DownloadJob::Start(DownloadFile* download_file_, diff --git a/chromium/components/download/internal/common/download_job_factory.cc b/chromium/components/download/internal/common/download_job_factory.cc index db41162941f..cd6274cac86 100644 --- a/chromium/components/download/internal/common/download_job_factory.cc +++ b/chromium/components/download/internal/common/download_job_factory.cc @@ -6,6 +6,7 @@ #include <memory> +#include "base/memory/weak_ptr.h" #include "components/download/internal/common/download_job_impl.h" #include "components/download/internal/common/parallel_download_job.h" #include "components/download/internal/common/parallel_download_utils.h" @@ -13,7 +14,6 @@ #include "components/download/public/common/download_features.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/download_stats.h" -#include "components/download/public/common/download_url_loader_factory_getter.h" #include "net/url_request/url_request_context_getter.h" namespace download { @@ -160,30 +160,28 @@ bool IsParallelizableDownload(const DownloadCreateInfo& create_info, // static std::unique_ptr<DownloadJob> DownloadJobFactory::CreateJob( DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> req_handle, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadCreateInfo& create_info, bool is_save_package_download, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, service_manager::Connector* connector) { if (is_save_package_download) { - return std::make_unique<SavePackageDownloadJob>(download_item, - std::move(req_handle)); + return std::make_unique<SavePackageDownloadJob>( + download_item, std::move(cancel_request_callback)); } bool is_parallelizable = IsParallelizableDownload(create_info, download_item); // Build parallel download job. if (IsParallelDownloadEnabled() && is_parallelizable) { return std::make_unique<ParallelDownloadJob>( - download_item, std::move(req_handle), create_info, - std::move(url_loader_factory_getter), url_request_context_getter, - connector); + download_item, std::move(cancel_request_callback), create_info, + std::move(url_loader_factory_provider), connector); } // An ordinary download job. - return std::make_unique<DownloadJobImpl>(download_item, std::move(req_handle), - is_parallelizable); + return std::make_unique<DownloadJobImpl>( + download_item, std::move(cancel_request_callback), is_parallelizable); } } // namespace download diff --git a/chromium/components/download/internal/common/download_job_impl.cc b/chromium/components/download/internal/common/download_job_impl.cc index e653a87c652..cf53b0c9428 100644 --- a/chromium/components/download/internal/common/download_job_impl.cc +++ b/chromium/components/download/internal/common/download_job_impl.cc @@ -6,11 +6,10 @@ namespace download { -DownloadJobImpl::DownloadJobImpl( - DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle, - bool is_parallizable) - : DownloadJob(download_item, std::move(request_handle)), +DownloadJobImpl::DownloadJobImpl(DownloadItem* download_item, + CancelRequestCallback cancel_request_callback, + bool is_parallizable) + : DownloadJob(download_item, std::move(cancel_request_callback)), is_parallizable_(is_parallizable) {} DownloadJobImpl::~DownloadJobImpl() = default; diff --git a/chromium/components/download/internal/common/download_job_impl.h b/chromium/components/download/internal/common/download_job_impl.h index 5be902d2755..a78ead6b538 100644 --- a/chromium/components/download/internal/common/download_job_impl.h +++ b/chromium/components/download/internal/common/download_job_impl.h @@ -14,10 +14,9 @@ class DownloadItem; class COMPONENTS_DOWNLOAD_EXPORT DownloadJobImpl : public DownloadJob { public: - DownloadJobImpl( - DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle, - bool is_parallizable); + DownloadJobImpl(DownloadItem* download_item, + CancelRequestCallback cancel_request_callback, + bool is_parallizable); ~DownloadJobImpl() override; // DownloadJob implementation. diff --git a/chromium/components/download/internal/common/download_path_reservation_tracker_unittest.cc b/chromium/components/download/internal/common/download_path_reservation_tracker_unittest.cc index 1b6ccde7434..5c4e3e169aa 100644 --- a/chromium/components/download/internal/common/download_path_reservation_tracker_unittest.cc +++ b/chromium/components/download/internal/common/download_path_reservation_tracker_unittest.cc @@ -16,7 +16,7 @@ #include "base/run_loop.h" #include "base/strings/stringprintf.h" #include "base/test/scoped_feature_list.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "base/test/test_file_util.h" #include "base/time/time.h" #include "build/build_config.h" @@ -75,7 +75,7 @@ class DownloadPathReservationTrackerTest : public testing::Test { base::ScopedTempDir test_download_dir_; base::FilePath default_download_path_; base::FilePath fallback_directory_; - base::test::ScopedTaskEnvironment scoped_task_environment_; + base::test::TaskEnvironment task_environment_; private: void TestReservedPathCallback(base::FilePath* return_path, @@ -138,7 +138,7 @@ bool DownloadPathReservationTrackerTest::IsPathInUse( } void DownloadPathReservationTrackerTest::RunUntilIdle() { - scoped_task_environment_.RunUntilIdle(); + task_environment_.RunUntilIdle(); } void DownloadPathReservationTrackerTest::CallGetReservedPath( @@ -157,7 +157,7 @@ void DownloadPathReservationTrackerTest::CallGetReservedPath( create_directory, conflict_action, base::Bind(&DownloadPathReservationTrackerTest::TestReservedPathCallback, weak_ptr_factory.GetWeakPtr(), return_path, return_result)); - scoped_task_environment_.RunUntilIdle(); + task_environment_.RunUntilIdle(); } void DownloadPathReservationTrackerTest::TestReservedPathCallback( diff --git a/chromium/components/download/internal/common/download_response_handler.cc b/chromium/components/download/internal/common/download_response_handler.cc index 1d5e3942653..e2e30819117 100644 --- a/chromium/components/download/internal/common/download_response_handler.cc +++ b/chromium/components/download/internal/common/download_response_handler.cc @@ -58,7 +58,6 @@ DownloadResponseHandler::DownloadResponseHandler( const DownloadUrlParameters::RequestHeadersType& request_headers, const std::string& request_origin, DownloadSource download_source, - bool ignore_content_length_mismatch, std::vector<GURL> url_chain, bool is_background_mode) : delegate_(delegate), @@ -75,7 +74,7 @@ DownloadResponseHandler::DownloadResponseHandler( request_headers_(request_headers), request_origin_(request_origin), download_source_(download_source), - ignore_content_length_mismatch_(ignore_content_length_mismatch), + has_strong_validators_(false), is_partial_request_(save_info_->offset > 0), completed_(false), abort_reason_(DOWNLOAD_INTERRUPT_REASON_NONE), @@ -90,24 +89,16 @@ DownloadResponseHandler::DownloadResponseHandler( DownloadResponseHandler::~DownloadResponseHandler() = default; void DownloadResponseHandler::OnReceiveResponse( - const network::ResourceResponseHead& head) { + network::mojom::URLResponseHeadPtr head) { create_info_ = CreateDownloadCreateInfo(head); - cert_status_ = head.cert_status; + cert_status_ = head->cert_status; // TODO(xingliu): Do not use http cache. // Sets page transition type correctly and call // |RecordDownloadSourcePageTransitionType| here. - if (head.headers) { - // ERR_CONTENT_LENGTH_MISMATCH can be caused by 1 of the following reasons: - // 1. Server or proxy closes the connection too early. - // 2. The content-length header is wrong. - // If the download has strong validators, we can interrupt the download - // and let it resume automatically. Otherwise, resuming the download will - // cause it to restart and the download may never complete if the error was - // caused by reason 2. As a result, downloads without strong validators are - // treated as completed here. - ignore_content_length_mismatch_ |= !head.headers->HasStrongValidators(); - RecordDownloadHttpResponseCode(head.headers->response_code(), + if (head->headers) { + has_strong_validators_ = head->headers->HasStrongValidators(); + RecordDownloadHttpResponseCode(head->headers->response_code(), is_background_mode_); RecordDownloadContentDisposition(create_info_->content_disposition); } @@ -166,7 +157,7 @@ DownloadResponseHandler::CreateDownloadCreateInfo( void DownloadResponseHandler::OnReceiveRedirect( const net::RedirectInfo& redirect_info, - const network::ResourceResponseHead& head) { + network::mojom::URLResponseHeadPtr head) { if (!follow_cross_origin_redirects_ && !first_origin_.IsSameOriginWith( url::Origin::Create(redirect_info.new_url))) { @@ -234,9 +225,8 @@ void DownloadResponseHandler::OnComplete( completed_ = true; DownloadInterruptReason reason = HandleRequestCompletionStatus( - static_cast<net::Error>(status.error_code), - ignore_content_length_mismatch_, cert_status_, is_partial_request_, - abort_reason_); + static_cast<net::Error>(status.error_code), has_strong_validators_, + cert_status_, is_partial_request_, abort_reason_); if (client_ptr_) { client_ptr_->OnStreamCompleted( diff --git a/chromium/components/download/internal/common/download_stats.cc b/chromium/components/download/internal/common/download_stats.cc index 1060096e1d8..d12e8625626 100644 --- a/chromium/components/download/internal/common/download_stats.cc +++ b/chromium/components/download/internal/common/download_stats.cc @@ -613,19 +613,13 @@ void RecordDownloadInterrupted(DownloadInterruptReason reason, int64_t total, bool is_parallelizable, bool is_parallel_download_enabled, - DownloadSource download_source, - bool post_content_length_mismatch) { + DownloadSource download_source) { RecordDownloadCountWithSource(INTERRUPTED_COUNT, download_source); if (is_parallelizable) { RecordParallelizableDownloadCount(INTERRUPTED_COUNT, is_parallel_download_enabled); } - if (post_content_length_mismatch) { - base::UmaHistogramSparse( - "Download.ResumptionAfterContentLengthMismatch.Reason", reason); - } - std::vector<base::HistogramBase::Sample> samples = base::CustomHistogram::ArrayToCustomEnumRanges(kAllInterruptReasonCodes); UMA_HISTOGRAM_CUSTOM_ENUMERATION("Download.InterruptedReason", reason, @@ -674,11 +668,6 @@ void RecordDownloadInterrupted(DownloadInterruptReason reason, } } -void RecordMaliciousDownloadClassified(DownloadDangerType danger_type) { - UMA_HISTOGRAM_ENUMERATION("Download.MaliciousDownloadClassified", danger_type, - DOWNLOAD_DANGER_TYPE_MAX); -} - void RecordDangerousDownloadAccept(DownloadDangerType danger_type, const base::FilePath& file_path) { UMA_HISTOGRAM_ENUMERATION("Download.DangerousDownloadValidated", danger_type, @@ -690,31 +679,6 @@ void RecordDangerousDownloadAccept(DownloadDangerType danger_type, } } -void RecordDangerousDownloadDiscard(DownloadDiscardReason reason, - DownloadDangerType danger_type, - const base::FilePath& file_path) { - switch (reason) { - case DOWNLOAD_DISCARD_DUE_TO_USER_ACTION: - UMA_HISTOGRAM_ENUMERATION("Download.UserDiscard", danger_type, - DOWNLOAD_DANGER_TYPE_MAX); - if (danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE) { - base::UmaHistogramSparse("Download.DangerousFile.UserDiscard", - GetDangerousFileType(file_path)); - } - break; - case DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN: - UMA_HISTOGRAM_ENUMERATION("Download.Discard", danger_type, - DOWNLOAD_DANGER_TYPE_MAX); - if (danger_type == DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE) { - base::UmaHistogramSparse("Download.DangerousFile.Discard", - GetDangerousFileType(file_path)); - } - break; - default: - NOTREACHED(); - } -} - namespace { int GetMimeTypeMatch(const std::string& mime_type_string, @@ -1235,14 +1199,6 @@ void RecordDownloadValidationMetrics(DownloadMetricsCallsite callsite, DownloadContent::MAX); } -void RecordDownloadConnectionSecurity(const GURL& download_url, - const std::vector<GURL>& url_chain) { - UMA_HISTOGRAM_ENUMERATION( - "Download.TargetConnectionSecurity", - CheckDownloadConnectionSecurity(download_url, url_chain), - DOWNLOAD_CONNECTION_SECURITY_MAX); -} - void RecordDownloadContentTypeSecurity( const GURL& download_url, const std::vector<GURL>& url_chain, diff --git a/chromium/components/download/internal/common/download_ukm_helper_unittest.cc b/chromium/components/download/internal/common/download_ukm_helper_unittest.cc index a3c3fed2de8..a2f75e7f13a 100644 --- a/chromium/components/download/internal/common/download_ukm_helper_unittest.cc +++ b/chromium/components/download/internal/common/download_ukm_helper_unittest.cc @@ -5,7 +5,7 @@ #include "components/download/public/common/download_ukm_helper.h" #include "base/run_loop.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "components/ukm/test_ukm_recorder.h" #include "services/metrics/public/cpp/ukm_builders.h" #include "testing/gmock/include/gmock/gmock.h" @@ -45,7 +45,7 @@ class DownloadUkmHelperTest : public testing::Test { protected: int download_id_; - base::test::ScopedTaskEnvironment scoped_task_environment_; + base::test::TaskEnvironment task_environment_; std::unique_ptr<ukm::TestAutoSetUkmRecorder> test_recorder_; DISALLOW_COPY_AND_ASSIGN(DownloadUkmHelperTest); diff --git a/chromium/components/download/internal/common/download_url_loader_factory_getter.cc b/chromium/components/download/internal/common/download_url_loader_factory_getter.cc deleted file mode 100644 index 4f3b2823b7d..00000000000 --- a/chromium/components/download/internal/common/download_url_loader_factory_getter.cc +++ /dev/null @@ -1,23 +0,0 @@ -// 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/download_url_loader_factory_getter.h" - -#include "components/download/public/common/download_task_runner.h" - -namespace download { - -DownloadURLLoaderFactoryGetter::DownloadURLLoaderFactoryGetter() = default; - -DownloadURLLoaderFactoryGetter::~DownloadURLLoaderFactoryGetter() = default; - -void DownloadURLLoaderFactoryGetter::DeleteOnCorrectThread() const { - if (GetIOTaskRunner() && !GetIOTaskRunner()->BelongsToCurrentThread()) { - GetIOTaskRunner()->DeleteSoon(FROM_HERE, this); - return; - } - delete this; -} - -} // namespace download diff --git a/chromium/components/download/internal/common/download_url_loader_factory_getter_impl.cc b/chromium/components/download/internal/common/download_url_loader_factory_getter_impl.cc deleted file mode 100644 index 83c5b9390d2..00000000000 --- a/chromium/components/download/internal/common/download_url_loader_factory_getter_impl.cc +++ /dev/null @@ -1,25 +0,0 @@ -// 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/download_url_loader_factory_getter_impl.h" - -namespace download { - -DownloadURLLoaderFactoryGetterImpl::DownloadURLLoaderFactoryGetterImpl( - std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory) - : url_loader_factory_info_(std::move(url_loader_factory)) {} - -DownloadURLLoaderFactoryGetterImpl::~DownloadURLLoaderFactoryGetterImpl() = - default; - -scoped_refptr<network::SharedURLLoaderFactory> -DownloadURLLoaderFactoryGetterImpl::GetURLLoaderFactory() { - if (!url_loader_factory_) { - url_loader_factory_ = network::SharedURLLoaderFactory::Create( - std::move(url_loader_factory_info_)); - } - return url_loader_factory_; -} - -} // namespace download diff --git a/chromium/components/download/internal/common/download_utils.cc b/chromium/components/download/internal/common/download_utils.cc index b1c816f2dee..d9bcc7b5ba0 100644 --- a/chromium/components/download/internal/common/download_utils.cc +++ b/chromium/components/download/internal/common/download_utils.cc @@ -53,12 +53,23 @@ const uint32_t DownloadItem::kInvalidId = 0; DownloadInterruptReason HandleRequestCompletionStatus( net::Error error_code, - bool ignore_content_length_mismatch, + bool has_strong_validators, net::CertStatus cert_status, bool is_partial_request, DownloadInterruptReason abort_reason) { + // ERR_CONTENT_LENGTH_MISMATCH can be caused by 1 of the following reasons: + // 1. Server or proxy closes the connection too early. + // 2. The content-length header is wrong. + // If the download has strong validators, we can interrupt the download + // and let it resume automatically. Otherwise, resuming the download will + // cause it to restart and the download may never complete if the error was + // caused by reason 2. As a result, downloads without strong validators are + // treated as completed here. + // TODO(qinmin): check the metrics from downloads with strong validators, + // and decide whether we should interrupt downloads without strong validators + // rather than complete them. if (error_code == net::ERR_CONTENT_LENGTH_MISMATCH && - ignore_content_length_mismatch) { + !has_strong_validators) { error_code = net::OK; RecordDownloadCount(COMPLETED_WITH_CONTENT_LENGTH_MISMATCH_COUNT); } @@ -253,7 +264,6 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest( request->site_for_cookies = params->url(); request->referrer = params->referrer(); request->referrer_policy = params->referrer_policy(); - request->allow_download = true; request->is_main_frame = true; // Downloads should be treated as navigations from Fetch spec perspective. diff --git a/chromium/components/download/internal/common/download_worker.cc b/chromium/components/download/internal/common/download_worker.cc index cd55965b6a7..b1e9792c36c 100644 --- a/chromium/components/download/internal/common/download_worker.cc +++ b/chromium/components/download/internal/common/download_worker.cc @@ -9,12 +9,9 @@ #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/url_download_handler_factory.h" -#include "net/url_request/url_request_context_getter.h" -#include "services/network/public/cpp/features.h" #include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/service_manager/public/cpp/connector.h" @@ -52,16 +49,16 @@ class CompletedInputStream : public InputStream { void CreateUrlDownloadHandler( std::unique_ptr<DownloadUrlParameters> params, base::WeakPtr<UrlDownloadHandler::Delegate> delegate, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + URLLoaderFactoryProvider* url_loader_factory_provider, const URLSecurityPolicy& url_security_policy, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, std::unique_ptr<service_manager::Connector> connector, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) { auto downloader = UrlDownloadHandlerFactory::Create( - std::move(params), delegate, std::move(url_loader_factory_getter), - url_security_policy, std::move(url_request_context_getter), - std::move(connector), task_runner); + std::move(params), delegate, + url_loader_factory_provider + ? url_loader_factory_provider->GetURLLoaderFactory() + : nullptr, + url_security_policy, std::move(connector), task_runner); task_runner->PostTask( FROM_HERE, base::BindOnce(&UrlDownloadHandler::Delegate::OnUrlDownloadHandlerCreated, @@ -78,7 +75,6 @@ DownloadWorker::DownloadWorker(DownloadWorker::Delegate* delegate, length_(length), is_paused_(false), is_canceled_(false), - is_user_cancel_(false), url_download_handler_(nullptr, base::OnTaskRunnerDeleter(nullptr)) { DCHECK(delegate_); } @@ -87,44 +83,38 @@ DownloadWorker::~DownloadWorker() = default; void DownloadWorker::SendRequest( std::unique_ptr<DownloadUrlParameters> params, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, + URLLoaderFactoryProvider* url_loader_factory_provider, service_manager::Connector* connector) { GetIOTaskRunner()->PostTask( FROM_HERE, base::BindOnce(&CreateUrlDownloadHandler, std::move(params), weak_factory_.GetWeakPtr(), - std::move(url_loader_factory_getter), + // This is safe because URLLoaderFactoryProvider + // deleter is called on the same task sequence. + base::Unretained(url_loader_factory_provider), base::BindRepeating(&IsURLSafe), - std::move(url_request_context_getter), connector ? connector->Clone() : nullptr, base::ThreadTaskRunnerHandle::Get())); } void DownloadWorker::Pause() { is_paused_ = true; - if (request_handle_) - request_handle_->PauseRequest(); } void DownloadWorker::Resume() { is_paused_ = false; - if (request_handle_) - request_handle_->ResumeRequest(); } void DownloadWorker::Cancel(bool user_cancel) { is_canceled_ = true; - is_user_cancel_ = user_cancel; - if (request_handle_) - request_handle_->CancelRequest(user_cancel); + url_download_handler_.reset(); } void DownloadWorker::OnUrlDownloadStarted( std::unique_ptr<DownloadCreateInfo> create_info, std::unique_ptr<InputStream> input_stream, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + UrlDownloadHandler* downloader, const DownloadUrlParameters::OnStartedCallback& callback) { // |callback| is not used in subsequent requests. DCHECK(callback.is_null()); @@ -133,7 +123,7 @@ void DownloadWorker::OnUrlDownloadStarted( if (is_canceled_) { VLOG(kWorkerVerboseLevel) << "Byte stream arrived after user cancel the request."; - create_info->request_handle->CancelRequest(is_user_cancel_); + url_download_handler_.reset(); return; } @@ -145,8 +135,6 @@ void DownloadWorker::OnUrlDownloadStarted( input_stream.reset(new CompletedInputStream(create_info->result)); } - request_handle_ = std::move(create_info->request_handle); - // Pause the stream if user paused, still push the stream reader to the sink. if (is_paused_) { VLOG(kWorkerVerboseLevel) diff --git a/chromium/components/download/internal/common/download_worker.h b/chromium/components/download/internal/common/download_worker.h index 26b57219725..d560f0da584 100644 --- a/chromium/components/download/internal/common/download_worker.h +++ b/chromium/components/download/internal/common/download_worker.h @@ -11,20 +11,14 @@ #include "base/memory/scoped_refptr.h" #include "base/memory/weak_ptr.h" #include "components/download/public/common/download_export.h" -#include "components/download/public/common/download_request_handle_interface.h" #include "components/download/public/common/download_url_parameters.h" #include "components/download/public/common/url_download_handler.h" -namespace net { -class URLRequestContextGetter; -} // namespace net - namespace service_manager { class Connector; } // namespace service_manager 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. @@ -53,12 +47,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<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, - service_manager::Connector* connector); + void SendRequest(std::unique_ptr<DownloadUrlParameters> params, + URLLoaderFactoryProvider* url_loader_factory_provider, + service_manager::Connector* connector); // Download operations. void Pause(); @@ -70,8 +61,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker void OnUrlDownloadStarted( std::unique_ptr<DownloadCreateInfo> create_info, std::unique_ptr<InputStream> input_stream, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + UrlDownloadHandler* downloader, const DownloadUrlParameters::OnStartedCallback& callback) override; void OnUrlDownloadStopped(UrlDownloadHandler* downloader) override; void OnUrlDownloadHandlerCreated( @@ -88,10 +80,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker // States of the worker. bool is_paused_; bool is_canceled_; - bool is_user_cancel_; - - // Used to control the network request. Live on UI thread. - std::unique_ptr<DownloadRequestHandleInterface> request_handle_; // Used to handle the url request. Live and die on IO thread. UrlDownloadHandler::UniqueUrlDownloadHandlerPtr url_download_handler_; diff --git a/chromium/components/download/internal/common/in_progress_download_manager.cc b/chromium/components/download/internal/common/in_progress_download_manager.cc index de9122bcedc..694651a8ee3 100644 --- a/chromium/components/download/internal/common/in_progress_download_manager.cc +++ b/chromium/components/download/internal/common/in_progress_download_manager.cc @@ -20,11 +20,10 @@ #include "components/download/public/common/download_start_observer.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/features.h" +#include "components/leveldb_proto/public/proto_database_provider.h" #include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/resource_response.h" #include "services/service_manager/public/cpp/connector.h" @@ -82,7 +81,8 @@ void OnUrlDownloadHandlerCreated( void BeginResourceDownload( std::unique_ptr<DownloadUrlParameters> params, std::unique_ptr<network::ResourceRequest> request, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + std::unique_ptr<network::SharedURLLoaderFactoryInfo> + url_loader_factory_info, const URLSecurityPolicy& url_security_policy, bool is_new_download, base::WeakPtr<InProgressDownloadManager> download_manager, @@ -96,9 +96,11 @@ void BeginResourceDownload( UrlDownloadHandler::UniqueUrlDownloadHandlerPtr downloader( ResourceDownloader::BeginDownload( download_manager, std::move(params), std::move(request), - std::move(url_loader_factory_getter), url_security_policy, site_url, - tab_url, tab_referrer_url, is_new_download, false, - std::move(connector), is_background_mode, main_task_runner) + network::SharedURLLoaderFactory::Create( + std::move(url_loader_factory_info)), + url_security_policy, site_url, tab_url, tab_referrer_url, + is_new_download, false, std::move(connector), is_background_mode, + main_task_runner) .release(), base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); @@ -119,7 +121,8 @@ void CreateDownloadHandlerForNavigation( scoped_refptr<network::ResourceResponse> response_head, mojo::ScopedDataPipeConsumerHandle response_body, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + std::unique_ptr<network::SharedURLLoaderFactoryInfo> + url_loader_factory_info, const URLSecurityPolicy& url_security_policy, std::unique_ptr<service_manager::Connector> connector, const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) { @@ -131,8 +134,9 @@ void CreateDownloadHandlerForNavigation( std::move(url_chain), std::move(cert_status), std::move(response_head), std::move(response_body), std::move(url_loader_client_endpoints), - std::move(url_loader_factory_getter), url_security_policy, - std::move(connector), main_task_runner) + network::SharedURLLoaderFactory::Create( + std::move(url_loader_factory_info)), + url_security_policy, std::move(connector), main_task_runner) .release(), base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); @@ -193,15 +197,10 @@ InProgressDownloadManager::Delegate::GetDefaultDownloadDirectory() { return base::FilePath(); } -net::URLRequestContextGetter* -InProgressDownloadManager::Delegate::GetURLRequestContextGetter( - const DownloadCreateInfo& download_create_info) { - return nullptr; -} - InProgressDownloadManager::InProgressDownloadManager( Delegate* delegate, const base::FilePath& in_progress_db_dir, + leveldb_proto::ProtoDatabaseProvider* db_provider, const IsOriginSecureCallback& is_origin_secure_cb, const URLSecurityPolicy& url_security_policy, service_manager::Connector* connector) @@ -212,7 +211,7 @@ InProgressDownloadManager::InProgressDownloadManager( url_security_policy_(url_security_policy), use_empty_db_(in_progress_db_dir.empty()), connector_(connector) { - Initialize(in_progress_db_dir); + Initialize(in_progress_db_dir, db_provider); } InProgressDownloadManager::~InProgressDownloadManager() = default; @@ -220,10 +219,15 @@ 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, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + UrlDownloadHandler* downloader, const DownloadUrlParameters::OnStartedCallback& callback) { StartDownload(std::move(download_create_info), std::move(input_stream), - std::move(url_loader_factory_getter), callback); + std::move(url_loader_factory_provider), + base::BindOnce(&InProgressDownloadManager::CancelUrlDownload, + weak_factory_.GetWeakPtr(), downloader), + callback); } void InProgressDownloadManager::OnUrlDownloadStopped( @@ -250,7 +254,7 @@ void InProgressDownloadManager::DownloadUrl( // Start the new download, the download should be saved to the file path // specifcied in the |params|. - BeginDownload(std::move(params), url_loader_factory_getter_, + BeginDownload(std::move(params), url_loader_factory_->Clone(), true /* is_new_download */, GURL() /* site_url */, GURL() /* tab_url */, GURL() /* tab_referral_url */); } @@ -259,7 +263,7 @@ bool InProgressDownloadManager::CanDownload(DownloadUrlParameters* params) { if (!params->is_transient()) return false; - if (!url_loader_factory_getter_) + if (!url_loader_factory_) return false; if (params->require_safety_checks()) @@ -288,7 +292,8 @@ DownloadItem* InProgressDownloadManager::GetDownloadByGuid( void InProgressDownloadManager::BeginDownload( std::unique_ptr<DownloadUrlParameters> params, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + std::unique_ptr<network::SharedURLLoaderFactoryInfo> + url_loader_factory_info, bool is_new_download, const GURL& site_url, const GURL& tab_url, @@ -299,7 +304,7 @@ void InProgressDownloadManager::BeginDownload( FROM_HERE, base::BindOnce( &BeginResourceDownload, std::move(params), std::move(request), - std::move(url_loader_factory_getter), url_security_policy_, + std::move(url_loader_factory_info), url_security_policy_, is_new_download, weak_factory_.GetWeakPtr(), site_url, tab_url, tab_referrer_url, connector_ ? connector_->Clone() : nullptr, !delegate_ /* is_background_mode */, @@ -318,7 +323,8 @@ void InProgressDownloadManager::InterceptDownloadFromNavigation( scoped_refptr<network::ResourceResponse> response_head, mojo::ScopedDataPipeConsumerHandle response_body, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter) { + std::unique_ptr<network::SharedURLLoaderFactoryInfo> + url_loader_factory_info) { GetIOTaskRunner()->PostTask( FROM_HERE, base::BindOnce( @@ -327,19 +333,26 @@ void InProgressDownloadManager::InterceptDownloadFromNavigation( site_url, tab_url, tab_referrer_url, std::move(url_chain), std::move(cert_status), std::move(response_head), std::move(response_body), std::move(url_loader_client_endpoints), - std::move(url_loader_factory_getter), url_security_policy_, + std::move(url_loader_factory_info), url_security_policy_, connector_ ? connector_->Clone() : nullptr, base::ThreadTaskRunnerHandle::Get())); } void InProgressDownloadManager::Initialize( - const base::FilePath& in_progress_db_dir) { - download_db_cache_ = std::make_unique<DownloadDBCache>( - in_progress_db_dir.empty() - ? std::make_unique<DownloadDB>() - : std::make_unique<DownloadDBImpl>( - DownloadNamespace::NAMESPACE_BROWSER_DOWNLOAD, - in_progress_db_dir)); + const base::FilePath& in_progress_db_dir, + leveldb_proto::ProtoDatabaseProvider* db_provider) { + std::unique_ptr<DownloadDB> download_db; + + if (use_empty_db_) { + download_db = std::make_unique<DownloadDB>(); + } else { + download_db = std::make_unique<DownloadDBImpl>( + DownloadNamespace::NAMESPACE_BROWSER_DOWNLOAD, in_progress_db_dir, + db_provider); + } + + download_db_cache_ = + std::make_unique<DownloadDBCache>(std::move(download_db)); download_db_cache_->Initialize(base::BindOnce( &InProgressDownloadManager::OnDBInitialized, weak_factory_.GetWeakPtr())); } @@ -390,11 +403,11 @@ void InProgressDownloadManager::DetermineDownloadTarget( void InProgressDownloadManager::ResumeInterruptedDownload( std::unique_ptr<DownloadUrlParameters> params, const GURL& site_url) { - if (!url_loader_factory_getter_) + if (!url_loader_factory_) return; - BeginDownload(std::move(params), url_loader_factory_getter_, false, site_url, - GURL(), GURL()); + BeginDownload(std::move(params), url_loader_factory_->Clone(), false, + site_url, GURL(), GURL()); } bool InProgressDownloadManager::ShouldOpenDownload( @@ -425,7 +438,9 @@ void InProgressDownloadManager::RemoveInProgressDownload( void InProgressDownloadManager::StartDownload( std::unique_ptr<DownloadCreateInfo> info, std::unique_ptr<InputStream> stream, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadUrlParameters::OnStartedCallback& on_started) { DCHECK(info); @@ -434,6 +449,8 @@ void InProgressDownloadManager::StartDownload( info->result == DOWNLOAD_INTERRUPT_REASON_SERVER_CROSS_ORIGIN_REDIRECT)) { if (delegate_ && delegate_->InterceptDownload(*info)) { + if (cancel_request_callback) + std::move(cancel_request_callback).Run(false); GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release()); return; } @@ -451,7 +468,6 @@ void InProgressDownloadManager::StartDownload( std::string mime_type = info->mime_type; if (info->is_new_download) { - RecordDownloadConnectionSecurity(info->url(), info->url_chain); RecordDownloadContentTypeSecurity(info->url(), info->url_chain, info->mime_type, is_origin_secure_cb_); } @@ -463,7 +479,8 @@ void InProgressDownloadManager::StartDownload( std::move(info), on_started, base::BindOnce(&InProgressDownloadManager::StartDownloadWithItem, weak_factory_.GetWeakPtr(), std::move(stream), - std::move(url_loader_factory_getter))); + std::move(url_loader_factory_provider), + std::move(cancel_request_callback))); } else { std::string guid = info->guid; if (info->is_new_download) { @@ -473,15 +490,17 @@ void InProgressDownloadManager::StartDownload( in_progress_downloads_.push_back(std::move(download)); } StartDownloadWithItem( - std::move(stream), std::move(url_loader_factory_getter), - std::move(info), + std::move(stream), std::move(url_loader_factory_provider), + std::move(cancel_request_callback), std::move(info), static_cast<DownloadItemImpl*>(GetDownloadByGuid(guid)), false); } } void InProgressDownloadManager::StartDownloadWithItem( std::unique_ptr<InputStream> stream, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + DownloadJob::CancelRequestCallback cancel_request_callback, std::unique_ptr<DownloadCreateInfo> info, DownloadItemImpl* download, bool should_persist_new_download) { @@ -489,8 +508,8 @@ void InProgressDownloadManager::StartDownloadWithItem( // 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 (cancel_request_callback) + std::move(cancel_request_callback).Run(false); // The ByteStreamReader lives and dies on the download sequence. if (info->result == DOWNLOAD_INTERRUPT_REASON_NONE) GetDownloadTaskRunner()->DeleteSoon(FROM_HERE, stream.release()); @@ -523,15 +542,8 @@ void InProgressDownloadManager::StartDownloadWithItem( // so that the DownloadItem can salvage what it can out of a failed // resumption attempt. - net::URLRequestContextGetter* url_request_context_getter = nullptr; - if (delegate_ && - !base::FeatureList::IsEnabled(network::features::kNetworkService)) { - url_request_context_getter = delegate_->GetURLRequestContextGetter(*info); - } - - download->Start(std::move(download_file), std::move(info->request_handle), - *info, std::move(url_loader_factory_getter), - url_request_context_getter); + download->Start(std::move(download_file), std::move(cancel_request_callback), + *info, std::move(url_loader_factory_provider)); if (download_start_observer_) download_start_observer_->OnDownloadStarted(download); @@ -653,4 +665,10 @@ void InProgressDownloadManager::AddInProgressDownloadForTest( in_progress_downloads_.push_back(std::move(download)); } +void InProgressDownloadManager::CancelUrlDownload( + UrlDownloadHandler* downloader, + bool user_cancel) { + OnUrlDownloadStopped(downloader); +} + } // 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 45600c7054f..a2e1e42495d 100644 --- a/chromium/components/download/internal/common/parallel_download_job.cc +++ b/chromium/components/download/internal/common/parallel_download_job.cc @@ -12,9 +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" -#include "net/url_request/url_request_context_getter.h" namespace download { namespace { @@ -23,21 +21,19 @@ const int kDownloadJobVerboseLevel = 1; ParallelDownloadJob::ParallelDownloadJob( DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle, + CancelRequestCallback cancel_request_callback, const DownloadCreateInfo& create_info, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, service_manager::Connector* connector) - : DownloadJobImpl(download_item, std::move(request_handle), true), + : DownloadJobImpl(download_item, std::move(cancel_request_callback), true), initial_request_offset_(create_info.offset), initial_received_slices_(download_item->GetReceivedSlices()), content_length_(create_info.total_bytes), requests_sent_(false), is_canceled_(false), range_support_(create_info.accept_range), - url_loader_factory_getter_(std::move(url_loader_factory_getter)), - url_request_context_getter_(url_request_context_getter), + url_loader_factory_provider_(std::move(url_loader_factory_provider)), connector_(connector) {} ParallelDownloadJob::~ParallelDownloadJob() = default; @@ -298,8 +294,8 @@ void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) { download_params->set_follow_cross_origin_redirects(false); // Send the request. - worker->SendRequest(std::move(download_params), url_loader_factory_getter_, - url_request_context_getter_, connector_); + worker->SendRequest(std::move(download_params), + url_loader_factory_provider_.get(), connector_); 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 ca410838e13..e502805555b 100644 --- a/chromium/components/download/internal/common/parallel_download_job.h +++ b/chromium/components/download/internal/common/parallel_download_job.h @@ -10,17 +10,14 @@ #include <vector> #include "base/macros.h" -#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" #include "base/timer/timer.h" #include "components/download/internal/common/download_job_impl.h" #include "components/download/internal/common/download_worker.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_export.h" #include "components/download/public/common/parallel_download_configs.h" - -namespace net { -class URLRequestContextGetter; -} // namespace net +#include "components/download/public/common/url_loader_factory_provider.h" namespace service_manager { class Connector; @@ -37,14 +34,12 @@ class COMPONENTS_DOWNLOAD_EXPORT ParallelDownloadJob public: // TODO(qinmin): Remove |url_request_context_getter| once network service is // enabled. - ParallelDownloadJob( - DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle, - const DownloadCreateInfo& create_info, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter, - service_manager::Connector* connector); + ParallelDownloadJob(DownloadItem* download_item, + CancelRequestCallback cancel_request_callback, + const DownloadCreateInfo& create_info, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + service_manager::Connector* connector); ~ParallelDownloadJob() override; // DownloadJobImpl implementation. @@ -123,13 +118,10 @@ class COMPONENTS_DOWNLOAD_EXPORT ParallelDownloadJob // Whether the server accepts range requests. RangeRequestSupportType range_support_; - // 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. - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_; + // URLLoaderFactoryProvider to retrieve the URLLoaderFactory and issue + // parallel requests. + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider_; // Connector used for establishing the connection to the ServiceManager. service_manager::Connector* connector_; 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 d227cf4232e..7909c2ee000 100644 --- a/chromium/components/download/internal/common/parallel_download_job_unittest.cc +++ b/chromium/components/download/internal/common/parallel_download_job_unittest.cc @@ -10,7 +10,7 @@ #include "base/bind.h" #include "base/run_loop.h" #include "base/test/mock_callback.h" -#include "base/test/scoped_task_environment.h" +#include "base/test/task_environment.h" #include "components/download/internal/common/parallel_download_utils.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_destination_observer.h" @@ -31,13 +31,6 @@ namespace download { namespace { -class MockDownloadRequestHandle : public DownloadRequestHandleInterface { - public: - MOCK_METHOD0(PauseRequest, void()); - MOCK_METHOD0(ResumeRequest, void()); - MOCK_METHOD1(CancelRequest, void(bool)); -}; - class MockDownloadDestinationObserver : public DownloadDestinationObserver { public: MOCK_METHOD3(DestinationUpdate, @@ -60,16 +53,15 @@ class ParallelDownloadJobForTest : public ParallelDownloadJob { public: ParallelDownloadJobForTest( DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadCreateInfo& create_info, int request_count, int64_t min_slice_size, int min_remaining_time) : ParallelDownloadJob(download_item, - std::move(request_handle), + std::move(cancel_request_callback), create_info, - nullptr, - nullptr, + URLLoaderFactoryProvider::GetNullPtr(), nullptr), request_count_(request_count), min_slice_size_(min_slice_size), @@ -115,9 +107,9 @@ class ParallelDownloadJobForTest : public ParallelDownloadJob { class ParallelDownloadJobTest : public testing::Test { public: ParallelDownloadJobTest() - : task_environment_(base::test::ScopedTaskEnvironment::MainThreadType::UI, - base::test::ScopedTaskEnvironment:: - ThreadPoolExecutionMode::QUEUED) {} + : task_environment_( + base::test::TaskEnvironment::MainThreadType::UI, + base::test::TaskEnvironment::ThreadPoolExecutionMode::QUEUED) {} void CreateParallelJob(int64_t initial_request_offset, int64_t content_length, @@ -137,19 +129,17 @@ class ParallelDownloadJobTest : public testing::Test { DownloadCreateInfo info; info.offset = initial_request_offset; info.total_bytes = content_length; - std::unique_ptr<MockDownloadRequestHandle> request_handle = - std::make_unique<MockDownloadRequestHandle>(); - mock_request_handle_ = request_handle.get(); job_ = std::make_unique<ParallelDownloadJobForTest>( - download_item_.get(), std::move(request_handle), info, request_count, - min_slice_size, min_remaining_time); + download_item_.get(), + base::Bind(&ParallelDownloadJobTest::CancelRequest, + base::Unretained(this)), + info, request_count, min_slice_size, min_remaining_time); file_initialized_ = false; } void DestroyParallelJob() { job_.reset(); download_item_.reset(); - mock_request_handle_ = nullptr; } void BuildParallelRequests() { job_->BuildParallelRequests(); } @@ -160,18 +150,7 @@ class ParallelDownloadJobTest : public testing::Test { bool IsJobCanceled() const { return job_->is_canceled_; } - void MakeWorkerReady( - DownloadWorker* worker, - std::unique_ptr<MockDownloadRequestHandle> request_handle) { - UrlDownloadHandler::Delegate* delegate = - static_cast<UrlDownloadHandler::Delegate*>(worker); - std::unique_ptr<DownloadCreateInfo> create_info = - std::make_unique<DownloadCreateInfo>(); - create_info->request_handle = std::move(request_handle); - delegate->OnUrlDownloadStarted(std::move(create_info), - std::make_unique<MockInputStream>(), nullptr, - DownloadUrlParameters::OnStartedCallback()); - } + void CancelRequest(bool user_cancel) { canceled_ = true; } void VerifyWorker(int64_t offset, int64_t length) const { EXPECT_TRUE(job_->workers_.find(offset) != job_->workers_.end()); @@ -183,12 +162,11 @@ class ParallelDownloadJobTest : public testing::Test { file_initialized_ = true; } - base::test::ScopedTaskEnvironment task_environment_; + base::test::TaskEnvironment task_environment_; std::unique_ptr<MockDownloadItem> download_item_; std::unique_ptr<ParallelDownloadJobForTest> job_; bool file_initialized_; - // Request handle for the original request. - MockDownloadRequestHandle* mock_request_handle_; + bool canceled_ = false; // The received slices used to return in // |MockDownloadItemImpl::GetReceivedSlices| mock function. @@ -404,11 +382,11 @@ TEST_F(ParallelDownloadJobTest, LastReceivedSliceFinished) { // built. TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeBuildRequests) { CreateParallelJob(0, 100, DownloadItem::ReceivedSlices(), 2, 1, 10); - EXPECT_CALL(*mock_request_handle_, CancelRequest(_)); // Job is canceled before building parallel requests. job_->Cancel(true); EXPECT_TRUE(IsJobCanceled()); + EXPECT_TRUE(canceled_); BuildParallelRequests(); EXPECT_TRUE(job_->workers().empty()); @@ -416,55 +394,6 @@ TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeBuildRequests) { DestroyParallelJob(); } -// Ensure cancel before adding the byte stream will result in workers being -// canceled. -TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeByteStreamReady) { - CreateParallelJob(0, 100, DownloadItem::ReceivedSlices(), 2, 1, 10); - EXPECT_CALL(*mock_request_handle_, CancelRequest(_)); - - BuildParallelRequests(); - VerifyWorker(50, 0); - - // Job is canceled after building parallel requests and before byte streams - // are added to the file sink. - job_->Cancel(true); - EXPECT_TRUE(IsJobCanceled()); - - for (auto& worker : job_->workers()) { - std::unique_ptr<MockDownloadRequestHandle> mock_handle = - std::make_unique<MockDownloadRequestHandle>(); - EXPECT_CALL(*mock_handle, CancelRequest(_)); - MakeWorkerReady(worker.second.get(), std::move(mock_handle)); - } - - DestroyParallelJob(); -} - -// Ensure pause before adding the byte stream will result in workers being -// paused. -TEST_F(ParallelDownloadJobTest, EarlyPauseBeforeByteStreamReady) { - CreateParallelJob(0, 100, DownloadItem::ReceivedSlices(), 2, 1, 10); - EXPECT_CALL(*mock_request_handle_, PauseRequest()); - - BuildParallelRequests(); - VerifyWorker(50, 0); - - // Job is paused after building parallel requests and before adding the byte - // stream to the file sink. - job_->Pause(); - EXPECT_TRUE(job_->is_paused()); - - for (auto& worker : job_->workers()) { - EXPECT_CALL(*job_, CountOnInputStreamReady()); - std::unique_ptr<MockDownloadRequestHandle> mock_handle = - std::make_unique<MockDownloadRequestHandle>(); - EXPECT_CALL(*mock_handle, PauseRequest()); - MakeWorkerReady(worker.second.get(), std::move(mock_handle)); - } - - DestroyParallelJob(); -} - // Test that parallel request is not created if the remaining content can be // finish downloading soon. TEST_F(ParallelDownloadJobTest, RemainingContentWillFinishSoon) { diff --git a/chromium/components/download/internal/common/resource_downloader.cc b/chromium/components/download/internal/common/resource_downloader.cc index 15de44ecd42..cf8829ee8d4 100644 --- a/chromium/components/download/internal/common/resource_downloader.cc +++ b/chromium/components/download/internal/common/resource_downloader.cc @@ -7,9 +7,9 @@ #include <memory> #include "base/bind.h" -#include "components/download/public/common/download_url_loader_factory_getter.h" #include "components/download/public/common/stream_handle_input_stream.h" -#include "components/download/public/common/url_download_request_handle.h" +#include "components/download/public/common/url_loader_factory_provider.h" +#include "mojo/public/cpp/bindings/remote.h" #include "services/device/public/mojom/constants.mojom.h" #include "services/device/public/mojom/wake_lock_provider.mojom.h" #include "services/network/public/cpp/shared_url_loader_factory.h" @@ -31,9 +31,9 @@ class URLLoaderStatusMonitor : public network::mojom::URLLoaderClient { ~URLLoaderStatusMonitor() override = default; // network::mojom::URLLoaderClient - void OnReceiveResponse(const network::ResourceResponseHead& head) override {} + void OnReceiveResponse(network::mojom::URLResponseHeadPtr head) override {} void OnReceiveRedirect(const net::RedirectInfo& redirect_info, - const network::ResourceResponseHead& head) override {} + network::mojom::URLResponseHeadPtr head) override {} void OnUploadProgress(int64_t current_position, int64_t total_size, OnUploadProgressCallback callback) override {} @@ -62,8 +62,7 @@ std::unique_ptr<ResourceDownloader> ResourceDownloader::BeginDownload( base::WeakPtr<UrlDownloadHandler::Delegate> delegate, std::unique_ptr<DownloadUrlParameters> params, std::unique_ptr<network::ResourceRequest> request, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, const GURL& site_url, const GURL& tab_url, @@ -77,8 +76,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, is_new_download, task_runner, - std::move(url_loader_factory_getter), url_security_policy, - std::move(connector)); + std::move(url_loader_factory), url_security_policy, std::move(connector)); downloader->Start(std::move(params), is_parallel_request, is_background_mode); return downloader; @@ -99,16 +97,14 @@ ResourceDownloader::InterceptNavigationResponse( const scoped_refptr<network::ResourceResponse>& response_head, mojo::ScopedDataPipeConsumerHandle response_body, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, std::unique_ptr<service_manager::Connector> connector, 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, true, task_runner, - std::move(url_loader_factory_getter), url_security_policy, - std::move(connector)); + std::move(url_loader_factory), url_security_policy, std::move(connector)); downloader->InterceptResponse( std::move(url_chain), cert_status, std::move(response_head), std::move(response_body), std::move(url_loader_client_endpoints)); @@ -125,8 +121,7 @@ ResourceDownloader::ResourceDownloader( const GURL& tab_referrer_url, bool is_new_download, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, std::unique_ptr<service_manager::Connector> connector) : delegate_(delegate), @@ -138,7 +133,7 @@ ResourceDownloader::ResourceDownloader( tab_url_(tab_url), tab_referrer_url_(tab_referrer_url), delegate_task_runner_(task_runner), - url_loader_factory_getter_(std::move(url_loader_factory_getter)), + url_loader_factory_(url_loader_factory), url_security_policy_(url_security_policy), is_content_initiated_(false) { RequestWakeLock(connector.get()); @@ -166,8 +161,8 @@ void ResourceDownloader::Start( download_url_parameters->request_headers(), download_url_parameters->request_origin(), download_url_parameters->download_source(), - download_url_parameters->ignore_content_length_mismatch(), std::vector<GURL>(1, resource_request_->url), is_background_mode); + network::mojom::URLLoaderClientPtr url_loader_client_ptr; url_loader_client_binding_ = std::make_unique<mojo::Binding<network::mojom::URLLoaderClient>>( @@ -176,7 +171,7 @@ void ResourceDownloader::Start( // Set up the URLLoader network::mojom::URLLoaderRequest url_loader_request = mojo::MakeRequest(&url_loader_); - url_loader_factory_getter_->GetURLLoaderFactory()->CreateLoaderAndStart( + url_loader_factory_->CreateLoaderAndStart( std::move(url_loader_request), 0, // routing_id 0, // request_id @@ -206,8 +201,7 @@ void ResourceDownloader::InterceptResponse( true, /* follow_cross_origin_redirects */ download::DownloadUrlParameters::RequestHeadersType(), std::string(), /* request_origin */ - download::DownloadSource::NAVIGATION, - false /* ignore_content_length_mismatch */, std::move(url_chain), + download::DownloadSource::NAVIGATION, std::move(url_chain), false /* is_background_mode */); // Simulate on the new URLLoaderClient calls that happened on the old client. @@ -227,8 +221,6 @@ void ResourceDownloader::InterceptResponse( void ResourceDownloader::OnResponseStarted( std::unique_ptr<DownloadCreateInfo> download_create_info, mojom::DownloadStreamHandlePtr stream_handle) { - download_create_info->request_handle.reset(new UrlDownloadRequestHandle( - weak_ptr_factory_.GetWeakPtr(), base::SequencedTaskRunnerHandle::Get())); download_create_info->is_new_download = is_new_download_; download_create_info->guid = guid_; download_create_info->site_url = site_url_; @@ -245,7 +237,10 @@ void ResourceDownloader::OnResponseStarted( &UrlDownloadHandler::Delegate::OnUrlDownloadStarted, delegate_, std::move(download_create_info), std::make_unique<StreamHandleInputStream>(std::move(stream_handle)), - std::move(url_loader_factory_getter_), callback_)); + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr( + new URLLoaderFactoryProvider(url_loader_factory_), + base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())), + this, callback_)); } void ResourceDownloader::OnReceiveRedirect() { @@ -289,13 +284,13 @@ void ResourceDownloader::RequestWakeLock( service_manager::Connector* connector) { if (!connector) return; - device::mojom::WakeLockProviderPtr wake_lock_provider; - connector->BindInterface(device::mojom::kServiceName, - mojo::MakeRequest(&wake_lock_provider)); + mojo::Remote<device::mojom::WakeLockProvider> wake_lock_provider; + connector->Connect(device::mojom::kServiceName, + wake_lock_provider.BindNewPipeAndPassReceiver()); wake_lock_provider->GetWakeLockWithoutContext( device::mojom::WakeLockType::kPreventAppSuspension, device::mojom::WakeLockReason::kOther, "Download in progress", - mojo::MakeRequest(&wake_lock_)); + wake_lock_.BindNewPipeAndPassReceiver()); wake_lock_->RequestWakeLock(); } diff --git a/chromium/components/download/internal/common/resource_downloader.h b/chromium/components/download/internal/common/resource_downloader.h index 230269b856d..fe7cb243a75 100644 --- a/chromium/components/download/internal/common/resource_downloader.h +++ b/chromium/components/download/internal/common/resource_downloader.h @@ -11,6 +11,7 @@ #include "components/download/public/common/download_utils.h" #include "components/download/public/common/url_download_handler.h" #include "mojo/public/cpp/bindings/binding.h" +#include "mojo/public/cpp/bindings/remote.h" #include "net/cert/cert_status_flags.h" #include "services/device/public/mojom/wake_lock.mojom.h" #include "services/network/public/cpp/resource_request.h" @@ -21,20 +22,17 @@ class Connector; } // namespace service_manager namespace download { -class DownloadURLLoaderFactoryGetter; - // Class for handing the download of a url. Lives on IO thread. class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader - : public download::UrlDownloadHandler, - public download::DownloadResponseHandler::Delegate { + : public UrlDownloadHandler, + public DownloadResponseHandler::Delegate { public: // Called to start a download, must be called on IO thread. static std::unique_ptr<ResourceDownloader> BeginDownload( base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, std::unique_ptr<download::DownloadUrlParameters> download_url_parameters, std::unique_ptr<network::ResourceRequest> request, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, const GURL& site_url, const GURL& tab_url, @@ -61,8 +59,7 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader const scoped_refptr<network::ResourceResponse>& response_head, mojo::ScopedDataPipeConsumerHandle response_body, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, std::unique_ptr<service_manager::Connector> connector, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); @@ -77,16 +74,15 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader const GURL& tab_referrer_url, bool is_new_download, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, std::unique_ptr<service_manager::Connector> connector); ~ResourceDownloader() override; - // download::DownloadResponseHandler::Delegate + // DownloadResponseHandler::Delegate void OnResponseStarted( std::unique_ptr<download::DownloadCreateInfo> download_create_info, - download::mojom::DownloadStreamHandlePtr stream_handle) override; + mojom::DownloadStreamHandlePtr stream_handle) override; void OnReceiveRedirect() override; void OnResponseCompleted() override; bool CanRequestURL(const GURL& url) override; @@ -162,9 +158,8 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader // TaskRunner to post callbacks to the |delegate_| scoped_refptr<base::SingleThreadTaskRunner> delegate_task_runner_; - // URLLoaderFactory getter for issueing network requests. - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter_; + // URLLoaderFactory for issuing network requests. + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; // Used to check if the URL is safe to request. URLSecurityPolicy url_security_policy_; @@ -175,7 +170,7 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader // Used to keep the system from sleeping while a download is ongoing. If the // system enters power saving mode while a download is alive, it can cause // download to be interrupted. - device::mojom::WakeLockPtr wake_lock_; + mojo::Remote<device::mojom::WakeLock> wake_lock_; base::WeakPtrFactory<ResourceDownloader> weak_ptr_factory_{this}; diff --git a/chromium/components/download/internal/common/save_package_download_job.cc b/chromium/components/download/internal/common/save_package_download_job.cc index ae0c9277798..bd6734e652f 100644 --- a/chromium/components/download/internal/common/save_package_download_job.cc +++ b/chromium/components/download/internal/common/save_package_download_job.cc @@ -8,8 +8,8 @@ namespace download { SavePackageDownloadJob::SavePackageDownloadJob( DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle) - : DownloadJob(download_item, std::move(request_handle)) {} + DownloadJob::CancelRequestCallback cancel_request_callback) + : DownloadJob(download_item, std::move(cancel_request_callback)) {} SavePackageDownloadJob::~SavePackageDownloadJob() = default; diff --git a/chromium/components/download/internal/common/save_package_download_job.h b/chromium/components/download/internal/common/save_package_download_job.h index fea7fb1bbb5..fdcffcabc2c 100644 --- a/chromium/components/download/internal/common/save_package_download_job.h +++ b/chromium/components/download/internal/common/save_package_download_job.h @@ -8,7 +8,6 @@ #include "base/macros.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/download_job.h" -#include "components/download/public/common/download_request_handle_interface.h" namespace download { @@ -16,7 +15,7 @@ class SavePackageDownloadJob : public DownloadJob { public: SavePackageDownloadJob( DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle); + DownloadJob::CancelRequestCallback cancel_request_callback); ~SavePackageDownloadJob() override; // DownloadJob implementation. 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 d0634a03292..056a9f562fa 100644 --- a/chromium/components/download/internal/common/url_download_handler_factory.cc +++ b/chromium/components/download/internal/common/url_download_handler_factory.cc @@ -8,89 +8,31 @@ #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 "net/url_request/url_request_context_getter.h" #include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/service_manager/public/cpp/connector.h" namespace download { -namespace { - -// Factory for creating URLDownloadHandler used by network service. -class DefaultUrlDownloadHandlerFactory : public UrlDownloadHandlerFactory { - public: - DefaultUrlDownloadHandlerFactory() = default; - ~DefaultUrlDownloadHandlerFactory() override = default; - - protected: - UrlDownloadHandler::UniqueUrlDownloadHandlerPtr CreateUrlDownloadHandler( - std::unique_ptr<download::DownloadUrlParameters> params, - base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - const URLSecurityPolicy& url_security_policy, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, - std::unique_ptr<service_manager::Connector> connector, - 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(url_loader_factory_getter), url_security_policy, GURL(), - GURL(), GURL(), true, true, std::move(connector), - false /* is_background_mode */, task_runner) - .release(), - base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); - } - - private: - DISALLOW_COPY_AND_ASSIGN(DefaultUrlDownloadHandlerFactory); -}; - -UrlDownloadHandlerFactory* g_url_download_handler_factory; - -// Lock to protect |g_url_download_handler_factory| -base::Lock& GetURLDownloadHandlerFactoryLock() { - static base::NoDestructor<base::Lock> instance; - return *instance; -} - -} // namespace - // static UrlDownloadHandler::UniqueUrlDownloadHandlerPtr UrlDownloadHandlerFactory::Create( std::unique_ptr<download::DownloadUrlParameters> params, base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, std::unique_ptr<service_manager::Connector> connector, 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(url_loader_factory_getter), - std::move(url_security_policy), std::move(url_request_context_getter), - std::move(connector), task_runner); -} - -// static -void UrlDownloadHandlerFactory::Install(UrlDownloadHandlerFactory* factory) { - base::AutoLock auto_lock(GetURLDownloadHandlerFactoryLock()); - if (factory == g_url_download_handler_factory) - return; - delete g_url_download_handler_factory; - g_url_download_handler_factory = factory; + std::unique_ptr<network::ResourceRequest> request = + CreateResourceRequest(params.get()); + return UrlDownloadHandler::UniqueUrlDownloadHandlerPtr( + download::ResourceDownloader::BeginDownload( + delegate, std::move(params), std::move(request), + std::move(url_loader_factory), url_security_policy, GURL(), GURL(), + GURL(), true, true, std::move(connector), + false /* is_background_mode */, task_runner) + .release(), + base::OnTaskRunnerDeleter(base::ThreadTaskRunnerHandle::Get())); } -UrlDownloadHandlerFactory::UrlDownloadHandlerFactory() = default; - -UrlDownloadHandlerFactory::~UrlDownloadHandlerFactory() = default; - } // namespace download diff --git a/chromium/components/download/internal/common/url_download_request_handle.cc b/chromium/components/download/internal/common/url_download_request_handle.cc index 9faf499f03d..186adf819e1 100644 --- a/chromium/components/download/internal/common/url_download_request_handle.cc +++ b/chromium/components/download/internal/common/url_download_request_handle.cc @@ -27,22 +27,4 @@ UrlDownloadRequestHandle& UrlDownloadRequestHandle::operator=( UrlDownloadRequestHandle::~UrlDownloadRequestHandle() = default; -void UrlDownloadRequestHandle::PauseRequest() { - downloader_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&UrlDownloadHandler::PauseRequest, downloader_)); -} - -void UrlDownloadRequestHandle::ResumeRequest() { - downloader_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&UrlDownloadHandler::ResumeRequest, downloader_)); -} - -void UrlDownloadRequestHandle::CancelRequest(bool user_cancel) { - downloader_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&UrlDownloadHandler::CancelRequest, downloader_)); -} - } // namespace download diff --git a/chromium/components/download/internal/common/url_loader_factory_provider.cc b/chromium/components/download/internal/common/url_loader_factory_provider.cc new file mode 100644 index 00000000000..d4c26271a59 --- /dev/null +++ b/chromium/components/download/internal/common/url_loader_factory_provider.cc @@ -0,0 +1,29 @@ +// Copyright 2019 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/url_loader_factory_provider.h" + +#include "components/download/public/common/download_task_runner.h" + +namespace download { + +URLLoaderFactoryProvider::URLLoaderFactoryProvider( + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) + : url_loader_factory_(std::move(url_loader_factory)) {} + +URLLoaderFactoryProvider::~URLLoaderFactoryProvider() = default; + +scoped_refptr<network::SharedURLLoaderFactory> +URLLoaderFactoryProvider::GetURLLoaderFactory() { + return url_loader_factory_; +} + +// static +URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr +URLLoaderFactoryProvider::GetNullPtr() { + return URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr( + nullptr, base::OnTaskRunnerDeleter(nullptr)); +} + +} // namespace download diff --git a/chromium/components/download/public/common/BUILD.gn b/chromium/components/download/public/common/BUILD.gn index 5883cbbcb11..3febd81f0b4 100644 --- a/chromium/components/download/public/common/BUILD.gn +++ b/chromium/components/download/public/common/BUILD.gn @@ -40,7 +40,6 @@ component("public") { "download_job.h", "download_job_factory.h", "download_path_reservation_tracker.h", - "download_request_handle_interface.h", "download_response_handler.h", "download_save_info.cc", "download_save_info.h", @@ -49,8 +48,6 @@ component("public") { "download_stats.h", "download_task_runner.h", "download_ukm_helper.h", - "download_url_loader_factory_getter.h", - "download_url_loader_factory_getter_impl.h", "download_url_parameters.cc", "download_url_parameters.h", "download_utils.h", @@ -58,6 +55,7 @@ component("public") { "input_stream.cc", "input_stream.h", "parallel_download_configs.h", + "quarantine_connection.h", "rate_estimator.h", "resume_mode.h", "simple_download_manager.h", @@ -65,19 +63,22 @@ component("public") { "stream_handle_input_stream.h", "url_download_handler_factory.h", "url_download_request_handle.h", + "url_loader_factory_provider.h", ] configs += [ ":components_download_implementation" ] public_deps = [ ":interfaces", + "//base", "//components/download/network", "//components/download/public/background_service:public", + "//components/services/quarantine/public/mojom", + "//mojo/public/cpp/bindings", "//services/network/public/cpp", ] deps = [ - "//base", "//components/download/database", "//components/download/internal/common:internal", "//components/services/quarantine:quarantine", diff --git a/chromium/components/download/public/common/base_file.h b/chromium/components/download/public/common/base_file.h index 4a01d7b7d0e..5a80471d12d 100644 --- a/chromium/components/download/public/common/base_file.h +++ b/chromium/components/download/public/common/base_file.h @@ -27,13 +27,11 @@ #include "components/download/public/common/download_interrupt_reasons.h" #include "components/services/quarantine/public/mojom/quarantine.mojom.h" #include "crypto/secure_hash.h" +#include "mojo/public/cpp/bindings/pending_remote.h" +#include "mojo/public/cpp/bindings/remote.h" #include "net/base/net_errors.h" #include "url/gurl.h" -namespace service_manager { -class Connector; -} - namespace download { // File being downloaded and saved to disk. This is a base class @@ -169,8 +167,7 @@ class COMPONENTS_DOWNLOAD_EXPORT BaseFile { using OnAnnotationDoneCallback = base::OnceCallback<void(DownloadInterruptReason)>; - // Called when a quarantine service is used, - // connector is used to populate a quarantine::mojom::QuarantinePtr, + // Called when a quarantine service is used. // and the callback will be called from the service. // TODO (crbug.com/973497): Remove non-service version when // kPreventDownloadsWithSamePath feature is removed. @@ -178,7 +175,7 @@ class COMPONENTS_DOWNLOAD_EXPORT BaseFile { const std::string& client_guid, const GURL& source_url, const GURL& referrer_url, - std::unique_ptr<service_manager::Connector> connector, + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine, OnAnnotationDoneCallback on_annotation_done_callback); #if defined(OS_ANDROID) @@ -304,8 +301,8 @@ class COMPONENTS_DOWNLOAD_EXPORT BaseFile { // ID of the download, used for trace events. uint32_t download_id_; - // Mojo pointer for quarantine service. - quarantine::mojom::QuarantinePtr quarantine_service_; + // Mojo remote for quarantine service. + mojo::Remote<quarantine::mojom::Quarantine> quarantine_service_; // Callback invoked after quarantine service finishes. OnAnnotationDoneCallback on_annotation_done_callback_; diff --git a/chromium/components/download/public/common/download_create_info.h b/chromium/components/download/public/common/download_create_info.h index 84553dc1b97..d1699c66c7b 100644 --- a/chromium/components/download/public/common/download_create_info.h +++ b/chromium/components/download/public/common/download_create_info.h @@ -10,6 +10,7 @@ #include <string> #include <vector> +#include "base/callback_forward.h" #include "base/files/file_path.h" #include "base/macros.h" #include "base/memory/ref_counted.h" @@ -18,7 +19,6 @@ #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_request_handle_interface.h" #include "components/download/public/common/download_save_info.h" #include "components/download/public/common/download_source.h" #include "components/download/public/common/download_url_parameters.h" @@ -34,7 +34,6 @@ class HttpResponseHeaders; } namespace download { - // Server support for range request inferred from the response headers. // |kSupport| value means the server supports range requests. |kNoSupport| // means no range request is accepted by server. and |kUnknown| is used if @@ -124,10 +123,6 @@ struct COMPONENTS_DOWNLOAD_EXPORT DownloadCreateInfo { // The render frame id that initiates this download. int render_frame_id; - // The handle to the URLRequest sourcing this download. - // TODO(qinmin): remove this when network service is fully enabled. - std::unique_ptr<DownloadRequestHandleInterface> request_handle; - // --------------------------------------------------------------------------- // The remaining fields are Entity-body properties. These are only set if // |result| is DOWNLOAD_INTERRUPT_REASON_NONE. diff --git a/chromium/components/download/public/common/download_features.cc b/chromium/components/download/public/common/download_features.cc index 3d27465f635..3411080f7eb 100644 --- a/chromium/components/download/public/common/download_features.cc +++ b/chromium/components/download/public/common/download_features.cc @@ -54,10 +54,10 @@ const base::Feature kUseParallelRequestsForUnknwonRangeSupport{ base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kUseParallelRequestsForHTTP2{ - "kUseParallelRequestsForHTTP2", base::FEATURE_DISABLED_BY_DEFAULT}; + "UseParallelRequestsForHTTP2", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kUseParallelRequestsForQUIC{ - "kUseParallelRequestsForQUIC", base::FEATURE_DISABLED_BY_DEFAULT}; + "UseParallelRequestsForQUIC", base::FEATURE_DISABLED_BY_DEFAULT}; } // namespace features } // namespace download diff --git a/chromium/components/download/public/common/download_file.h b/chromium/components/download/public/common/download_file.h index 6cfd902a6d4..7f81cfbebc1 100644 --- a/chromium/components/download/public/common/download_file.h +++ b/chromium/components/download/public/common/download_file.h @@ -18,14 +18,12 @@ #include "components/download/public/common/download_interrupt_reasons.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/input_stream.h" +#include "components/services/quarantine/public/mojom/quarantine.mojom.h" +#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/system/data_pipe.h" class GURL; -namespace service_manager { -class Connector; -} - namespace download { // These objects live exclusively on the download sequence and handle the @@ -86,16 +84,15 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFile { // Rename the download file to |full_path| and annotate it with // "Mark of the Web" information about its source. No uniquification // will be performed. - // connector is a clone of the service manager connector from - // DownloadItemImpl's delegate. It used to create the quarantine service. - // In the unexpected case that connector is null, or the service otherwise - // fails, mark-of-the-web is manually applied as a fallback. + // |remote_quarantine| must be connected to an instance of the Quarantine + // service. In the unexpected case that |remote_quarantine| is invalid, or the + // service otherwise fails, mark-of-the-web is manually applied as a fallback. virtual void RenameAndAnnotate( const base::FilePath& full_path, const std::string& client_guid, const GURL& source_url, const GURL& referrer_url, - std::unique_ptr<service_manager::Connector> connector, + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine, const RenameCompletionCallback& callback) = 0; // Detach the file so it is not deleted on destruction. diff --git a/chromium/components/download/public/common/download_file_impl.h b/chromium/components/download/public/common/download_file_impl.h index 1b33f39da91..59ef5d5ee4a 100644 --- a/chromium/components/download/public/common/download_file_impl.h +++ b/chromium/components/download/public/common/download_file_impl.h @@ -32,10 +32,6 @@ #include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/system/simple_watcher.h" -namespace service_manager { -class Connector; -} - namespace download { class DownloadDestinationObserver; @@ -68,12 +64,13 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFileImpl : public DownloadFile { int64_t length) override; void RenameAndUniquify(const base::FilePath& full_path, const RenameCompletionCallback& callback) override; - void RenameAndAnnotate(const base::FilePath& full_path, - const std::string& client_guid, - const GURL& source_url, - const GURL& referrer_url, - std::unique_ptr<service_manager::Connector> connector, - const RenameCompletionCallback& callback) override; + void RenameAndAnnotate( + const base::FilePath& full_path, + const std::string& client_guid, + const GURL& source_url, + const GURL& referrer_url, + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine, + const RenameCompletionCallback& callback) override; void Detach() override; void Cancel() override; void SetPotentialFileLength(int64_t length) override; @@ -234,7 +231,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFileImpl : public DownloadFile { std::string client_guid; // See BaseFile::AnnotateWithSourceInformation() GURL source_url; // See BaseFile::AnnotateWithSourceInformation() GURL referrer_url; // See BaseFile::AnnotateWithSourceInformation() - std::unique_ptr<service_manager::Connector> connector; + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine; int retries_left; // RenameWithRetryInternal() will // automatically retry until this // count reaches 0. Each attempt @@ -280,6 +277,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFileImpl : public DownloadFile { // Register callback and start to read data from the stream. void RegisterAndActivateStream(SourceStream* source_stream); + // Helper method to activate a stream and start reading from it. + void ActivateStream(SourceStream* source_stream); + // Called when a stream completes. void OnStreamCompleted(SourceStream* source_stream); diff --git a/chromium/components/download/public/common/download_item_factory.h b/chromium/components/download/public/common/download_item_factory.h index 8453cd85349..f89fb11f0fc 100644 --- a/chromium/components/download/public/common/download_item_factory.h +++ b/chromium/components/download/public/common/download_item_factory.h @@ -18,6 +18,7 @@ #include "base/optional.h" #include "components/download/public/common/download_export.h" #include "components/download/public/common/download_item.h" +#include "components/download/public/common/download_job.h" #include "url/origin.h" class GURL; @@ -31,7 +32,6 @@ namespace download { struct DownloadCreateInfo; class DownloadItemImpl; class DownloadItemImplDelegate; -class DownloadRequestHandleInterface; class COMPONENTS_DOWNLOAD_EXPORT DownloadItemFactory { public: @@ -77,7 +77,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemFactory { const base::FilePath& path, const GURL& url, const std::string& mime_type, - std::unique_ptr<DownloadRequestHandleInterface> request_handle) = 0; + download::DownloadJob::CancelRequestCallback cancel_request_callback) = 0; }; } // namespace download diff --git a/chromium/components/download/public/common/download_item_impl.h b/chromium/components/download/public/common/download_item_impl.h index 72e52e201a7..9df918424e7 100644 --- a/chromium/components/download/public/common/download_item_impl.h +++ b/chromium/components/download/public/common/download_item_impl.h @@ -23,21 +23,16 @@ #include "components/download/public/common/download_destination_observer.h" #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_job.h" #include "components/download/public/common/download_url_parameters.h" #include "components/download/public/common/resume_mode.h" +#include "components/download/public/common/url_loader_factory_provider.h" #include "url/gurl.h" #include "url/origin.h" -namespace net { -class URLRequestContextGetter; -} - namespace download { class DownloadFile; class DownloadItemImplDelegate; -class DownloadJob; // See download_item.h for usage. class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl @@ -206,13 +201,12 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl // Constructing for the "Save Page As..." feature: // |net_log| is constructed externally for our use. - DownloadItemImpl( - DownloadItemImplDelegate* delegate, - uint32_t id, - const base::FilePath& path, - const GURL& url, - const std::string& mime_type, - std::unique_ptr<DownloadRequestHandleInterface> request_handle); + DownloadItemImpl(DownloadItemImplDelegate* delegate, + uint32_t id, + const base::FilePath& path, + const GURL& url, + const std::string& mime_type, + DownloadJob::CancelRequestCallback cancel_request_callback); ~DownloadItemImpl() override; @@ -310,17 +304,16 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl // Start the download. // |download_file| is the associated file on the storage medium. - // |req_handle| is the new request handle associated with the download. + // |cancel_request_callback| is the callback to cancel the download. // |new_create_info| is a DownloadCreateInfo containing the new response // parameters. It may be different from the DownloadCreateInfo used to create // the DownloadItem if Start() is being called in response for a // download resumption request. virtual void Start(std::unique_ptr<DownloadFile> download_file, - std::unique_ptr<DownloadRequestHandleInterface> req_handle, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadCreateInfo& new_create_info, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter); + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider); // Needed because of intertwining with DownloadManagerImpl ------------------- diff --git a/chromium/components/download/public/common/download_item_impl_delegate.h b/chromium/components/download/public/common/download_item_impl_delegate.h index e67be44238b..975357cffac 100644 --- a/chromium/components/download/public/common/download_item_impl_delegate.h +++ b/chromium/components/download/public/common/download_item_impl_delegate.h @@ -14,6 +14,9 @@ #include "components/download/public/common/download_export.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/download_url_parameters.h" +#include "components/download/public/common/quarantine_connection.h" +#include "components/services/quarantine/public/mojom/quarantine.mojom.h" +#include "mojo/public/cpp/bindings/pending_remote.h" namespace service_manager { class Connector; @@ -120,6 +123,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImplDelegate { // Gets the ServiceManager connector that can be used on UI thread. virtual service_manager::Connector* GetServiceManagerConnector(); + // Gets a callback that can connect to the Quarantine Service if available. + virtual QuarantineConnectionCallback GetQuarantineConnectionCallback(); + private: // For "Outlives attached DownloadItemImpl" invariant assertion. int count_; diff --git a/chromium/components/download/public/common/download_job.h b/chromium/components/download/public/common/download_job.h index 0831b93fc4f..a4348310bf3 100644 --- a/chromium/components/download/public/common/download_job.h +++ b/chromium/components/download/public/common/download_job.h @@ -11,7 +11,6 @@ #include "components/download/public/common/download_export.h" #include "components/download/public/common/download_file.h" #include "components/download/public/common/download_interrupt_reasons.h" -#include "components/download/public/common/download_request_handle_interface.h" namespace download { @@ -22,8 +21,13 @@ class DownloadItem; // The base class is a friend class of DownloadItem. class COMPONENTS_DOWNLOAD_EXPORT DownloadJob { public: + // Callback to cancel the download request. + using CancelRequestCallback = + base::OnceCallback<void(bool /* user_cancel */)>; + CancelRequestCallback cancel_request_callback; + DownloadJob(DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> request_handle); + CancelRequestCallback cancel_request_callback); virtual ~DownloadJob(); // Download operations. @@ -66,9 +70,8 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadJob { DownloadItem* download_item_; - // Used to perform operations on network request. - // Can be null on interrupted download. - std::unique_ptr<DownloadRequestHandleInterface> request_handle_; + // Callback to cancel the download, can be null. + CancelRequestCallback cancel_request_callback_; private: // If the download progress is paused by the user. diff --git a/chromium/components/download/public/common/download_job_factory.h b/chromium/components/download/public/common/download_job_factory.h index d6660c1073c..0d51cdaf3c8 100644 --- a/chromium/components/download/public/common/download_job_factory.h +++ b/chromium/components/download/public/common/download_job_factory.h @@ -8,35 +8,29 @@ #include <memory> #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_export.h" - -namespace net { -class URLRequestContextGetter; -} // namespace net +#include "components/download/public/common/download_job.h" +#include "components/download/public/common/url_loader_factory_provider.h" namespace service_manager { class Connector; } // namespace service_manager namespace download { - class DownloadItem; -class DownloadJob; -class DownloadRequestHandleInterface; -class DownloadURLLoaderFactoryGetter; // Factory class to create different kinds of DownloadJob. class COMPONENTS_DOWNLOAD_EXPORT DownloadJobFactory { public: static std::unique_ptr<DownloadJob> CreateJob( DownloadItem* download_item, - std::unique_ptr<DownloadRequestHandleInterface> req_handle, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadCreateInfo& create_info, bool is_save_package_download, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, service_manager::Connector* connector); private: diff --git a/chromium/components/download/public/common/download_request_handle_interface.h b/chromium/components/download/public/common/download_request_handle_interface.h deleted file mode 100644 index 31678df4a4d..00000000000 --- a/chromium/components/download/public/common/download_request_handle_interface.h +++ /dev/null @@ -1,29 +0,0 @@ -// 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_DOWNLOAD_REQUEST_HANDLE_INTERFACE_H_ -#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_REQUEST_HANDLE_INTERFACE_H_ - -#include "base/compiler_specific.h" -#include "base/memory/weak_ptr.h" -#include "components/download/public/common/download_export.h" - -namespace download { - -// A handle used by the download system for operations on the network request. -class COMPONENTS_DOWNLOAD_EXPORT DownloadRequestHandleInterface { - public: - virtual ~DownloadRequestHandleInterface() = default; - - // Pauses or resumes the network request. - virtual void PauseRequest() = 0; - virtual void ResumeRequest() = 0; - - // Cancels the request. - virtual void CancelRequest(bool user_cancel) = 0; -}; - -} // namespace download - -#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_REQUEST_HANDLE_INTERFACE_H_ diff --git a/chromium/components/download/public/common/download_response_handler.h b/chromium/components/download/public/common/download_response_handler.h index 9b67e71af4b..08d93025b27 100644 --- a/chromium/components/download/public/common/download_response_handler.h +++ b/chromium/components/download/public/common/download_response_handler.h @@ -52,15 +52,14 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadResponseHandler const DownloadUrlParameters::RequestHeadersType& request_headers, const std::string& request_origin, DownloadSource download_source, - bool ignore_content_length_mismatch, std::vector<GURL> url_chain, bool is_background_mode); ~DownloadResponseHandler() override; // network::mojom::URLLoaderClient - void OnReceiveResponse(const network::ResourceResponseHead& head) override; + void OnReceiveResponse(network::mojom::URLResponseHeadPtr head) override; void OnReceiveRedirect(const net::RedirectInfo& redirect_info, - const network::ResourceResponseHead& head) override; + network::mojom::URLResponseHeadPtr head) override; void OnUploadProgress(int64_t current_position, int64_t total_size, OnUploadProgressCallback callback) override; @@ -97,7 +96,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadResponseHandler std::string request_origin_; DownloadSource download_source_; net::CertStatus cert_status_; - bool ignore_content_length_mismatch_; + bool has_strong_validators_; base::Optional<url::Origin> request_initiator_; bool is_partial_request_; bool completed_; diff --git a/chromium/components/download/public/common/download_stats.h b/chromium/components/download/public/common/download_stats.h index 9e72887e191..ffafffa18b8 100644 --- a/chromium/components/download/public/common/download_stats.h +++ b/chromium/components/download/public/common/download_stats.h @@ -267,24 +267,13 @@ COMPONENTS_DOWNLOAD_EXPORT void RecordDownloadInterrupted( int64_t total, bool is_parallelizable, bool is_parallel_download_enabled, - DownloadSource download_source, - bool post_content_length_mismatch); - -// Record that a download has been classified as malicious. -COMPONENTS_DOWNLOAD_EXPORT void RecordMaliciousDownloadClassified( - DownloadDangerType danger_type); + DownloadSource download_source); // Record a dangerous download accept event. COMPONENTS_DOWNLOAD_EXPORT void RecordDangerousDownloadAccept( DownloadDangerType danger_type, const base::FilePath& file_path); -// Record a dangerous download discard event. -COMPONENTS_DOWNLOAD_EXPORT void RecordDangerousDownloadDiscard( - DownloadDiscardReason reason, - DownloadDangerType danger_type, - const base::FilePath& file_path); - // Returns the type of download. COMPONENTS_DOWNLOAD_EXPORT DownloadContent DownloadContentFromMimeType(const std::string& mime_type_string, diff --git a/chromium/components/download/public/common/download_url_loader_factory_getter.h b/chromium/components/download/public/common/download_url_loader_factory_getter.h deleted file mode 100644 index c9aef992c76..00000000000 --- a/chromium/components/download/public/common/download_url_loader_factory_getter.h +++ /dev/null @@ -1,57 +0,0 @@ -// 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_DOWNLOAD_URL_LOADER_FACTORY_GETTER_H_ -#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_URL_LOADER_FACTORY_GETTER_H_ - -#include "base/macros.h" -#include "base/memory/ref_counted.h" -#include "base/sequenced_task_runner_helpers.h" -#include "components/download/public/common/download_export.h" - -namespace network { -class SharedURLLoaderFactory; -} // namespace network - -namespace download { - -struct DownloadURLLoaderFactoryGetterDeleter; - -// Class for retrieving a URLLoaderFactory on IO thread. This class can be -// created on any thread and will be destroyed on the IO thread. -// GetURLLoaderFactory() has to be called on the IO thread. -class COMPONENTS_DOWNLOAD_EXPORT DownloadURLLoaderFactoryGetter - : public base::RefCountedThreadSafe<DownloadURLLoaderFactoryGetter, - DownloadURLLoaderFactoryGetterDeleter> { - public: - DownloadURLLoaderFactoryGetter(); - - // Called on the IO thread to get a URLLoaderFactory. - virtual scoped_refptr<network::SharedURLLoaderFactory> - GetURLLoaderFactory() = 0; - - protected: - virtual ~DownloadURLLoaderFactoryGetter(); - - private: - friend class base::DeleteHelper<DownloadURLLoaderFactoryGetter>; - friend class base::RefCountedThreadSafe< - DownloadURLLoaderFactoryGetter, - DownloadURLLoaderFactoryGetterDeleter>; - friend struct DownloadURLLoaderFactoryGetterDeleter; - - void DeleteOnCorrectThread() const; - - DISALLOW_COPY_AND_ASSIGN(DownloadURLLoaderFactoryGetter); -}; - -struct DownloadURLLoaderFactoryGetterDeleter { - static void Destruct(const DownloadURLLoaderFactoryGetter* factory_getter) { - factory_getter->DeleteOnCorrectThread(); - } -}; - -} // namespace download - -#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_URL_LOADER_FACTORY_GETTER_H_ diff --git a/chromium/components/download/public/common/download_url_loader_factory_getter_impl.h b/chromium/components/download/public/common/download_url_loader_factory_getter_impl.h deleted file mode 100644 index dcfb250636c..00000000000 --- a/chromium/components/download/public/common/download_url_loader_factory_getter_impl.h +++ /dev/null @@ -1,39 +0,0 @@ -// 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_DOWNLOAD_URL_LOADER_FACTORY_GETTER_IMPL_H_ -#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_URL_LOADER_FACTORY_GETTER_IMPL_H_ - -#include "components/download/public/common/download_url_loader_factory_getter.h" -#include "services/network/public/cpp/shared_url_loader_factory.h" - -namespace download { - -// Class for retrieving a fixed SharedURLLoaderFactory. -class COMPONENTS_DOWNLOAD_EXPORT DownloadURLLoaderFactoryGetterImpl - : public DownloadURLLoaderFactoryGetter { - public: - explicit DownloadURLLoaderFactoryGetterImpl( - std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory); - - // download::DownloadURLLoaderFactoryGetter implementation. - scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override; - - protected: - ~DownloadURLLoaderFactoryGetterImpl() override; - - private: - // Only one of the following two members is ever set. Initially that would be - // |url_loader_factory_info_|, but after GetURLLoaderFactory is called for the - // first time instead |url_loader_factory_| will be set. This is safe because - // GetURLLoaderFactory is always called from the same thread. - std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory_info_; - scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; - - DISALLOW_COPY_AND_ASSIGN(DownloadURLLoaderFactoryGetterImpl); -}; - -} // namespace download - -#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_URL_LOADER_FACTORY_GETTER_IMPL_H_ diff --git a/chromium/components/download/public/common/download_url_parameters.cc b/chromium/components/download/public/common/download_url_parameters.cc index 0c85c8f6a9a..fd465914dc1 100644 --- a/chromium/components/download/public/common/download_url_parameters.cc +++ b/chromium/components/download/public/common/download_url_parameters.cc @@ -40,8 +40,7 @@ DownloadUrlParameters::DownloadUrlParameters( transient_(false), traffic_annotation_(traffic_annotation), download_source_(DownloadSource::UNKNOWN), - require_safety_checks_(true), - ignore_content_length_mismatch_(false) {} + require_safety_checks_(true) {} DownloadUrlParameters::~DownloadUrlParameters() = default; diff --git a/chromium/components/download/public/common/download_url_parameters.h b/chromium/components/download/public/common/download_url_parameters.h index ecbc0aff525..f276eddce7a 100644 --- a/chromium/components/download/public/common/download_url_parameters.h +++ b/chromium/components/download/public/common/download_url_parameters.h @@ -257,11 +257,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters { require_safety_checks_ = require_safety_checks; } - // Sets whether to ignore content length mismatch errors. - void set_ignore_content_length_mismatch(bool ignore_content_length_mismatch) { - ignore_content_length_mismatch_ = ignore_content_length_mismatch; - } - const OnStartedCallback& callback() const { return callback_; } bool content_initiated() const { return content_initiated_; } const std::string& last_modified() const { return last_modified_; } @@ -315,9 +310,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters { bool is_transient() const { return transient_; } std::string guid() const { return guid_; } bool require_safety_checks() const { return require_safety_checks_; } - bool ignore_content_length_mismatch() const { - return ignore_content_length_mismatch_; - } // STATE CHANGING: All save_info_ sub-objects will be in an indeterminate // state following this call. @@ -365,7 +357,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters { DownloadSource download_source_; UploadProgressCallback upload_callback_; bool require_safety_checks_; - bool ignore_content_length_mismatch_; DISALLOW_COPY_AND_ASSIGN(DownloadUrlParameters); }; diff --git a/chromium/components/download/public/common/download_utils.h b/chromium/components/download/public/common/download_utils.h index 69f4824e5f7..d48f61ae8ed 100644 --- a/chromium/components/download/public/common/download_utils.h +++ b/chromium/components/download/public/common/download_utils.h @@ -39,7 +39,7 @@ using URLSecurityPolicy = // |cert_status| is ignored if error_code is not net::ERR_ABORTED. COMPONENTS_DOWNLOAD_EXPORT DownloadInterruptReason HandleRequestCompletionStatus(net::Error error_code, - bool ignore_content_length_mismatch, + bool has_strong_validators, net::CertStatus cert_status, bool is_partial_request, DownloadInterruptReason abort_reason); diff --git a/chromium/components/download/public/common/in_progress_download_manager.h b/chromium/components/download/public/common/in_progress_download_manager.h index 4feb5060ceb..25d2058628b 100644 --- a/chromium/components/download/public/common/in_progress_download_manager.h +++ b/chromium/components/download/public/common/in_progress_download_manager.h @@ -14,33 +14,33 @@ #include "base/memory/scoped_refptr.h" #include "base/memory/weak_ptr.h" #include "base/optional.h" -#include "build/build_config.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/download_job.h" #include "components/download/public/common/download_utils.h" #include "components/download/public/common/simple_download_manager.h" #include "components/download/public/common/url_download_handler.h" #include "mojo/public/cpp/system/data_pipe.h" #include "url/gurl.h" -namespace net { -class URLRequestContextGetter; -} // namespace net - namespace network { struct ResourceResponse; +class SharedURLLoaderFactory; } // namespace network namespace service_manager { class Connector; } // namespace service_manager +namespace leveldb_proto { +class ProtoDatabaseProvider; +} // namespace leveldb_proto + namespace download { class DownloadDBCache; class DownloadStartObserver; -class DownloadURLLoaderFactoryGetter; class DownloadUrlParameters; struct DownloadDBEntry; @@ -78,16 +78,15 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager std::unique_ptr<DownloadCreateInfo> info, const DownloadUrlParameters::OnStartedCallback& on_started, StartDownloadItemCallback callback) {} - - // 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); }; using IsOriginSecureCallback = base::RepeatingCallback<bool(const GURL&)>; + // Creates a new InProgressDownloadManager instance. If |in_progress_db_dir| + // is empty then it will use an empty database and no history will be saved. + // |db_provider| can be nullptr if |in_progress_db_dir| is empty. InProgressDownloadManager(Delegate* delegate, const base::FilePath& in_progress_db_dir, + leveldb_proto::ProtoDatabaseProvider* db_provider, const IsOriginSecureCallback& is_origin_secure_cb, const URLSecurityPolicy& url_security_policy, service_manager::Connector* connector); @@ -101,13 +100,13 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager DownloadItem* GetDownloadByGuid(const std::string& guid) override; // Called to start a download. - void BeginDownload( - std::unique_ptr<DownloadUrlParameters> params, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, - bool is_new_download, - const GURL& site_url, - const GURL& tab_url, - const GURL& tab_referrer_url); + void BeginDownload(std::unique_ptr<DownloadUrlParameters> params, + std::unique_ptr<network::SharedURLLoaderFactoryInfo> + url_loader_factory_info, + bool is_new_download, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url); // Intercepts a download from navigation. void InterceptDownloadFromNavigation( @@ -122,12 +121,15 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager scoped_refptr<network::ResourceResponse> response_head, mojo::ScopedDataPipeConsumerHandle response_body, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter); + std::unique_ptr<network::SharedURLLoaderFactoryInfo> + url_loader_factory_info); void StartDownload( std::unique_ptr<DownloadCreateInfo> info, std::unique_ptr<InputStream> stream, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + DownloadJob::CancelRequestCallback cancel_request_callback, const DownloadUrlParameters::OnStartedCallback& on_started); // Shutting down the manager and stop all downloads. @@ -186,9 +188,9 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager } DownloadFileFactory* file_factory() { return file_factory_.get(); } - void set_url_loader_factory_getter( - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter) { - url_loader_factory_getter_ = std::move(url_loader_factory_getter); + void set_url_loader_factory( + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) { + url_loader_factory_ = url_loader_factory; } void SetDelegate(Delegate* delegate); @@ -203,13 +205,16 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager std::unique_ptr<download::DownloadItemImpl> download); private: - void Initialize(const base::FilePath& in_progress_db_dir); + void Initialize(const base::FilePath& in_progress_db_dir, + leveldb_proto::ProtoDatabaseProvider* db_provider); // 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, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + UrlDownloadHandler* downloader, const DownloadUrlParameters::OnStartedCallback& callback) override; void OnUrlDownloadStopped(UrlDownloadHandler* downloader) override; void OnUrlDownloadHandlerCreated( @@ -227,7 +232,9 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager // Start a DownloadItemImpl. void StartDownloadWithItem( std::unique_ptr<InputStream> stream, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + DownloadJob::CancelRequestCallback cancel_request_callback, std::unique_ptr<DownloadCreateInfo> info, DownloadItemImpl* download, bool should_persist_new_download); @@ -238,6 +245,9 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager // Called to notify |delegate_| that downloads are initialized. void NotifyDownloadsInitialized(); + // Cancels the given UrlDownloadHandler. + void CancelUrlDownload(UrlDownloadHandler* downloader, bool user_cancel); + // Active download handlers. std::vector<UrlDownloadHandler::UniqueUrlDownloadHandlerPtr> url_download_handlers_; @@ -282,9 +292,9 @@ class COMPONENTS_DOWNLOAD_EXPORT InProgressDownloadManager // A list of download GUIDs that should not be persisted. std::set<std::string> non_persistent_download_guids_; - // URLLoaderFactoryGetter for issuing network request when DownloadMangerImpl + // URLLoaderFactory for issuing network request when DownloadManagerImpl // is not available. - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter_; + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; // Mapping between download URIs and display names. // TODO(qinmin): move display name to history and in-progress DB. diff --git a/chromium/components/download/public/common/input_stream.cc b/chromium/components/download/public/common/input_stream.cc index 83558fa72fb..33390e7a479 100644 --- a/chromium/components/download/public/common/input_stream.cc +++ b/chromium/components/download/public/common/input_stream.cc @@ -10,6 +10,10 @@ InputStream::~InputStream() = default; void InputStream::Initialize() {} +bool InputStream::IsEmpty() { + return true; +} + void InputStream::RegisterDataReadyCallback( const mojo::SimpleWatcher::ReadyCallback& callback) {} @@ -17,4 +21,13 @@ void InputStream::ClearDataReadyCallback() {} void InputStream::RegisterCompletionCallback(base::OnceClosure callback) {} +InputStream::StreamState InputStream::Read(scoped_refptr<net::IOBuffer>* data, + size_t* length) { + return StreamState::EMPTY; +} + +DownloadInterruptReason InputStream::GetCompletionStatus() { + return DOWNLOAD_INTERRUPT_REASON_NONE; +} + } // namespace download diff --git a/chromium/components/download/public/common/input_stream.h b/chromium/components/download/public/common/input_stream.h index 938daba2eb4..87871ea8979 100644 --- a/chromium/components/download/public/common/input_stream.h +++ b/chromium/components/download/public/common/input_stream.h @@ -30,7 +30,7 @@ class COMPONENTS_DOWNLOAD_EXPORT InputStream { virtual void Initialize(); // Returns true if the input stream contains no data, or false otherwise. - virtual bool IsEmpty() = 0; + virtual bool IsEmpty(); // Register/clear callbacks when data become available. virtual void RegisterDataReadyCallback( @@ -42,11 +42,10 @@ class COMPONENTS_DOWNLOAD_EXPORT InputStream { // Reads data from the stream into |data|, |length| is the number of bytes // returned. - virtual StreamState Read(scoped_refptr<net::IOBuffer>* data, - size_t* length) = 0; + virtual StreamState Read(scoped_refptr<net::IOBuffer>* data, size_t* length); // Returns the completion status. - virtual DownloadInterruptReason GetCompletionStatus() = 0; + virtual DownloadInterruptReason GetCompletionStatus(); }; } // namespace download diff --git a/chromium/components/download/public/common/mock_download_file.h b/chromium/components/download/public/common/mock_download_file.h index b0a7d25a3aa..585223e493a 100644 --- a/chromium/components/download/public/common/mock_download_file.h +++ b/chromium/components/download/public/common/mock_download_file.h @@ -48,13 +48,14 @@ class MockDownloadFile : public DownloadFile { MOCK_METHOD2(RenameAndUniquify, void(const base::FilePath& full_path, const RenameCompletionCallback& callback)); - MOCK_METHOD6(RenameAndAnnotate, - void(const base::FilePath& full_path, - const std::string& client_guid, - const GURL& source_url, - const GURL& referrer_url, - std::unique_ptr<service_manager::Connector> connector, - const RenameCompletionCallback& callback)); + MOCK_METHOD6( + RenameAndAnnotate, + void(const base::FilePath& full_path, + const std::string& client_guid, + const GURL& source_url, + const GURL& referrer_url, + mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine, + const RenameCompletionCallback& callback)); MOCK_METHOD0(Detach, void()); MOCK_METHOD0(Cancel, void()); MOCK_METHOD1(SetPotentialFileLength, void(int64_t length)); 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 612485d1f7b..8da0c0f6f5b 100644 --- a/chromium/components/download/public/common/mock_download_item_impl.h +++ b/chromium/components/download/public/common/mock_download_item_impl.h @@ -14,7 +14,6 @@ #include "components/download/public/common/download_create_info.h" #include "components/download/public/common/download_file.h" #include "components/download/public/common/download_item_impl.h" -#include "components/download/public/common/download_request_handle_interface.h" #include "testing/gmock/include/gmock/gmock.h" #include "url/origin.h" @@ -51,17 +50,15 @@ class MockDownloadItemImpl : public DownloadItemImpl { NOTREACHED(); } MOCK_METHOD0(OnDownloadedFileRemoved, void()); - void Start( - std::unique_ptr<DownloadFile> download_file, - std::unique_ptr<DownloadRequestHandleInterface> req_handle, - const DownloadCreateInfo& create_info, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - net::URLRequestContextGetter* url_request_context_getter) override { - MockStart(download_file.get(), req_handle.get()); + void Start(std::unique_ptr<DownloadFile> download_file, + DownloadJob::CancelRequestCallback cancel_request_callback, + const DownloadCreateInfo& create_info, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider) override { + MockStart(download_file.get()); } - MOCK_METHOD2(MockStart, void(DownloadFile*, DownloadRequestHandleInterface*)); + MOCK_METHOD1(MockStart, void(DownloadFile*)); MOCK_METHOD0(Remove, void()); MOCK_CONST_METHOD1(TimeRemaining, bool(base::TimeDelta*)); diff --git a/chromium/components/download/public/common/quarantine_connection.h b/chromium/components/download/public/common/quarantine_connection.h new file mode 100644 index 00000000000..f646b29aa0c --- /dev/null +++ b/chromium/components/download/public/common/quarantine_connection.h @@ -0,0 +1,21 @@ +// Copyright 2019 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_QUARANTINE_CONNECTION_H_ +#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_QUARANTINE_CONNECTION_H_ + +#include "base/callback.h" +#include "components/services/quarantine/public/mojom/quarantine.mojom.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" + +namespace download { + +// A callback which can be used to acquire a connection to a Quarantine +// Service instance if available. +using QuarantineConnectionCallback = base::RepeatingCallback<void( + mojo::PendingReceiver<quarantine::mojom::Quarantine>)>; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_QUARANTINE_CONNECTION_H_ diff --git a/chromium/components/download/public/common/url_download_handler.h b/chromium/components/download/public/common/url_download_handler.h index 79725204200..c2d514914bd 100644 --- a/chromium/components/download/public/common/url_download_handler.h +++ b/chromium/components/download/public/common/url_download_handler.h @@ -5,14 +5,13 @@ #ifndef COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_DOWNLOAD_HANDLER_H_ #define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_DOWNLOAD_HANDLER_H_ +#include "base/memory/weak_ptr.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" +#include "components/download/public/common/url_loader_factory_provider.h" namespace download { struct DownloadCreateInfo; -class DownloadURLLoaderFactoryGetter; class InputStream; // Class for handling the download of a url. Implemented by child classes. @@ -26,7 +25,9 @@ class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadHandler { virtual void OnUrlDownloadStarted( std::unique_ptr<DownloadCreateInfo> download_create_info, std::unique_ptr<InputStream> input_stream, - scoped_refptr<DownloadURLLoaderFactoryGetter> url_loader_factory_getter, + URLLoaderFactoryProvider::URLLoaderFactoryProviderPtr + url_loader_factory_provider, + UrlDownloadHandler* downloader, 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 8b30691e11d..b20def2782e 100644 --- a/chromium/components/download/public/common/url_download_handler_factory.h +++ b/chromium/components/download/public/common/url_download_handler_factory.h @@ -9,16 +9,11 @@ #include "components/download/public/common/download_utils.h" #include "components/download/public/common/url_download_handler.h" -namespace net { -class URLRequestContextGetter; -} // namespace net - namespace service_manager { class Connector; } // namespace service_manager namespace download { -class DownloadURLLoaderFactoryGetter; class DownloadUrlParameters; // Class for handling the creation of a URLDownloadHandler. This is used to @@ -26,39 +21,15 @@ class DownloadUrlParameters; // TODO(qinmin): remove this factory once network service is fully enabled. class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadHandlerFactory { public: - // Installs a new factory for creating the URLDownloadHandler. Can be called - // on any thread. - static void Install(UrlDownloadHandlerFactory* factory); - // Creates a URLDownloadHandler. By default the handler is used for network // service. Must be called on the IO thread. static UrlDownloadHandler::UniqueUrlDownloadHandlerPtr Create( std::unique_ptr<download::DownloadUrlParameters> params, base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const URLSecurityPolicy& url_security_policy, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, std::unique_ptr<service_manager::Connector> connector, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); - - virtual ~UrlDownloadHandlerFactory(); - - protected: - UrlDownloadHandlerFactory(); - - // To be implemented by child classes to provide their own URLDownloadHandler - // instances. - virtual UrlDownloadHandler::UniqueUrlDownloadHandlerPtr - CreateUrlDownloadHandler( - std::unique_ptr<download::DownloadUrlParameters> params, - base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate, - scoped_refptr<download::DownloadURLLoaderFactoryGetter> - url_loader_factory_getter, - const URLSecurityPolicy& url_security_policy, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, - std::unique_ptr<service_manager::Connector> connector, - const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) = 0; }; } // namespace download diff --git a/chromium/components/download/public/common/url_download_request_handle.h b/chromium/components/download/public/common/url_download_request_handle.h index 46c77564efa..6d10c614a93 100644 --- a/chromium/components/download/public/common/url_download_request_handle.h +++ b/chromium/components/download/public/common/url_download_request_handle.h @@ -7,26 +7,19 @@ #include "base/memory/weak_ptr.h" #include "components/download/public/common/download_export.h" -#include "components/download/public/common/download_request_handle_interface.h" #include "components/download/public/common/url_download_handler.h" namespace download { // Implementation of the DownloadRequestHandleInterface to handle url download. -class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadRequestHandle - : public DownloadRequestHandleInterface { +class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadRequestHandle { public: UrlDownloadRequestHandle( base::WeakPtr<UrlDownloadHandler> downloader, scoped_refptr<base::SequencedTaskRunner> downloader_task_runner); UrlDownloadRequestHandle(UrlDownloadRequestHandle&& other); UrlDownloadRequestHandle& operator=(UrlDownloadRequestHandle&& other); - ~UrlDownloadRequestHandle() override; - - // DownloadRequestHandleInterface - void PauseRequest() override; - void ResumeRequest() override; - void CancelRequest(bool user_cancel) override; + ~UrlDownloadRequestHandle(); private: base::WeakPtr<UrlDownloadHandler> downloader_; diff --git a/chromium/components/download/public/common/url_loader_factory_provider.h b/chromium/components/download/public/common/url_loader_factory_provider.h new file mode 100644 index 00000000000..ae04d8f4f8f --- /dev/null +++ b/chromium/components/download/public/common/url_loader_factory_provider.h @@ -0,0 +1,40 @@ +// Copyright 2019 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_URL_LOADER_FACTORY_PROVIDER_H_ +#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_LOADER_FACTORY_PROVIDER_H_ + +#include "base/sequenced_task_runner.h" +#include "components/download/public/common/download_export.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" + +namespace download { + +// Class for wrapping a SharedURLLoaderFactory that can be passed across thread +// so that it can be used later on IO thread to retrieve the factory to create +// parallel download requests. +class COMPONENTS_DOWNLOAD_EXPORT URLLoaderFactoryProvider { + public: + using URLLoaderFactoryProviderPtr = + std::unique_ptr<URLLoaderFactoryProvider, base::OnTaskRunnerDeleter>; + + explicit URLLoaderFactoryProvider( + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory); + virtual ~URLLoaderFactoryProvider(); + + // Called on the io thread to get the URL loader. + scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory(); + + // Helper method to get an null ptr. + static URLLoaderFactoryProviderPtr GetNullPtr(); + + private: + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; + + DISALLOW_COPY_AND_ASSIGN(URLLoaderFactoryProvider); +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_LOADER_FACTORY_PROVIDER_H_ |