diff options
Diffstat (limited to 'chromium/components/download/public')
21 files changed, 506 insertions, 81 deletions
diff --git a/chromium/components/download/public/common/BUILD.gn b/chromium/components/download/public/common/BUILD.gn index ef5b2063428..e8edc1a31cd 100644 --- a/chromium/components/download/public/common/BUILD.gn +++ b/chromium/components/download/public/common/BUILD.gn @@ -43,6 +43,8 @@ component("public") { "download_response_handler.h", "download_save_info.cc", "download_save_info.h", + "download_schedule.cc", + "download_schedule.h", "download_source.h", "download_start_observer.h", "download_stats.h", @@ -94,6 +96,10 @@ component("public") { "//components/download/internal/common:internal", "//components/download/database", ] + + if (is_win && is_component_build) { + ldflags = [ "/IGNORE:4217" ] + } } if (is_android) { @@ -156,7 +162,10 @@ source_set("test_support") { source_set("unit_tests") { testonly = true - sources = [ "auto_resumption_handler_unittest.cc" ] + sources = [ + "auto_resumption_handler_unittest.cc", + "download_schedule_unittest.cc", + ] deps = [ ":public", diff --git a/chromium/components/download/public/common/auto_resumption_handler.cc b/chromium/components/download/public/common/auto_resumption_handler.cc index 9cfdb609737..1b73224daa6 100644 --- a/chromium/components/download/public/common/auto_resumption_handler.cc +++ b/chromium/components/download/public/common/auto_resumption_handler.cc @@ -4,15 +4,22 @@ #include "components/download/public/common/auto_resumption_handler.h" +#include <memory> +#include <utility> #include <vector> #include "base/bind.h" #include "base/metrics/field_trial_params.h" #include "base/strings/string_number_conversions.h" #include "base/threading/thread_task_runner_handle.h" +#include "base/time/clock.h" +#include "base/time/time.h" +#include "components/download/public/common/download_item.h" #include "components/download/public/task/task_scheduler.h" +#include "services/network/public/cpp/network_connection_tracker.h" #include "url/gurl.h" +namespace download { namespace { static download::AutoResumptionHandler* g_auto_resumption_handler = nullptr; @@ -42,22 +49,8 @@ const int64_t kWindowStartTimeSeconds = 0; // The window end time before which the system should fire the task. const int64_t kWindowEndTimeSeconds = 24 * 60 * 60; -bool IsMetered(network::mojom::ConnectionType type) { - switch (type) { - case network::mojom::ConnectionType::CONNECTION_2G: - case network::mojom::ConnectionType::CONNECTION_3G: - case network::mojom::ConnectionType::CONNECTION_4G: - return true; - case network::mojom::ConnectionType::CONNECTION_ETHERNET: - case network::mojom::ConnectionType::CONNECTION_WIFI: - case network::mojom::ConnectionType::CONNECTION_UNKNOWN: - case network::mojom::ConnectionType::CONNECTION_NONE: - case network::mojom::ConnectionType::CONNECTION_BLUETOOTH: - return false; - } - NOTREACHED(); - return false; -} +// The window length for download later task. +const int64_t kDownloadLaterTaskWindowSeconds = 15; /* 15 seconds.*/ bool IsConnected(network::mojom::ConnectionType type) { switch (type) { @@ -72,8 +65,6 @@ bool IsConnected(network::mojom::ConnectionType type) { } // namespace -namespace download { - AutoResumptionHandler::Config::Config() : auto_resumption_size_limit(0), is_auto_resumption_enabled_in_native(false) {} @@ -82,10 +73,12 @@ AutoResumptionHandler::Config::Config() void AutoResumptionHandler::Create( std::unique_ptr<download::NetworkStatusListener> network_listener, std::unique_ptr<download::TaskManager> task_manager, - std::unique_ptr<Config> config) { + std::unique_ptr<Config> config, + base::Clock* clock) { DCHECK(!g_auto_resumption_handler); g_auto_resumption_handler = new AutoResumptionHandler( - std::move(network_listener), std::move(task_manager), std::move(config)); + std::move(network_listener), std::move(task_manager), std::move(config), + clock); } // static @@ -96,10 +89,12 @@ AutoResumptionHandler* AutoResumptionHandler::Get() { AutoResumptionHandler::AutoResumptionHandler( std::unique_ptr<download::NetworkStatusListener> network_listener, std::unique_ptr<download::TaskManager> task_manager, - std::unique_ptr<Config> config) + std::unique_ptr<Config> config, + base::Clock* clock) : network_listener_(std::move(network_listener)), task_manager_(std::move(task_manager)), - config_(std::move(config)) { + config_(std::move(config)), + clock_(clock) { network_listener_->Start(this); } @@ -126,7 +121,8 @@ void AutoResumptionHandler::SetResumableDownloads( } bool AutoResumptionHandler::IsActiveNetworkMetered() const { - return IsMetered(network_listener_->GetConnectionType()); + return network::NetworkConnectionTracker::IsConnectionCellular( + network_listener_->GetConnectionType()); } void AutoResumptionHandler::OnNetworkChanged( @@ -151,7 +147,7 @@ void AutoResumptionHandler::OnDownloadUpdated(download::DownloadItem* item) { resumable_downloads_.erase(item->GetGuid()); if (item->GetState() == download::DownloadItem::INTERRUPTED && - IsAutoResumableDownload(item) && SatisfiesNetworkRequirements(item)) { + IsAutoResumableDownload(item) && ShouldResumeNow(item)) { downloads_to_retry_.insert(item); base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, @@ -165,6 +161,7 @@ void AutoResumptionHandler::OnDownloadUpdated(download::DownloadItem* item) { void AutoResumptionHandler::OnDownloadRemoved(download::DownloadItem* item) { resumable_downloads_.erase(item->GetGuid()); + downloads_to_retry_.erase(item); RecomputeTaskParams(); } @@ -174,8 +171,11 @@ void AutoResumptionHandler::OnDownloadDestroyed(download::DownloadItem* item) { } void AutoResumptionHandler::ResumeDownloadImmediately() { + if (!config_->is_auto_resumption_enabled_in_native) + return; + for (auto* download : std::move(downloads_to_retry_)) { - if (SatisfiesNetworkRequirements(download)) + if (ShouldResumeNow(download)) download->Resume(false); else RecomputeTaskParams(); @@ -184,13 +184,14 @@ void AutoResumptionHandler::ResumeDownloadImmediately() { } void AutoResumptionHandler::OnStartScheduledTask( + DownloadTaskType type, download::TaskFinishedCallback callback) { - task_manager_->OnStartScheduledTask(kResumptionTaskType, std::move(callback)); + task_manager_->OnStartScheduledTask(type, std::move(callback)); ResumePendingDownloads(); } -bool AutoResumptionHandler::OnStopScheduledTask() { - task_manager_->OnStopScheduledTask(kResumptionTaskType); +bool AutoResumptionHandler::OnStopScheduledTask(DownloadTaskType type) { + task_manager_->OnStopScheduledTask(type); RescheduleTaskIfNecessary(); return false; } @@ -207,6 +208,12 @@ void AutoResumptionHandler::RecomputeTaskParams() { kBatchDownloadUpdatesInterval); } +// Go through all the downloads. +// 1- If there is no immediately resumable downloads, finish the task +// 2- If there are resumable downloads, schedule a task +// 3- If there are no resumable downloads, unschedule the task. +// At any point either a task is running or is scheduled but not both, which is +// handled by TaskManager. void AutoResumptionHandler::RescheduleTaskIfNecessary() { if (!config_->is_auto_resumption_enabled_in_native) return; @@ -216,21 +223,33 @@ void AutoResumptionHandler::RescheduleTaskIfNecessary() { bool has_resumable_downloads = false; bool has_actionable_downloads = false; bool can_download_on_metered = false; + + std::vector<DownloadItem*> download_later_items; + auto now = clock_->Now(); + for (auto iter = resumable_downloads_.begin(); iter != resumable_downloads_.end(); ++iter) { download::DownloadItem* download = iter->second; if (!IsAutoResumableDownload(download)) continue; + if (ShouldDownloadLater(download, now)) { + download_later_items.push_back(download); + continue; + } + has_resumable_downloads = true; - has_actionable_downloads |= SatisfiesNetworkRequirements(download); + has_actionable_downloads |= ShouldResumeNow(download); can_download_on_metered |= download->AllowMetered(); - if (can_download_on_metered) - break; } - if (!has_actionable_downloads) + if (!has_actionable_downloads) { task_manager_->NotifyTaskFinished(kResumptionTaskType, false); + task_manager_->NotifyTaskFinished(DownloadTaskType::DOWNLOAD_LATER_TASK, + false); + } + + RescheduleDownloadLaterTask(download_later_items); if (!has_resumable_downloads) { task_manager_->UnscheduleTask(kResumptionTaskType); @@ -248,27 +267,45 @@ void AutoResumptionHandler::ResumePendingDownloads() { if (!config_->is_auto_resumption_enabled_in_native) return; - for (auto iter = resumable_downloads_.begin(); - iter != resumable_downloads_.end(); ++iter) { - download::DownloadItem* download = iter->second; + int resumed = MaybeResumeDownloads(resumable_downloads_); + + // If we resume nothing, finish the current task and reschedule. + if (!resumed) + RecomputeTaskParams(); +} + +int AutoResumptionHandler::MaybeResumeDownloads( + const std::map<std::string, DownloadItem*>& downloads) { + int resumed = 0; + for (const auto& pair : downloads) { + DownloadItem* download = pair.second; if (!IsAutoResumableDownload(download)) continue; - if (SatisfiesNetworkRequirements(download)) + if (ShouldResumeNow(download)) { download->Resume(false); + resumed++; + } } + + return resumed; } -bool AutoResumptionHandler::SatisfiesNetworkRequirements( - download::DownloadItem* download) { +bool AutoResumptionHandler::ShouldResumeNow( + download::DownloadItem* download) const { if (!IsConnected(network_listener_->GetConnectionType())) return false; + // If the user selects a time to start in the future, don't resume now. + if (ShouldDownloadLater(download, clock_->Now())) { + return false; + } + return download->AllowMetered() || !IsActiveNetworkMetered(); } bool AutoResumptionHandler::IsAutoResumableDownload( - download::DownloadItem* item) { + download::DownloadItem* item) const { if (!item || item->IsDangerous()) return false; @@ -290,6 +327,51 @@ bool AutoResumptionHandler::IsAutoResumableDownload( } // static +bool AutoResumptionHandler::ShouldDownloadLater(DownloadItem* item, + base::Time now) { + const auto& download_schedule = item->GetDownloadSchedule(); + if (download_schedule && + download_schedule->start_time().value_or(base::Time()) > now) { + return true; + } + + return false; +} + +void AutoResumptionHandler::RescheduleDownloadLaterTask( + const std::vector<DownloadItem*> downloads) { + base::Time window_start = base::Time::Max(); + for (auto* download : downloads) { + const auto schedule = download->GetDownloadSchedule(); + if (!schedule || !schedule->start_time().has_value()) + continue; + + if (schedule->start_time().value() < window_start) + window_start = schedule->start_time().value(); + } + + base::Time now = clock_->Now(); + if (window_start.is_max() || window_start < now) { + // Unschedule download later task, nothing to schedule. + task_manager_->UnscheduleTask(DownloadTaskType::DOWNLOAD_LATER_TASK); + } else { + // Fulfill the user scheduled time. + TaskManager::TaskParams task_params; + task_params.window_start_time_seconds = (window_start - now).InSeconds(); + task_params.window_end_time_seconds = + task_params.window_start_time_seconds + kDownloadLaterTaskWindowSeconds; + task_params.require_charging = false; + task_params.require_unmetered_network = false; + + // Needs to call |UnscheduleTask| first to make |task_manager_| set + // needs_reschedule to false. + task_manager_->UnscheduleTask(DownloadTaskType::DOWNLOAD_LATER_TASK); + task_manager_->ScheduleTask(DownloadTaskType::DOWNLOAD_LATER_TASK, + task_params); + } +} + +// static bool AutoResumptionHandler::IsInterruptedDownloadAutoResumable( download::DownloadItem* download_item, int auto_resumption_size_limit) { @@ -303,6 +385,9 @@ bool AutoResumptionHandler::IsInterruptedDownloadAutoResumable( if (download_item->GetBytesWasted() > auto_resumption_size_limit) return false; + if (download_item->GetTargetFilePath().empty()) + return false; + int interrupt_reason = download_item->GetLastReason(); DCHECK_NE(interrupt_reason, download::DOWNLOAD_INTERRUPT_REASON_NONE); return interrupt_reason == diff --git a/chromium/components/download/public/common/auto_resumption_handler.h b/chromium/components/download/public/common/auto_resumption_handler.h index 955f8aa8d6d..c6f4f1d0a86 100644 --- a/chromium/components/download/public/common/auto_resumption_handler.h +++ b/chromium/components/download/public/common/auto_resumption_handler.h @@ -18,6 +18,10 @@ #include "components/download/public/common/download_item.h" #include "components/download/public/task/task_manager.h" +namespace base { +class Clock; +} // namespace base + namespace download { // Handles auto-resumptions for downloads. Listens to network changes and @@ -38,7 +42,8 @@ class COMPONENTS_DOWNLOAD_EXPORT AutoResumptionHandler static void Create( std::unique_ptr<download::NetworkStatusListener> network_listener, std::unique_ptr<download::TaskManager> task_manager, - std::unique_ptr<Config> config); + std::unique_ptr<Config> config, + base::Clock* clock); // Returns the singleton instance of the AutoResumptionHandler, or nullptr if // initialization is not yet complete. @@ -53,14 +58,16 @@ class COMPONENTS_DOWNLOAD_EXPORT AutoResumptionHandler AutoResumptionHandler( std::unique_ptr<download::NetworkStatusListener> network_listener, std::unique_ptr<download::TaskManager> task_manager, - std::unique_ptr<Config> config); + std::unique_ptr<Config> config, + base::Clock* clock); ~AutoResumptionHandler() override; void SetResumableDownloads( const std::vector<download::DownloadItem*>& downloads); bool IsActiveNetworkMetered() const; - void OnStartScheduledTask(download::TaskFinishedCallback callback); - bool OnStopScheduledTask(); + void OnStartScheduledTask(DownloadTaskType type, + TaskFinishedCallback callback); + bool OnStopScheduledTask(DownloadTaskType type); void OnDownloadStarted(download::DownloadItem* item); @@ -70,25 +77,42 @@ class COMPONENTS_DOWNLOAD_EXPORT AutoResumptionHandler void OnDownloadDestroyed(download::DownloadItem* item) override; private: + using DownloadMap = std::map<std::string, DownloadItem*>; + // NetworkStatusListener::Observer implementation. void OnNetworkChanged(network::mojom::ConnectionType type) override; void ResumePendingDownloads(); + + // Maybe resume some of the |downloads|. Returns the number of downloads + // resumed. + int MaybeResumeDownloads(const DownloadMap& downloads); + void RecomputeTaskParams(); void RescheduleTaskIfNecessary(); void ResumeDownloadImmediately(); - bool SatisfiesNetworkRequirements(download::DownloadItem* download); - bool IsAutoResumableDownload(download::DownloadItem* item); + bool ShouldResumeNow(download::DownloadItem* download) const; + bool IsAutoResumableDownload(download::DownloadItem* item) const; + // Returns whether the user has scheduled the download to happen later. + static bool ShouldDownloadLater(DownloadItem* item, base::Time now); + + // Reschedule the download later background task. May cancel the task when no + // need to run a future task. + void RescheduleDownloadLaterTask(const std::vector<DownloadItem*> downloads); + + // Listens to network events to stop/resume downloads accordingly. std::unique_ptr<download::NetworkStatusListener> network_listener_; std::unique_ptr<download::TaskManager> task_manager_; std::unique_ptr<Config> config_; + base::Clock* clock_; + // List of downloads that are auto-resumable. These will be resumed as soon as // network conditions becomes favorable. - std::map<std::string, download::DownloadItem*> resumable_downloads_; + DownloadMap resumable_downloads_; // A temporary list of downloads which are being retried immediately. std::set<download::DownloadItem*> downloads_to_retry_; diff --git a/chromium/components/download/public/common/auto_resumption_handler_unittest.cc b/chromium/components/download/public/common/auto_resumption_handler_unittest.cc index f728dd504f4..9951bfb11cd 100644 --- a/chromium/components/download/public/common/auto_resumption_handler_unittest.cc +++ b/chromium/components/download/public/common/auto_resumption_handler_unittest.cc @@ -9,22 +9,52 @@ #include "base/bind.h" #include "base/callback.h" #include "base/guid.h" +#include "base/optional.h" +#include "base/test/simple_test_clock.h" #include "base/test/test_mock_time_task_runner.h" #include "base/threading/thread_task_runner_handle.h" #include "components/download/network/network_status_listener_impl.h" +#include "components/download/public/common/download_schedule.h" #include "components/download/public/common/mock_download_item.h" #include "components/download/public/task/mock_task_manager.h" #include "services/network/test/test_network_connection_tracker.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +using TaskParams = download::TaskManager::TaskParams; using network::mojom::ConnectionType; using testing::_; using testing::NiceMock; using testing::Return; +using testing::ReturnRef; using testing::ReturnRefOfCopy; namespace download { +namespace { + +const char kNow[] = "1 Sep 2020 01:00:00 GMT"; +const DownloadTaskType kResumptionTaskType = + DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK; +const DownloadTaskType kDownloadLaterTaskType = + DownloadTaskType::DOWNLOAD_LATER_TASK; +const int64_t kDownloadLaterTaskWindowSeconds = 15; + +TaskManager::TaskParams TaskParams(int64_t window_start_time_seconds, + int64_t window_end_time_seconds, + bool require_wifi) { + TaskManager::TaskParams params; + params.window_start_time_seconds = window_start_time_seconds; + params.window_end_time_seconds = window_end_time_seconds; + params.require_unmetered_network = require_wifi; + return params; +} + +base::Time GetNow() { + base::Time now; + bool success = base::Time::FromString(kNow, &now); + EXPECT_TRUE(success); + return now; +} class AutoResumptionHandlerTest : public testing::Test { public: @@ -39,14 +69,14 @@ class AutoResumptionHandlerTest : public testing::Test { network::TestNetworkConnectionTracker::GetInstance()); auto task_manager = std::make_unique<download::test::MockTaskManager>(); task_manager_ = task_manager.get(); - auto config = std::make_unique<AutoResumptionHandler::Config>(); config->auto_resumption_size_limit = 100; config->is_auto_resumption_enabled_in_native = true; + clock_.SetNow(GetNow()); auto_resumption_handler_ = std::make_unique<AutoResumptionHandler>( - std::move(network_listener), std::move(task_manager), - std::move(config)); + std::move(network_listener), std::move(task_manager), std::move(config), + &clock_); std::vector<DownloadItem*> empty_list; auto_resumption_handler_->SetResumableDownloads(empty_list); @@ -58,19 +88,26 @@ class AutoResumptionHandlerTest : public testing::Test { void SetDownloadState(MockDownloadItem* download, DownloadItem::DownloadState state, bool paused, - bool metered) { + bool allow_metered, + bool has_target_file_path = true) { ON_CALL(*download, GetGuid()) .WillByDefault(ReturnRefOfCopy(base::GenerateGUID())); ON_CALL(*download, GetURL()) .WillByDefault(ReturnRefOfCopy(GURL("http://example.com/foo"))); ON_CALL(*download, GetState()).WillByDefault(Return(state)); ON_CALL(*download, IsPaused()).WillByDefault(Return(paused)); - ON_CALL(*download, AllowMetered()).WillByDefault(Return(metered)); + ON_CALL(*download, AllowMetered()).WillByDefault(Return(allow_metered)); + ON_CALL(*download, GetTargetFilePath()) + .WillByDefault(ReturnRefOfCopy( + has_target_file_path ? base::FilePath(FILE_PATH_LITERAL("a.txt")) + : base::FilePath())); auto last_reason = state == DownloadItem::INTERRUPTED ? download::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED : download::DOWNLOAD_INTERRUPT_REASON_NONE; ON_CALL(*download, GetLastReason()).WillByDefault(Return(last_reason)); + ON_CALL(*download, GetDownloadSchedule()) + .WillByDefault(ReturnRefOfCopy(base::Optional<DownloadSchedule>())); } void SetNetworkConnectionType(ConnectionType connection_type) { @@ -78,10 +115,18 @@ class AutoResumptionHandlerTest : public testing::Test { connection_type); } + void SetDownloadSchedule(MockDownloadItem* download, + DownloadSchedule download_schedule) { + base::Optional<DownloadSchedule> copy = download_schedule; + ON_CALL(*download, GetDownloadSchedule()) + .WillByDefault(ReturnRefOfCopy(copy)); + } + scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; base::ThreadTaskRunnerHandle handle_; download::test::MockTaskManager* task_manager_; std::unique_ptr<AutoResumptionHandler> auto_resumption_handler_; + base::SimpleTestClock clock_; DISALLOW_COPY_AND_ASSIGN(AutoResumptionHandlerTest); }; @@ -107,8 +152,11 @@ TEST_F(AutoResumptionHandlerTest, TaskFinishedCalledOnDownloadCompletion) { task_runner_->FastForwardUntilNoTasksRemain(); // Complete the download. - EXPECT_CALL(*task_manager_, NotifyTaskFinished(_, _)).Times(1); - EXPECT_CALL(*task_manager_, UnscheduleTask(_)).Times(1); + EXPECT_CALL(*task_manager_, NotifyTaskFinished(kResumptionTaskType, _)); + EXPECT_CALL(*task_manager_, + NotifyTaskFinished(kDownloadLaterTaskType, false)); + EXPECT_CALL(*task_manager_, UnscheduleTask(kResumptionTaskType)); + EXPECT_CALL(*task_manager_, UnscheduleTask(kDownloadLaterTaskType)); SetDownloadState(item.get(), DownloadItem::COMPLETE, false, false); auto_resumption_handler_->OnDownloadUpdated(item.get()); task_runner_->FastForwardUntilNoTasksRemain(); @@ -126,7 +174,9 @@ TEST_F(AutoResumptionHandlerTest, TaskFinishedCalledOnDownloadRemoved) { task_runner_->FastForwardUntilNoTasksRemain(); // Remove the download. - EXPECT_CALL(*task_manager_, NotifyTaskFinished(_, _)).Times(1); + EXPECT_CALL(*task_manager_, NotifyTaskFinished(kResumptionTaskType, _)); + EXPECT_CALL(*task_manager_, + NotifyTaskFinished(kDownloadLaterTaskType, false)); SetDownloadState(item.get(), DownloadItem::COMPLETE, false, false); auto_resumption_handler_->OnDownloadRemoved(item.get()); task_runner_->FastForwardUntilNoTasksRemain(); @@ -148,15 +198,19 @@ TEST_F(AutoResumptionHandlerTest, MultipleDownloads) { task_runner_->FastForwardUntilNoTasksRemain(); // Finish item1. The task should still be running. - EXPECT_CALL(*task_manager_, UnscheduleTask(_)).Times(0); + EXPECT_CALL(*task_manager_, UnscheduleTask(kResumptionTaskType)).Times(0); + EXPECT_CALL(*task_manager_, UnscheduleTask(kDownloadLaterTaskType)); EXPECT_CALL(*task_manager_, NotifyTaskFinished(_, _)).Times(0); SetDownloadState(item1.get(), DownloadItem::CANCELLED, false, false); auto_resumption_handler_->OnDownloadUpdated(item1.get()); task_runner_->FastForwardUntilNoTasksRemain(); // Finish item2. The task should now complete. - EXPECT_CALL(*task_manager_, UnscheduleTask(_)).Times(1); - EXPECT_CALL(*task_manager_, NotifyTaskFinished(_, _)).Times(1); + EXPECT_CALL(*task_manager_, UnscheduleTask(kResumptionTaskType)); + EXPECT_CALL(*task_manager_, UnscheduleTask(kDownloadLaterTaskType)); + EXPECT_CALL(*task_manager_, NotifyTaskFinished(kResumptionTaskType, _)); + EXPECT_CALL(*task_manager_, + NotifyTaskFinished(kDownloadLaterTaskType, false)); SetDownloadState(item2.get(), DownloadItem::COMPLETE, false, false); auto_resumption_handler_->OnDownloadUpdated(item2.get()); task_runner_->FastForwardUntilNoTasksRemain(); @@ -221,7 +275,70 @@ TEST_F(AutoResumptionHandlerTest, // Start the task. It should resume all downloads. EXPECT_CALL(*item.get(), Resume(_)).Times(1); TaskFinishedCallback callback; - auto_resumption_handler_->OnStartScheduledTask(std::move(callback)); + auto_resumption_handler_->OnStartScheduledTask( + DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK, std::move(callback)); + task_runner_->FastForwardUntilNoTasksRemain(); +} + +TEST_F(AutoResumptionHandlerTest, DownloadWithoutTargetPathNotAutoResumed) { + SetNetworkConnectionType(ConnectionType::CONNECTION_WIFI); + auto item = std::make_unique<NiceMock<MockDownloadItem>>(); + SetDownloadState(item.get(), DownloadItem::INTERRUPTED, false, false, false); + auto_resumption_handler_->OnDownloadStarted(item.get()); + task_runner_->FastForwardUntilNoTasksRemain(); + + EXPECT_CALL(*item.get(), Resume(_)).Times(0); + auto_resumption_handler_->OnStartScheduledTask( + DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK, base::DoNothing()); task_runner_->FastForwardUntilNoTasksRemain(); } + +// Download scheduled to start in the future should not be auto resumed now. +TEST_F(AutoResumptionHandlerTest, DownloadLaterStartFutureNotAutoResumed) { + SetNetworkConnectionType(ConnectionType::CONNECTION_WIFI); + auto item = std::make_unique<NiceMock<MockDownloadItem>>(); + auto delta = base::TimeDelta::FromDays(10); + base::Time future_time = GetNow() + delta; + SetDownloadState(item.get(), DownloadItem::INTERRUPTED, false, false); + SetDownloadSchedule(item.get(), + DownloadSchedule(false /*only_on_wifi*/, future_time)); + + int64_t window_start = delta.InSeconds(); + auto task_params = TaskParams( + window_start, window_start + kDownloadLaterTaskWindowSeconds, false); + EXPECT_CALL(*item.get(), Resume(_)).Times(0); + EXPECT_CALL(*task_manager_, + ScheduleTask(kDownloadLaterTaskType, task_params)); + EXPECT_CALL(*task_manager_, ScheduleTask(kResumptionTaskType, _)).Times(0); + EXPECT_CALL(*task_manager_, UnscheduleTask(kResumptionTaskType)); + EXPECT_CALL(*task_manager_, UnscheduleTask(kDownloadLaterTaskType)); + EXPECT_CALL(*task_manager_, NotifyTaskFinished(kDownloadLaterTaskType, _)); + EXPECT_CALL(*task_manager_, NotifyTaskFinished(kResumptionTaskType, _)); + + auto_resumption_handler_->OnDownloadStarted(item.get()); + auto_resumption_handler_->OnStartScheduledTask( + DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK, base::DoNothing()); + task_runner_->FastForwardUntilNoTasksRemain(); +} + +// Use DownloadItem::AllowMetered() instead of DownloadSchedule::only_on_wifi() +// to determine network condition for download later. +TEST_F(AutoResumptionHandlerTest, DownloadLaterMeteredAutoResumed) { + SetNetworkConnectionType(ConnectionType::CONNECTION_3G); + auto item = std::make_unique<NiceMock<MockDownloadItem>>(); + SetDownloadState(item.get(), DownloadItem::INTERRUPTED, false, + true /*allow_metered*/); + SetDownloadSchedule(item.get(), + DownloadSchedule(true /*only_on_wifi*/, base::nullopt)); + + auto_resumption_handler_->OnDownloadStarted(item.get()); + task_runner_->FastForwardUntilNoTasksRemain(); + + EXPECT_CALL(*item.get(), Resume(_)); + auto_resumption_handler_->OnStartScheduledTask( + DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK, base::DoNothing()); + task_runner_->FastForwardUntilNoTasksRemain(); +} + +} // namespace } // namespace download diff --git a/chromium/components/download/public/common/base_file.h b/chromium/components/download/public/common/base_file.h index 5a80471d12d..67893e9a986 100644 --- a/chromium/components/download/public/common/base_file.h +++ b/chromium/components/download/public/common/base_file.h @@ -13,10 +13,10 @@ #include "base/callback.h" #include "base/callback_forward.h" +#include "base/check.h" #include "base/files/file.h" #include "base/files/file_path.h" #include "base/gtest_prod_util.h" -#include "base/logging.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/optional.h" diff --git a/chromium/components/download/public/common/download_features.cc b/chromium/components/download/public/common/download_features.cc index 6b17ba5fa24..41ec09e2715 100644 --- a/chromium/components/download/public/common/download_features.cc +++ b/chromium/components/download/public/common/download_features.cc @@ -30,10 +30,10 @@ const base::Feature kParallelDownloading { #endif }; -#if defined(OS_ANDROID) const base::Feature kDownloadLater{"DownloadLater", base::FEATURE_DISABLED_BY_DEFAULT}; +#if defined(OS_ANDROID) const base::Feature kRefreshExpirationDate{"RefreshExpirationDate", base::FEATURE_ENABLED_BY_DEFAULT}; #endif @@ -66,4 +66,10 @@ const base::Feature kDeleteExpiredDownloads{"DeleteExpiredDownloads", } // namespace features +namespace switches { + +const char kDownloadLaterDebugOnWifi[] = "download-later-debug-on-wifi"; + +} // namespace switches + } // namespace download diff --git a/chromium/components/download/public/common/download_features.h b/chromium/components/download/public/common/download_features.h index acb7a1c5399..55a1097789e 100644 --- a/chromium/components/download/public/common/download_features.h +++ b/chromium/components/download/public/common/download_features.h @@ -12,6 +12,10 @@ namespace download { namespace features { +// The Finch parameter for download later feature to function only on cellular +// network. +constexpr char kDownloadLaterRequireCellular[] = "require_cellular"; + // Whether offline content provider should be used for the downloads UI.. COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature kUseDownloadOfflineContentProvider; @@ -23,10 +27,10 @@ COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature // Whether a download can be handled by parallel jobs. COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature kParallelDownloading; -#if defined(OS_ANDROID) // Whether to enable download later feature. COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature kDownloadLater; +#if defined(OS_ANDROID) // Whether download expiration date will be refreshed on resumption. COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature kRefreshExpirationDate; #endif @@ -58,6 +62,14 @@ COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature kDeleteExpiredDownloads; } // namespace features +namespace switches { + +// If set, show the download later dialog without the requirement of being on +// cellular network. +COMPONENTS_DOWNLOAD_EXPORT extern const char kDownloadLaterDebugOnWifi[]; + +} // namespace switches + } // namespace download #endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_FEATURES_H_ diff --git a/chromium/components/download/public/common/download_item.h b/chromium/components/download/public/common/download_item.h index 7e14a865013..ac9805719a4 100644 --- a/chromium/components/download/public/common/download_item.h +++ b/chromium/components/download/public/common/download_item.h @@ -32,6 +32,7 @@ #include "components/download/public/common/download_danger_type.h" #include "components/download/public/common/download_export.h" #include "components/download/public/common/download_interrupt_reasons.h" +#include "components/download/public/common/download_schedule.h" #include "components/download/public/common/download_source.h" #include "ui/base/page_transition_types.h" #include "url/origin.h" @@ -106,7 +107,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItem : public base::SupportsUserData { UNKNOWN = 0, // Download is not mixed content. SAFE = 1, - // Download has been explicitly OK'd by the user. + // Download has been explicitly OK'd by the user. Only used on Desktop. VALIDATED = 2, // Download is mixed content, and the user should be warned. WARN = 3, @@ -512,6 +513,10 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItem : public base::SupportsUserData { // Gets the DownloadCreationType of this item. virtual DownloadCreationType GetDownloadCreationType() const = 0; + // Gets the download schedule to start the time at particular time. + virtual const base::Optional<DownloadSchedule>& GetDownloadSchedule() + const = 0; + // External state transitions/setters ---------------------------------------- // TODO(rdsmith): These should all be removed; the download item should @@ -530,6 +535,10 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItem : public base::SupportsUserData { // Called when async scanning completes with the given |danger_type|. virtual void OnAsyncScanningCompleted(DownloadDangerType danger_type) = 0; + // Called when the user changes the download schedule options. + virtual void OnDownloadScheduleChanged( + base::Optional<DownloadSchedule> schedule) = 0; + // Mark the download to be auto-opened when completed. virtual void SetOpenWhenComplete(bool open) = 0; diff --git a/chromium/components/download/public/common/download_item_impl.h b/chromium/components/download/public/common/download_item_impl.h index 2c679770129..1e7083bb770 100644 --- a/chromium/components/download/public/common/download_item_impl.h +++ b/chromium/components/download/public/common/download_item_impl.h @@ -195,6 +195,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl base::Time last_access_time, bool transient, const std::vector<DownloadItem::ReceivedSlice>& received_slices, + base::Optional<DownloadSchedule> download_schedule, std::unique_ptr<DownloadEntry> download_entry); // Constructing for a regular download. @@ -297,9 +298,12 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl bool IsTransient() const override; bool IsParallelDownload() const override; DownloadCreationType GetDownloadCreationType() const override; + const base::Optional<DownloadSchedule>& GetDownloadSchedule() const override; void OnContentCheckCompleted(DownloadDangerType danger_type, DownloadInterruptReason reason) override; void OnAsyncScanningCompleted(DownloadDangerType danger_type) override; + void OnDownloadScheduleChanged( + base::Optional<DownloadSchedule> schedule) override; void SetOpenWhenComplete(bool open) override; void SetOpened(bool opened) override; void SetLastAccessTime(base::Time last_access_time) override; @@ -561,6 +565,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl DownloadDangerType danger_type, MixedContentStatus mixed_content_status, const base::FilePath& intermediate_path, + base::Optional<DownloadSchedule> download_schedule, DownloadInterruptReason interrupt_reason); void OnDownloadRenamedToIntermediateName(DownloadInterruptReason reason, @@ -568,6 +573,18 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl void OnTargetResolved(); + // If |download_schedule_| presents, maybe interrupt the download and start + // later. Returns whether the download should be started later. + bool MaybeDownloadLater(); + + // Returns whether the download should proceed later based on network + // condition and user scheduled start time defined in |download_schedule_|. + bool ShouldDownloadLater() const; + + // Swap the |download_schedule_| with new data, may pass in base::nullopt to + // remove the schedule. + void SwapDownloadSchedule(base::Optional<DownloadSchedule> download_schedule); + // If all pre-requisites have been met, complete download processing, i.e. do // internal cleanup, file rename, and potentially auto-open. (Dangerous // downloads still may block on user acceptance after this point.) @@ -841,6 +858,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl // The MixedContentStatus if determined. MixedContentStatus mixed_content_status_ = MixedContentStatus::UNKNOWN; + // Defines when to start the download. Used by download later feature. + base::Optional<DownloadSchedule> download_schedule_; + THREAD_CHECKER(thread_checker_); base::WeakPtrFactory<DownloadItemImpl> weak_ptr_factory_{this}; 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 804f217798c..e4608e95662 100644 --- a/chromium/components/download/public/common/download_item_impl_delegate.h +++ b/chromium/components/download/public/common/download_item_impl_delegate.h @@ -13,6 +13,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_schedule.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" @@ -45,6 +46,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImplDelegate { DownloadDangerType danger_type, DownloadItem::MixedContentStatus mixed_content_status, const base::FilePath& intermediate_path, + base::Optional<DownloadSchedule> download_schedule, DownloadInterruptReason interrupt_reason)>; // Request determination of the download target from the delegate. virtual void DetermineDownloadTarget(DownloadItemImpl* download, diff --git a/chromium/components/download/public/common/download_schedule.cc b/chromium/components/download/public/common/download_schedule.cc new file mode 100644 index 00000000000..eb53fe70024 --- /dev/null +++ b/chromium/components/download/public/common/download_schedule.cc @@ -0,0 +1,24 @@ +// Copyright 2020 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_schedule.h" + +#include "base/check.h" + +namespace download { + +DownloadSchedule::DownloadSchedule(bool only_on_wifi, + base::Optional<base::Time> start_time) + : only_on_wifi_(only_on_wifi), start_time_(start_time) {} + +DownloadSchedule::DownloadSchedule(const DownloadSchedule&) = default; + +DownloadSchedule::~DownloadSchedule() = default; + +bool DownloadSchedule::operator==(const DownloadSchedule& other) const { + return only_on_wifi_ == other.only_on_wifi() && + start_time_ == other.start_time(); +} + +} // namespace download diff --git a/chromium/components/download/public/common/download_schedule.h b/chromium/components/download/public/common/download_schedule.h new file mode 100644 index 00000000000..205fbe7a4f8 --- /dev/null +++ b/chromium/components/download/public/common/download_schedule.h @@ -0,0 +1,38 @@ +// Copyright 2020 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_SCHEDULE_H_ +#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_SCHEDULE_H_ + +#include "base/optional.h" +#include "base/time/time.h" +#include "components/download/public/common/download_export.h" + +namespace download { + +// Contains all information to schedule a download, used by download later +// feature. +class COMPONENTS_DOWNLOAD_EXPORT DownloadSchedule { + public: + DownloadSchedule(bool only_on_wifi, base::Optional<base::Time> start_time); + DownloadSchedule(const DownloadSchedule&); + ~DownloadSchedule(); + + bool operator==(const DownloadSchedule&) const; + + bool only_on_wifi() const { return only_on_wifi_; } + + const base::Optional<base::Time>& start_time() const { return start_time_; } + + private: + // Whether to download only on WIFI. If true, |start_time_| will be ignored. + bool only_on_wifi_; + + // Time to start the download. Will be ignored if |only_on_wifi_| is true. + base::Optional<base::Time> start_time_; +}; + +} // namespace download + +#endif // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_DOWNLOAD_SCHEDULE_H_ diff --git a/chromium/components/download/public/common/download_schedule_unittest.cc b/chromium/components/download/public/common/download_schedule_unittest.cc new file mode 100644 index 00000000000..4de42429ccb --- /dev/null +++ b/chromium/components/download/public/common/download_schedule_unittest.cc @@ -0,0 +1,30 @@ +// Copyright 2020 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_schedule.h" + +#include "base/optional.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace download { +namespace { + +TEST(DownloadScheduleTest, CtorAndCopy) { + DownloadSchedule download_schedule(false, base::nullopt); + EXPECT_FALSE(download_schedule.only_on_wifi()); + EXPECT_EQ(download_schedule.start_time(), base::nullopt); + + download_schedule = DownloadSchedule(true, base::nullopt); + EXPECT_TRUE(download_schedule.only_on_wifi()); + EXPECT_EQ(download_schedule.start_time(), base::nullopt); + + auto time = base::make_optional( + base::Time::FromDeltaSinceWindowsEpoch(base::TimeDelta::FromDays(1))); + download_schedule = DownloadSchedule(false, time); + EXPECT_FALSE(download_schedule.only_on_wifi()); + EXPECT_EQ(download_schedule.start_time(), time); +} + +} // namespace +} // namespace download diff --git a/chromium/components/download/public/common/download_stats.h b/chromium/components/download/public/common/download_stats.h index 3570e723235..dc25996957c 100644 --- a/chromium/components/download/public/common/download_stats.h +++ b/chromium/components/download/public/common/download_stats.h @@ -145,14 +145,6 @@ enum DownloadCountTypes { DOWNLOAD_COUNT_TYPES_LAST_ENTRY }; -enum DownloadDiscardReason { - // The download is being discarded due to a user action. - DOWNLOAD_DISCARD_DUE_TO_USER_ACTION, - - // The download is being discarded due to the browser being shut down. - DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN -}; - // Enum for in-progress download DB, used in histogram // "Download.InProgressDB.Counts". enum InProgressDBCountTypes { @@ -243,6 +235,18 @@ enum class ResumptionRestartCountTypes { kMaxValue = kMissingStrongValidatorsCount }; +// Events for user scheduled downloads. Used in histograms, don't reuse or +// remove items. Keep in sync with DownloadLaterEvent in enums.xml. +enum class DownloadLaterEvent { + // Schedule is added during download target determination process. + kScheduleAdded = 0, + // Scheduled is changed from the UI after download is scheduled. + kScheduleChanged = 1, + // Scheduled is removed during resumption. + kScheduleRemoved = 2, + kMaxValue = kScheduleRemoved +}; + // Increment one of the above counts. COMPONENTS_DOWNLOAD_EXPORT void RecordDownloadCount(DownloadCountTypes type); @@ -458,6 +462,10 @@ COMPONENTS_DOWNLOAD_EXPORT void RecordDownloadManagerMemoryUsage( COMPONENTS_DOWNLOAD_EXPORT void RecordParallelRequestCreationFailure( DownloadInterruptReason reason); +// Record download later events. +COMPONENTS_DOWNLOAD_EXPORT void RecordDownloadLaterEvent( + DownloadLaterEvent event); + #if defined(OS_ANDROID) enum class BackgroudTargetDeterminationResultTypes { // Target determination succeeded. diff --git a/chromium/components/download/public/common/mock_download_item.h b/chromium/components/download/public/common/mock_download_item.h index 4dabd4a677d..0160d4d4d28 100644 --- a/chromium/components/download/public/common/mock_download_item.h +++ b/chromium/components/download/public/common/mock_download_item.h @@ -124,6 +124,8 @@ class MockDownloadItem : public DownloadItem { MOCK_CONST_METHOD0(IsTransient, bool()); MOCK_CONST_METHOD0(IsParallelDownload, bool()); MOCK_CONST_METHOD0(GetDownloadCreationType, DownloadCreationType()); + MOCK_CONST_METHOD0(GetDownloadSchedule, + const base::Optional<DownloadSchedule>&()); MOCK_METHOD2(OnContentCheckCompleted, void(DownloadDangerType, DownloadInterruptReason)); MOCK_METHOD1(SetOpenWhenComplete, void(bool)); @@ -135,6 +137,10 @@ class MockDownloadItem : public DownloadItem { MOCK_METHOD1(SimulateErrorForTesting, void(DownloadInterruptReason)); MOCK_METHOD2(Rename, void(const base::FilePath&, RenameDownloadCallback)); MOCK_METHOD1(OnAsyncScanningCompleted, void(DownloadDangerType)); + MOCK_METHOD(void, + OnDownloadScheduleChanged, + (base::Optional<DownloadSchedule>), + (override)); private: base::ObserverList<Observer>::Unchecked observers_; diff --git a/chromium/components/download/public/common/mock_download_item_impl.cc b/chromium/components/download/public/common/mock_download_item_impl.cc index 9a47c3aefe6..d9cbdea889c 100644 --- a/chromium/components/download/public/common/mock_download_item_impl.cc +++ b/chromium/components/download/public/common/mock_download_item_impl.cc @@ -37,6 +37,7 @@ MockDownloadItemImpl::MockDownloadItemImpl(DownloadItemImplDelegate* delegate) base::Time(), true, DownloadItem::ReceivedSlices(), + base::nullopt /*download_schedule*/, nullptr /* download_entry */) {} MockDownloadItemImpl::~MockDownloadItemImpl() = default; 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 8ee0ae13272..3cafe813d18 100644 --- a/chromium/components/download/public/common/mock_download_item_impl.h +++ b/chromium/components/download/public/common/mock_download_item_impl.h @@ -14,6 +14,7 @@ #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_schedule.h" #include "testing/gmock/include/gmock/gmock.h" #include "url/origin.h" @@ -27,12 +28,13 @@ class MockDownloadItemImpl : public DownloadItemImpl { explicit MockDownloadItemImpl(DownloadItemImplDelegate* delegate); ~MockDownloadItemImpl() override; - MOCK_METHOD6(OnDownloadTargetDetermined, + MOCK_METHOD7(OnDownloadTargetDetermined, void(const base::FilePath&, TargetDisposition, DownloadDangerType, MixedContentStatus, const base::FilePath&, + base::Optional<DownloadSchedule>, DownloadInterruptReason)); MOCK_METHOD1(AddObserver, void(DownloadItem::Observer*)); MOCK_METHOD1(RemoveObserver, void(DownloadItem::Observer*)); diff --git a/chromium/components/download/public/task/download_task_types.h b/chromium/components/download/public/task/download_task_types.h index 559ca796f2d..a979fb8cde0 100644 --- a/chromium/components/download/public/task/download_task_types.h +++ b/chromium/components/download/public/task/download_task_types.h @@ -18,6 +18,9 @@ enum class DownloadTaskType { // Task to invoke the download auto-resumption handler. DOWNLOAD_AUTO_RESUMPTION_TASK = 2, + + // Task to start user scheduled downloads. + DOWNLOAD_LATER_TASK = 3, }; } // namespace download diff --git a/chromium/components/download/public/task/task_manager.h b/chromium/components/download/public/task/task_manager.h index 0bfe8854d81..a61269a2800 100644 --- a/chromium/components/download/public/task/task_manager.h +++ b/chromium/components/download/public/task/task_manager.h @@ -57,11 +57,13 @@ class TaskManager { virtual void OnStopScheduledTask(DownloadTaskType task_type) = 0; // Should be called once the task is complete. The callback passed through - // OnStartScheduledTask() will be run in order to notify that the task is done - // and the system should reschedule the task with the original params if - // |needs_reschedule| is true. If there are pending params for a new task, a - // new task will be scheduled immediately and reschedule logic will not be - // run. + // OnStartScheduledTask() will be run in order to notify that the task is + // done. If there are pending params for a new task, a new task will be + // scheduled immediately. + // + // Most caller should set |needs_reschedule| to false. The OS will reschedule + // the task with the original params but with a OS controlled back off time if + // |needs_reschedule| is true. virtual void NotifyTaskFinished(DownloadTaskType task_type, bool needs_reschedule) = 0; diff --git a/chromium/components/download/public/task/task_manager_impl.cc b/chromium/components/download/public/task/task_manager_impl.cc index ac40fba6360..772ac239658 100644 --- a/chromium/components/download/public/task/task_manager_impl.cc +++ b/chromium/components/download/public/task/task_manager_impl.cc @@ -49,11 +49,10 @@ void TaskManagerImpl::UnscheduleTask(DownloadTaskType task_type) { void TaskManagerImpl::OnStartScheduledTask(DownloadTaskType task_type, TaskFinishedCallback callback) { - DCHECK(pending_task_params_.find(task_type) != pending_task_params_.end()); - current_task_params_[task_type] = pending_task_params_[task_type]; - pending_task_params_.erase(task_type); + if (pending_task_params_.find(task_type) != pending_task_params_.end()) + current_task_params_[task_type] = pending_task_params_[task_type]; - DCHECK(!IsRunningTask(task_type)); + pending_task_params_.erase(task_type); task_finished_callbacks_[task_type] = std::move(callback); } diff --git a/chromium/components/download/public/task/task_manager_unittest.cc b/chromium/components/download/public/task/task_manager_unittest.cc index 526fa54e42b..5aac8623135 100644 --- a/chromium/components/download/public/task/task_manager_unittest.cc +++ b/chromium/components/download/public/task/task_manager_unittest.cc @@ -224,6 +224,34 @@ TEST_F(TaskManagerImplTest, StopTaskWillClearTheCallback) { task_runner_->RunUntilIdle(); } +// Verifies that OnStartScheduledTask() can be called without preceding +// ScheduleTask() calls. +TEST_F(TaskManagerImplTest, StartTaskWithoutPendingParams) { + MockTaskWaiter waiter; + auto callback = + base::BindOnce(&MockTaskWaiter::TaskFinished, base::Unretained(&waiter)); + task_manager_->OnStartScheduledTask(DownloadTaskType::DOWNLOAD_TASK, + std::move(callback)); + EXPECT_CALL(waiter, TaskFinished(false)).Times(1); + task_manager_->NotifyTaskFinished(DownloadTaskType::DOWNLOAD_TASK, false); + task_runner_->RunUntilIdle(); +} + +// Verifies that OnStopScheduledTask() can be called without preceding +// ScheduleTask() calls. +TEST_F(TaskManagerImplTest, StopTaskWithoutPendingParams) { + MockTaskWaiter waiter; + EXPECT_CALL(waiter, TaskFinished(false)).Times(0); + + auto callback = + base::BindOnce(&MockTaskWaiter::TaskFinished, base::Unretained(&waiter)); + task_manager_->OnStartScheduledTask(DownloadTaskType::DOWNLOAD_TASK, + std::move(callback)); + task_manager_->OnStopScheduledTask(DownloadTaskType::DOWNLOAD_TASK); + + task_runner_->RunUntilIdle(); +} + TEST_F(TaskManagerImplTest, StopTaskWillSchedulePendingParams) { auto params = CreateTaskParams(); task_manager_->ScheduleTask(DownloadTaskType::DOWNLOAD_TASK, params); |