diff options
Diffstat (limited to 'chromium/components/download')
40 files changed, 971 insertions, 200 deletions
diff --git a/chromium/components/download/database/download_db_conversions.cc b/chromium/components/download/database/download_db_conversions.cc index fc79a1483d2..d9c2ef32717 100644 --- a/chromium/components/download/database/download_db_conversions.cc +++ b/chromium/components/download/database/download_db_conversions.cc @@ -9,6 +9,20 @@ #include "base/pickle.h" namespace download { +namespace { + +// Converts base::Time to a timpstamp in milliseconds. +int64_t FromTimeToMilliseconds(base::Time time) { + return time.ToDeltaSinceWindowsEpoch().InMilliseconds(); +} + +// Converts a time stamp in milliseconds to base::Time. +base::Time FromMillisecondsToTime(int64_t time_ms) { + return base::Time::FromDeltaSinceWindowsEpoch( + base::TimeDelta::FromMilliseconds(time_ms)); +} + +} // namespace DownloadEntry DownloadDBConversions::DownloadEntryFromProto( const download_pb::DownloadEntry& proto) { @@ -173,12 +187,10 @@ download_pb::InProgressInfo DownloadDBConversions::InProgressInfoToProto( proto.set_start_time( in_progress_info.start_time.is_null() ? -1 - : in_progress_info.start_time.ToDeltaSinceWindowsEpoch() - .InMilliseconds()); + : FromTimeToMilliseconds(in_progress_info.start_time)); proto.set_end_time(in_progress_info.end_time.is_null() ? -1 - : in_progress_info.end_time.ToDeltaSinceWindowsEpoch() - .InMilliseconds()); + : FromTimeToMilliseconds(in_progress_info.end_time)); for (size_t i = 0; i < in_progress_info.received_slices.size(); ++i) { download_pb::ReceivedSlice* slice = proto.add_received_slices(); slice->set_received_bytes( @@ -195,6 +207,15 @@ download_pb::InProgressInfo DownloadDBConversions::InProgressInfoToProto( proto.set_metered(in_progress_info.metered); proto.set_bytes_wasted(in_progress_info.bytes_wasted); proto.set_auto_resume_count(in_progress_info.auto_resume_count); + if (in_progress_info.download_schedule.has_value()) { + DCHECK_NE(in_progress_info.download_schedule->only_on_wifi(), + in_progress_info.metered); + auto download_schedule_proto = + std::make_unique<download_pb::DownloadSchedule>(DownloadScheduleToProto( + in_progress_info.download_schedule.value())); + proto.set_allocated_download_schedule(download_schedule_proto.release()); + } + return proto; } @@ -223,16 +244,12 @@ InProgressInfo DownloadDBConversions::InProgressInfoFromProto( base::Pickle(proto.target_path().data(), proto.target_path().size())); info.target_path.ReadFromPickle(&target_path); info.received_bytes = proto.received_bytes(); - info.start_time = - proto.start_time() == -1 - ? base::Time() - : base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMilliseconds(proto.start_time())); - info.end_time = - proto.end_time() == -1 - ? base::Time() - : base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMilliseconds(proto.end_time())); + info.start_time = proto.start_time() == -1 + ? base::Time() + : FromMillisecondsToTime(proto.start_time()); + info.end_time = proto.end_time() == -1 + ? base::Time() + : FromMillisecondsToTime(proto.end_time()); for (int i = 0; i < proto.received_slices_size(); ++i) { info.received_slices.emplace_back(proto.received_slices(i).offset(), @@ -249,6 +266,12 @@ InProgressInfo DownloadDBConversions::InProgressInfoFromProto( info.metered = proto.metered(); info.bytes_wasted = proto.bytes_wasted(); info.auto_resume_count = proto.auto_resume_count(); + if (proto.has_download_schedule()) { + info.download_schedule = DownloadScheduleFromProto( + proto.download_schedule(), !proto.metered() /*only_on_wifi*/); + DCHECK_NE(info.download_schedule->only_on_wifi(), info.metered); + } + return info; } @@ -268,6 +291,27 @@ download_pb::UkmInfo DownloadDBConversions::UkmInfoToProto( return proto; } +download_pb::DownloadSchedule DownloadDBConversions::DownloadScheduleToProto( + const DownloadSchedule& download_schedule) { + // download::DownloadSchedule.only_on_wifi is not persisted, use + // InProgressInfo.metered instead. + download_pb::DownloadSchedule proto; + if (download_schedule.start_time().has_value()) { + proto.set_start_time( + FromTimeToMilliseconds(download_schedule.start_time().value())); + } + return proto; +} + +DownloadSchedule DownloadDBConversions::DownloadScheduleFromProto( + const download_pb::DownloadSchedule& proto, + bool only_on_wifi) { + base::Optional<base::Time> start_time; + if (proto.has_start_time()) + start_time = FromMillisecondsToTime(proto.start_time()); + return DownloadSchedule(only_on_wifi, std::move(start_time)); +} + DownloadInfo DownloadDBConversions::DownloadInfoFromProto( const download_pb::DownloadInfo& proto) { DownloadInfo info; diff --git a/chromium/components/download/database/download_db_conversions.h b/chromium/components/download/database/download_db_conversions.h index a113d86e7e2..39ad586d092 100644 --- a/chromium/components/download/database/download_db_conversions.h +++ b/chromium/components/download/database/download_db_conversions.h @@ -14,6 +14,7 @@ #include "components/download/database/in_progress/ukm_info.h" #include "components/download/database/proto/download_entry.pb.h" #include "components/download/database/proto/download_source.pb.h" +#include "components/download/public/common/download_schedule.h" namespace download { @@ -53,6 +54,13 @@ class DownloadDBConversions { static UkmInfo UkmInfoFromProto(const download_pb::UkmInfo& proto); + static download_pb::DownloadSchedule DownloadScheduleToProto( + const DownloadSchedule& download_schedule); + + static DownloadSchedule DownloadScheduleFromProto( + const download_pb::DownloadSchedule& proto, + bool metered); + static download_pb::DownloadInfo DownloadInfoToProto( const DownloadInfo& download_info); diff --git a/chromium/components/download/database/download_db_conversions_unittest.cc b/chromium/components/download/database/download_db_conversions_unittest.cc index 0dfbd563305..c89f8cbd06f 100644 --- a/chromium/components/download/database/download_db_conversions_unittest.cc +++ b/chromium/components/download/database/download_db_conversions_unittest.cc @@ -4,6 +4,8 @@ #include "components/download/database/download_db_conversions.h" +#include "base/optional.h" +#include "components/download/public/common/download_schedule.h" #include "components/download/public/common/download_url_parameters.h" #include "testing/gtest/include/gtest/gtest.h" @@ -46,6 +48,8 @@ InProgressInfo CreateInProgressInfo() { std::make_pair<std::string, std::string>("123", "456")); info.request_headers.emplace_back( std::make_pair<std::string, std::string>("ABC", "def")); + info.download_schedule = base::make_optional<DownloadSchedule>( + false /*only_on_wifi*/, base::nullopt); return info; } @@ -163,4 +167,22 @@ TEST_F(DownloadDBConversionsTest, DownloadDBEntry) { EXPECT_EQ(entry, DownloadDBEntryFromProto(DownloadDBEntryToProto(entry))); } +TEST_F(DownloadDBConversionsTest, DownloadSchedule) { + const bool kOnlyOnWifi = true; + DownloadSchedule download_schedule(kOnlyOnWifi, base::nullopt /*start_time*/); + // InProgressInfo.metered is used to set DownloadSchedule.only_on_wifi. + auto persisted_download_schedule = DownloadScheduleFromProto( + DownloadScheduleToProto(download_schedule), !kOnlyOnWifi); + EXPECT_FALSE(persisted_download_schedule.only_on_wifi()); + EXPECT_TRUE(download_schedule.only_on_wifi()); + + base::Time time; + bool success = base::Time::FromUTCString("2020-06-11 15:41", &time); + ASSERT_TRUE(success); + download_schedule = DownloadSchedule(kOnlyOnWifi, time); + persisted_download_schedule = DownloadScheduleFromProto( + DownloadScheduleToProto(download_schedule), kOnlyOnWifi); + EXPECT_EQ(persisted_download_schedule, download_schedule); +} + } // namespace download diff --git a/chromium/components/download/database/in_progress/in_progress_info.cc b/chromium/components/download/database/in_progress/in_progress_info.cc index 7cb0c20fc9b..61a7afbd023 100644 --- a/chromium/components/download/database/in_progress/in_progress_info.cc +++ b/chromium/components/download/database/in_progress/in_progress_info.cc @@ -30,7 +30,8 @@ bool InProgressInfo::operator==(const InProgressInfo& other) const { danger_type == other.danger_type && interrupt_reason == other.interrupt_reason && paused == other.paused && metered == other.metered && bytes_wasted == other.bytes_wasted && - auto_resume_count == other.auto_resume_count; + auto_resume_count == other.auto_resume_count && + download_schedule == other.download_schedule; } } // namespace download diff --git a/chromium/components/download/database/in_progress/in_progress_info.h b/chromium/components/download/database/in_progress/in_progress_info.h index 3f7b5fac15e..2b67686c04c 100644 --- a/chromium/components/download/database/in_progress/in_progress_info.h +++ b/chromium/components/download/database/in_progress/in_progress_info.h @@ -8,8 +8,10 @@ #include <string> #include <vector> +#include "base/optional.h" #include "components/download/public/common/download_danger_type.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 "url/gurl.h" @@ -118,8 +120,12 @@ struct InProgressInfo { // triggered resumption. int32_t auto_resume_count = 0; - // Whether the download is initiated on a metered network + // Whether the download is initiated on a metered network. If false, download + // can ony be resumed on WIFI. bool metered = false; + + // When to start the download. Used by download later feature. + base::Optional<DownloadSchedule> download_schedule; }; } // namespace download diff --git a/chromium/components/download/database/proto/download_entry.proto b/chromium/components/download/database/proto/download_entry.proto index cbed5951b5f..af788572287 100644 --- a/chromium/components/download/database/proto/download_entry.proto +++ b/chromium/components/download/database/proto/download_entry.proto @@ -44,6 +44,11 @@ message UkmInfo { optional int64 ukm_download_id = 2; } +// Information about when to start the download, used by download later feature. +message DownloadSchedule { + optional int64 start_time = 1; +} + // Information about an in progress download. message InProgressInfo { repeated string url_chain = 1; @@ -73,6 +78,7 @@ message InProgressInfo { optional bool metered = 25; optional int64 bytes_wasted = 26; optional int32 auto_resume_count = 27; + optional DownloadSchedule download_schedule = 28; } // Stores various metadata related to a download. diff --git a/chromium/components/download/internal/background_service/controller_impl.cc b/chromium/components/download/internal/background_service/controller_impl.cc index 914dd0742d1..cff032f274d 100644 --- a/chromium/components/download/internal/background_service/controller_impl.cc +++ b/chromium/components/download/internal/background_service/controller_impl.cc @@ -408,6 +408,7 @@ void ControllerImpl::HandleTaskFinished(DownloadTaskType task_type, ScheduleCleanupTask(); break; case DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK: + case DownloadTaskType::DOWNLOAD_LATER_TASK: NOTREACHED(); } } diff --git a/chromium/components/download/internal/background_service/file_monitor_impl.cc b/chromium/components/download/internal/background_service/file_monitor_impl.cc index a2dd120efb3..13a4b0d640c 100644 --- a/chromium/components/download/internal/background_service/file_monitor_impl.cc +++ b/chromium/components/download/internal/background_service/file_monitor_impl.cc @@ -8,6 +8,7 @@ #include "base/callback_helpers.h" #include "base/files/file_enumerator.h" #include "base/files/file_util.h" +#include "base/logging.h" #include "base/stl_util.h" #include "base/system/sys_info.h" #include "base/task_runner_util.h" diff --git a/chromium/components/download/internal/background_service/stats.cc b/chromium/components/download/internal/background_service/stats.cc index f646f5e51f6..f4b6d5d9cc3 100644 --- a/chromium/components/download/internal/background_service/stats.cc +++ b/chromium/components/download/internal/background_service/stats.cc @@ -45,7 +45,11 @@ std::string TaskTypeToHistogramSuffix(DownloadTaskType task_type) { case DownloadTaskType::CLEANUP_TASK: return "CleanUpTask"; case DownloadTaskType::DOWNLOAD_AUTO_RESUMPTION_TASK: + NOTREACHED(); return "DownloadAutoResumptionTask"; + case DownloadTaskType::DOWNLOAD_LATER_TASK: + NOTREACHED(); + return "DownloadLaterTask"; } NOTREACHED(); return std::string(); @@ -322,11 +326,5 @@ void LogHasUploadData(DownloadClient client, bool has_upload_data) { base::UmaHistogramBoolean(name, has_upload_data); } -void LogDownloadClientInflatedFullBrowser(DownloadClient client) { - std::string client_name(ClientToHistogramSuffix(client)); - base::UmaHistogramBoolean( - "Download.Service.Clients.InflatedFullBrowser." + client_name, true); -} - } // namespace stats } // namespace download diff --git a/chromium/components/download/internal/background_service/stats.h b/chromium/components/download/internal/background_service/stats.h index 16c2e56889a..950801342e8 100644 --- a/chromium/components/download/internal/background_service/stats.h +++ b/chromium/components/download/internal/background_service/stats.h @@ -216,10 +216,6 @@ void LogEntryRetryCount(uint32_t retry_count); // Records whether the entry was an upload. void LogHasUploadData(DownloadClient client, bool has_upload_data); -// Records count of reduced mode to full browser transitions requested by each -// client. -void LogDownloadClientInflatedFullBrowser(DownloadClient client); - } // namespace stats } // namespace download diff --git a/chromium/components/download/internal/common/base_file.cc b/chromium/components/download/internal/common/base_file.cc index 16d1f3f9640..0c3bbbe1048 100644 --- a/chromium/components/download/internal/common/base_file.cc +++ b/chromium/components/download/internal/common/base_file.cc @@ -95,14 +95,14 @@ void InitializeFile(base::File* file, const base::FilePath& file_path) { ); } -void DeleteFile(const base::FilePath& file_path) { +void DeleteFileWrapper(const base::FilePath& file_path) { #if defined(OS_ANDROID) if (file_path.IsContentUri()) { DownloadCollectionBridge::DeleteIntermediateUri(file_path); return; } #endif // defined(OS_ANDROID) - base::DeleteFile(file_path, false); + base::DeleteFile(file_path); } } // namespace @@ -312,7 +312,7 @@ void BaseFile::Cancel() { if (!full_path_.empty()) { CONDITIONAL_TRACE( INSTANT0("download", "DownloadFileDeleted", TRACE_EVENT_SCOPE_THREAD)); - DeleteFile(full_path_); + DeleteFileWrapper(full_path_); } Detach(); diff --git a/chromium/components/download/internal/common/download_item_impl.cc b/chromium/components/download/internal/common/download_item_impl.cc index 9576eafa71e..486eab9165c 100644 --- a/chromium/components/download/internal/common/download_item_impl.cc +++ b/chromium/components/download/internal/common/download_item_impl.cc @@ -314,6 +314,7 @@ DownloadItemImpl::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) : request_info_(url_chain, referrer_url, @@ -352,7 +353,8 @@ DownloadItemImpl::DownloadItemImpl( last_modified_time_(last_modified), etag_(etag), received_slices_(received_slices), - is_updating_observers_(false) { + is_updating_observers_(false), + download_schedule_(std::move(download_schedule)) { delegate_->Attach(); DCHECK(state_ == COMPLETE_INTERNAL || state_ == INTERRUPTED_INTERNAL || state_ == CANCELLED_INTERNAL); @@ -618,6 +620,7 @@ void DownloadItemImpl::Resume(bool user_resume) { paused_ = false; if (auto_resume_count_ >= kMaxAutoResumeAttempts) { RecordAutoResumeCountLimitReached(GetLastReason()); + UpdateObservers(); return; } @@ -640,11 +643,14 @@ void DownloadItemImpl::UpdateResumptionInfo(bool user_resume) { } auto_resume_count_ = user_resume ? 0 : ++auto_resume_count_; + download_schedule_ = base::nullopt; + RecordDownloadLaterEvent(DownloadLaterEvent::kScheduleRemoved); } void DownloadItemImpl::Cancel(bool user_cancel) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DVLOG(20) << __func__ << "() download = " << DebugString(true); + download_schedule_ = base::nullopt; InterruptAndDiscardPartialState( user_cancel ? DOWNLOAD_INTERRUPT_REASON_USER_CANCELED : DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN); @@ -1103,6 +1109,11 @@ DownloadItem::DownloadCreationType DownloadItemImpl::GetDownloadCreationType() return download_type_; } +const base::Optional<DownloadSchedule>& DownloadItemImpl::GetDownloadSchedule() + const { + return download_schedule_; +} + void DownloadItemImpl::OnContentCheckCompleted(DownloadDangerType danger_type, DownloadInterruptReason reason) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -1136,6 +1147,28 @@ void DownloadItemImpl::OnAsyncScanningCompleted( UpdateObservers(); } +void DownloadItemImpl::OnDownloadScheduleChanged( + base::Optional<DownloadSchedule> schedule) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!base::FeatureList::IsEnabled(features::kDownloadLater) || + state_ != INTERRUPTED_INTERNAL) { + return; + } + + RecordDownloadLaterEvent(DownloadLaterEvent::kScheduleChanged); + + SwapDownloadSchedule(std::move(schedule)); + + // Need to start later, don't proceed and ping observers. + if (ShouldDownloadLater()) { + UpdateObservers(); + return; + } + + // Download now. allow_metered_ will be updated afterward. + Resume(true /*user_resume*/); +} + void DownloadItemImpl::SetOpenWhenComplete(bool open) { open_when_complete_ = open; } @@ -1642,6 +1675,7 @@ void DownloadItemImpl::OnDownloadTargetDetermined( DownloadDangerType danger_type, MixedContentStatus mixed_content_status, const base::FilePath& intermediate_path, + base::Optional<DownloadSchedule> download_schedule, DownloadInterruptReason interrupt_reason) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); if (state_ == CANCELLED_INTERNAL) @@ -1664,6 +1698,10 @@ void DownloadItemImpl::OnDownloadTargetDetermined( return; } + if (download_schedule) + RecordDownloadLaterEvent(DownloadLaterEvent::kScheduleAdded); + SwapDownloadSchedule(std::move(download_schedule)); + // There were no other pending errors, and we just failed to determined the // download target. The target path, if it is non-empty, should be considered // suspect. The safe option here is to interrupt the download without doing an @@ -1801,6 +1839,14 @@ void DownloadItemImpl::OnTargetResolved() { return; } + // The download will be started later, interrupt it for now. + if (MaybeDownloadLater()) { + UpdateObservers(); + return; + } + + download_schedule_ = base::nullopt; + TransitionTo(IN_PROGRESS_INTERNAL); // TODO(asanka): Calling UpdateObservers() prior to MaybeCompleteDownload() is // not safe. The download could be in an underminate state after invoking @@ -1809,6 +1855,48 @@ void DownloadItemImpl::OnTargetResolved() { MaybeCompleteDownload(); } +bool DownloadItemImpl::MaybeDownloadLater() { + if (!base::FeatureList::IsEnabled(features::kDownloadLater) || + !download_schedule_.has_value()) { + return false; + } + + if (ShouldDownloadLater()) { + // TODO(xingliu): Maybe add a new interrupt reason for download later + // feature. + InterruptWithPartialState(GetReceivedBytes(), std::move(hash_state_), + DOWNLOAD_INTERRUPT_REASON_CRASH); + return true; + } + + return false; +} + +bool DownloadItemImpl::ShouldDownloadLater() const { + // No schedule, just proceed. + if (!download_schedule_) + return false; + + bool network_type_ok = !download_schedule_->only_on_wifi() || + !delegate_->IsActiveNetworkMetered(); + bool should_start_later = + download_schedule_->start_time().has_value() && + download_schedule_->start_time() > base::Time::Now(); + + // Don't proceed if network requirement is not met or has a scheduled start + // time. + return !network_type_ok || should_start_later; +} + +void DownloadItemImpl::SwapDownloadSchedule( + base::Optional<DownloadSchedule> download_schedule) { + if (!base::FeatureList::IsEnabled(features::kDownloadLater)) + return; + download_schedule_ = std::move(download_schedule); + if (download_schedule_) + allow_metered_ = !download_schedule_->only_on_wifi(); +} + // When SavePackage downloads MHTML to GData (see // SavePackageFilePickerChromeOS), GData calls MaybeCompleteDownload() like it // does for non-SavePackage downloads, but SavePackage downloads never satisfy @@ -2390,8 +2478,11 @@ void DownloadItemImpl::SetFullPath(const base::FilePath& new_path) { void DownloadItemImpl::AutoResumeIfValid() { DVLOG(20) << __func__ << "() " << DebugString(true); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - ResumeMode mode = GetResumeMode(); + if (download_schedule_.has_value()) + return; + + ResumeMode mode = GetResumeMode(); if (mode != ResumeMode::IMMEDIATE_RESTART && mode != ResumeMode::IMMEDIATE_CONTINUE) { return; 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 2de0cf61bd1..f31ba16df7e 100644 --- a/chromium/components/download/internal/common/download_item_impl_delegate.cc +++ b/chromium/components/download/internal/common/download_item_impl_delegate.cc @@ -33,13 +33,12 @@ void DownloadItemImplDelegate::Detach() { void DownloadItemImplDelegate::DetermineDownloadTarget( DownloadItemImpl* download, DownloadTargetCallback callback) { - // TODO(rdsmith/asanka): Do something useful if forced file path is null. base::FilePath target_path(download->GetForcedFilePath()); - std::move(callback).Run(target_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - target_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, target_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); } bool DownloadItemImplDelegate::ShouldCompleteDownload( 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 9c7ccba0902..252d3d9c537 100644 --- a/chromium/components/download/internal/common/download_item_impl_unittest.cc +++ b/chromium/components/download/internal/common/download_item_impl_unittest.cc @@ -21,16 +21,17 @@ #include "base/memory/ptr_util.h" #include "base/test/gmock_move_support.h" #include "base/test/metrics/histogram_tester.h" +#include "base/test/scoped_feature_list.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_features.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_url_parameters.h" #include "components/download/public/common/mock_download_file.h" -#include "components/download/public/common/mock_download_item.h" #include "crypto/secure_hash.h" #include "net/http/http_response_headers.h" #include "testing/gmock/include/gmock/gmock.h" @@ -54,9 +55,9 @@ const base::FilePath::CharType kDummyTargetPath[] = FILE_PATH_LITERAL("/testpath"); const base::FilePath::CharType kDummyIntermediatePath[] = FILE_PATH_LITERAL("/testpathx"); +const char kGuid[] = "8DF158E8-C980-4618-BB03-EBA3242EB48B"; namespace download { - namespace { template <typename T> @@ -101,6 +102,7 @@ class MockDelegate : public DownloadItemImplDelegate { MOCK_METHOD1(DownloadOpened, void(DownloadItemImpl*)); MOCK_METHOD1(DownloadRemoved, void(DownloadItemImpl*)); MOCK_CONST_METHOD0(IsOffTheRecord, bool()); + MOCK_METHOD(bool, IsActiveNetworkMetered, (), (const override)); void VerifyAndClearExpectations() { ::testing::Mock::VerifyAndClearExpectations(this); @@ -113,6 +115,7 @@ class MockDelegate : public DownloadItemImplDelegate { .WillRepeatedly(Return(false)); EXPECT_CALL(*this, ShouldOpenDownload_(_, _)).WillRepeatedly(Return(true)); EXPECT_CALL(*this, IsOffTheRecord()).WillRepeatedly(Return(false)); + EXPECT_CALL(*this, IsActiveNetworkMetered).WillRepeatedly(Return(false)); } }; @@ -213,8 +216,6 @@ const uint8_t kHashOfTestData1[] = { 0xef, 0x9c, 0x92, 0x33, 0x36, 0xe1, 0x06, 0x53, 0xfe, 0xc1, 0x95, 0xf4, 0x93, 0x45, 0x8b, 0x3b, 0x21, 0x89, 0x0e, 0x1b, 0x97}; -} // namespace - class DownloadItemTest : public testing::Test { public: DownloadItemTest() @@ -255,6 +256,25 @@ class DownloadItemTest : public testing::Test { return download; } + std::unique_ptr<DownloadItemImpl> CreateDownloadItem( + DownloadItem::DownloadState state, + download::DownloadInterruptReason reason) { + auto item = std::make_unique<download::DownloadItemImpl>( + mock_delegate(), kGuid, 10, base::FilePath(), base::FilePath(), + std::vector<GURL>(), GURL("http://example.com/a"), + GURL("http://example.com/a"), GURL("http://example.com/a"), + GURL("http://example.com/a"), + url::Origin::Create(GURL("http://example.com")), + "application/octet-stream", "application/octet-stream", + base::Time::Now(), base::Time::Now(), std::string(), std::string(), 10, + 10, 0, std::string(), state, + download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, reason, false, false, + false, base::Time::Now(), true, + std::vector<download::DownloadItem::ReceivedSlice>(), + base::nullopt /*download_schedule*/, nullptr /* download_entry */); + return item; + } + // Add DownloadFile to DownloadItem. MockDownloadFile* CallDownloadItemStart( DownloadItemImpl* item, @@ -299,7 +319,18 @@ class DownloadItemTest : public testing::Test { EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState()); EXPECT_TRUE(item->GetTargetFilePath().empty()); DownloadItemImplDelegate::DownloadTargetCallback callback; - MockDownloadFile* download_file = CallDownloadItemStart(item, &callback); + MockDownloadFile* file = CallDownloadItemStart(item, &callback); + DoRenameAndRunTargetCallback(item, file, std::move(callback), danger_type, + base::nullopt); + return file; + } + + void DoRenameAndRunTargetCallback( + DownloadItemImpl* item, + MockDownloadFile* download_file, + DownloadItemImplDelegate::DownloadTargetCallback callback, + DownloadDangerType danger_type, + base::Optional<DownloadSchedule> download_schedule) { base::FilePath target_path(kDummyTargetPath); base::FilePath intermediate_path(kDummyIntermediatePath); auto task_runner = base::ThreadTaskRunnerHandle::Get(); @@ -315,9 +346,8 @@ class DownloadItemTest : public testing::Test { std::move(callback).Run( target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, danger_type, DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, - DOWNLOAD_INTERRUPT_REASON_NONE); + download_schedule, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); - return download_file; } void DoDestinationComplete(DownloadItemImpl* item, @@ -560,11 +590,11 @@ TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) { // Currently, a notification would be generated if the danger type is anything // other than NOT_DANGEROUS. - std::move(callback).Run(target_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_FALSE(observer.CheckAndResetDownloadUpdated()); task_environment_.RunUntilIdle(); EXPECT_TRUE(observer.CheckAndResetDownloadUpdated()); @@ -857,11 +887,11 @@ TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) { EXPECT_CALL(*mock_download_file_ref, RenameAndUniquify(_, _)).Times(i == 0); ASSERT_TRUE(callback); - std::move(callback).Run(target_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // Use a continuable interrupt. @@ -955,7 +985,7 @@ TEST_F(DownloadItemTest, FailedResumptionDoesntUpdateOriginState) { DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(kDummyIntermediatePath), - DOWNLOAD_INTERRUPT_REASON_NONE); + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); ASSERT_TRUE(item->GetResponseHeaders()); @@ -1160,11 +1190,11 @@ TEST_F(DownloadItemTest, DisplayName) { intermediate_path)); })); - std::move(callback).Run(target_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); EXPECT_EQ(FILE_PATH_LITERAL("foo.bar"), item->GetFileNameToReportUser().value()); @@ -1218,7 +1248,7 @@ TEST_F(DownloadItemTest, InitDownloadFileFails) { DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(kDummyIntermediatePath), - DOWNLOAD_INTERRUPT_REASON_NONE); + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState()); @@ -1261,7 +1291,7 @@ TEST_F(DownloadItemTest, StartFailedDownload) { .Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, target_path, - DOWNLOAD_INTERRUPT_REASON_NONE); + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // Interrupt reason carried in create info should be recorded. @@ -1294,11 +1324,11 @@ TEST_F(DownloadItemTest, CallbackAfterRename) { new_intermediate_path)); })); - std::move(callback).Run(final_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // All the callbacks should have happened by now. ::testing::Mock::VerifyAndClearExpectations(download_file); @@ -1348,11 +1378,11 @@ TEST_F(DownloadItemTest, CallbackAfterInterruptedRename) { })); EXPECT_CALL(*download_file, Cancel()).Times(1); - std::move(callback).Run(final_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // All the callbacks should have happened by now. ::testing::Mock::VerifyAndClearExpectations(download_file); @@ -1418,11 +1448,11 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Restart) { })); EXPECT_CALL(*download_file, Cancel()).Times(1); - std::move(callback).Run(final_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // All the callbacks should have happened by now. ::testing::Mock::VerifyAndClearExpectations(download_file); @@ -1470,11 +1500,11 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) { .WillOnce(ReturnRefOfCopy(base::FilePath(new_intermediate_path))); EXPECT_CALL(*download_file, Detach()); - std::move(callback).Run(final_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // All the callbacks should have happened by now. ::testing::Mock::VerifyAndClearExpectations(download_file); @@ -1517,11 +1547,11 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Failed) { })); EXPECT_CALL(*download_file, Cancel()).Times(1); - std::move(callback).Run(final_path, - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, intermediate_path, + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); // All the callbacks should have happened by now. ::testing::Mock::VerifyAndClearExpectations(download_file); @@ -1564,6 +1594,7 @@ TEST_F(DownloadItemTest, DownloadTargetDetermined_Cancel) { DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(FILE_PATH_LITERAL("bar")), + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_USER_CANCELED); EXPECT_EQ(DownloadItem::CANCELLED, item->GetState()); } @@ -1574,11 +1605,11 @@ TEST_F(DownloadItemTest, DownloadTargetDetermined_CancelWithEmptyName) { MockDownloadFile* download_file = CallDownloadItemStart(item, &callback); EXPECT_CALL(*download_file, Cancel()); - std::move(callback).Run(base::FilePath(), - DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, - base::FilePath(), DOWNLOAD_INTERRUPT_REASON_NONE); + std::move(callback).Run( + base::FilePath(), DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(), + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_EQ(DownloadItem::CANCELLED, item->GetState()); } @@ -1589,11 +1620,12 @@ TEST_F(DownloadItemTest, DownloadTargetDetermined_Conflict) { base::FilePath target_path(FILE_PATH_LITERAL("/foo/bar")); EXPECT_CALL(*download_file, Cancel()); - std::move(callback).Run( - target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, - DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DownloadItem::MixedContentStatus::UNKNOWN, target_path, - DOWNLOAD_INTERRUPT_REASON_FILE_SAME_AS_SOURCE); + std::move(callback).Run(target_path, + DownloadItem::TARGET_DISPOSITION_OVERWRITE, + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DownloadItem::MixedContentStatus::UNKNOWN, + target_path, base::nullopt /*download_schedule*/, + DOWNLOAD_INTERRUPT_REASON_FILE_SAME_AS_SOURCE); EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState()); EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_SAME_AS_SOURCE, item->GetLastReason()); @@ -2179,8 +2211,6 @@ TEST_F(DownloadItemTest, AnnotationWithEmptyURLInIncognito) { task_environment_.RunUntilIdle(); } -namespace { - // The DownloadItemDestinationUpdateRaceTest fixture (defined below) is used to // test for race conditions between download destination events received via the // DownloadDestinationObserver interface, and the target determination logic. @@ -2405,8 +2435,6 @@ INSTANTIATE_TEST_SUITE_P(Failure, DownloadItemDestinationUpdateRaceTest, ::testing::ValuesIn(GenerateFailingEventLists())); -} // namespace - // Run through the DII workflow but the embedder cancels the download at target // determination. TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) { @@ -2444,7 +2472,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) { .Run(base::FilePath(), DownloadItem::TARGET_DISPOSITION_OVERWRITE, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(), - DOWNLOAD_INTERRUPT_REASON_NONE); + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_EQ(DownloadItem::CANCELLED, item_->GetState()); EXPECT_TRUE(canceled()); task_environment_.RunUntilIdle(); @@ -2499,7 +2527,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) { DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(kDummyIntermediatePath), - DOWNLOAD_INTERRUPT_REASON_NONE); + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); ASSERT_FALSE(intermediate_rename_callback.is_null()); @@ -2568,7 +2596,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) { DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DownloadItem::MixedContentStatus::UNKNOWN, base::FilePath(kDummyIntermediatePath), - DOWNLOAD_INTERRUPT_REASON_NONE); + base::nullopt /*download_schedule*/, DOWNLOAD_INTERRUPT_REASON_NONE); task_environment_.RunUntilIdle(); ASSERT_FALSE(intermediate_rename_callback.is_null()); @@ -2599,8 +2627,138 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) { task_environment_.RunUntilIdle(); } -TEST(MockDownloadItem, Compiles) { - MockDownloadItem mock_item; +// -------------------------------------------------------------- +// Download later feature test. +struct DownloadLaterTestParam { + // Input to build DownloadSchedule and config network type. + bool only_on_wifi; + base::Optional<base::Time> start_time; + bool is_active_network_metered; + + // Output and expectation. + DownloadItem::DownloadState state; + bool allow_metered; +}; + +std::vector<DownloadLaterTestParam> DownloadLaterTestParams() { + std::vector<DownloadLaterTestParam> params; + // Required wifi, and currently on wifi, the download won't stop. + params.push_back( + {true, base::nullopt, false, DownloadItem::IN_PROGRESS, false}); + // Don't require wifi, and currently not on wifi, the download won't stop. + params.push_back( + {false, base::nullopt, true, DownloadItem::IN_PROGRESS, true}); + // Download later, will be interrupted. + auto future_time = base::Time::Now() + base::TimeDelta::FromDays(10); + params.push_back({false, future_time, true, DownloadItem::INTERRUPTED, true}); + return params; +} + +class DownloadLaterTest + : public DownloadItemTest, + public ::testing::WithParamInterface<DownloadLaterTestParam> { + public: + DownloadLaterTest() = default; + ~DownloadLaterTest() override = default; +}; + +INSTANTIATE_TEST_SUITE_P(All, + DownloadLaterTest, + testing::ValuesIn(DownloadLaterTestParams())); + +TEST_P(DownloadLaterTest, TestDownloadScheduleAfterTargetDetermined) { + const auto& param = GetParam(); + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitAndEnableFeature(features::kDownloadLater); + + DownloadItemImpl* item = CreateDownloadItem(); + + // Setup network type and download schedule. + EXPECT_CALL(*mock_delegate(), IsActiveNetworkMetered) + .WillRepeatedly(Return(param.is_active_network_metered)); + EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState()); + EXPECT_TRUE(item->GetTargetFilePath().empty()); + DownloadItemImplDelegate::DownloadTargetCallback callback; + MockDownloadFile* download_file = CallDownloadItemStart(item, &callback); + + if (param.state == DownloadItem::INTERRUPTED) { + EXPECT_CALL(*download_file, Detach()); + } + + base::Optional<DownloadSchedule> download_schedule = + base::make_optional<DownloadSchedule>(param.only_on_wifi, + param.start_time); + + DoRenameAndRunTargetCallback(item, download_file, std::move(callback), + DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + std::move(download_schedule)); + + // Verify final states. + ASSERT_EQ(param.allow_metered, item->AllowMetered()); + ASSERT_EQ(param.state, item->GetState()); + ASSERT_EQ(0, item->GetAutoResumeCount()) + << "Download should not be auto resumed with DownloadSchedule."; + if (param.state == DownloadItem::INTERRUPTED) { + ASSERT_EQ(DOWNLOAD_INTERRUPT_REASON_CRASH, item->GetLastReason()); + } + + if (param.state == DownloadItem::IN_PROGRESS) { + EXPECT_FALSE(item->GetDownloadSchedule().has_value()) + << "Download schedule should be cleared before completion."; + } + + CleanupItem(item, download_file, param.state); } +TEST_P(DownloadLaterTest, TestOnDownloadScheduleChanged) { + const auto& param = GetParam(); + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitAndEnableFeature(features::kDownloadLater); + + auto item = CreateDownloadItem(DownloadItem::DownloadState::INTERRUPTED, + DOWNLOAD_INTERRUPT_REASON_CRASH); + EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState()); + + // Setup network type and download schedule. + EXPECT_CALL(*mock_delegate(), IsActiveNetworkMetered) + .WillRepeatedly(Return(param.is_active_network_metered)); + + bool will_resume = (param.state == DownloadItem::DownloadState::IN_PROGRESS); + EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_)) + .Times(will_resume); + + // Change the download schedule. + base::Optional<DownloadSchedule> download_schedule = + base::make_optional<DownloadSchedule>(param.only_on_wifi, + param.start_time); + + item->OnDownloadScheduleChanged(std::move(download_schedule)); + + // Verify final states. + ASSERT_EQ(param.allow_metered, item->AllowMetered()); + ASSERT_EQ(param.state, item->GetState()); + ASSERT_EQ(0, item->GetAutoResumeCount()) + << "Download should not be auto resumed with DownloadSchedule."; + if (param.state == DownloadItem::INTERRUPTED) { + ASSERT_EQ(DOWNLOAD_INTERRUPT_REASON_CRASH, item->GetLastReason()); + } +} + +TEST_F(DownloadItemTest, CancelWithDownloadSchedule) { + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitAndEnableFeature(features::kDownloadLater); + + auto item = CreateDownloadItem(DownloadItem::DownloadState::INTERRUPTED, + DOWNLOAD_INTERRUPT_REASON_CRASH); + auto download_schedule = base::make_optional<DownloadSchedule>( + false, base::Time::Now() + base::TimeDelta::FromDays(10)); + item->OnDownloadScheduleChanged(std::move(download_schedule)); + + EXPECT_EQ(item->GetState(), DownloadItem::DownloadState::INTERRUPTED); + EXPECT_TRUE(item->GetDownloadSchedule().has_value()); + item->Cancel(true); + EXPECT_FALSE(item->GetDownloadSchedule().has_value()); +} + +} // namespace } // namespace download diff --git a/chromium/components/download/internal/common/download_stats.cc b/chromium/components/download/internal/common/download_stats.cc index 362b1dd3945..db9e8099e4d 100644 --- a/chromium/components/download/internal/common/download_stats.cc +++ b/chromium/components/download/internal/common/download_stats.cc @@ -803,6 +803,10 @@ void RecordDownloadManagerMemoryUsage(size_t bytes_used) { bytes_used / 1000); } +void RecordDownloadLaterEvent(DownloadLaterEvent event) { + base::UmaHistogramEnumeration("Download.Later.Events", event); +} + #if defined(OS_ANDROID) void RecordBackgroundTargetDeterminationResult( BackgroudTargetDeterminationResultTypes type) { diff --git a/chromium/components/download/internal/common/download_stats_unittest.cc b/chromium/components/download/internal/common/download_stats_unittest.cc index 0046ae96b55..796fb1ae1d6 100644 --- a/chromium/components/download/internal/common/download_stats_unittest.cc +++ b/chromium/components/download/internal/common/download_stats_unittest.cc @@ -70,4 +70,11 @@ TEST(DownloadStatsTest, RecordDownloadCompleted) { ConnectionType::CONNECTION_WIFI, 1); } +TEST(DownloadStatsTest, RecordDownloadLaterEvent) { + base::HistogramTester histogram_tester; + RecordDownloadLaterEvent(DownloadLaterEvent::kScheduleRemoved); + histogram_tester.ExpectBucketCount("Download.Later.Events", + DownloadLaterEvent::kScheduleRemoved, 1); +} + } // namespace download diff --git a/chromium/components/download/internal/common/download_utils.cc b/chromium/components/download/internal/common/download_utils.cc index cde9913f372..11cb466d623 100644 --- a/chromium/components/download/internal/common/download_utils.cc +++ b/chromium/components/download/internal/common/download_utils.cc @@ -428,6 +428,7 @@ DownloadDBEntry CreateDownloadDBEntryFromItem(const DownloadItemImpl& item) { in_progress_info.metered = item.AllowMetered(); in_progress_info.bytes_wasted = item.GetBytesWasted(); in_progress_info.auto_resume_count = item.GetAutoResumeCount(); + in_progress_info.download_schedule = item.GetDownloadSchedule(); download_info.in_progress_info = in_progress_info; 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 5d3a1878d4c..79288ed7f01 100644 --- a/chromium/components/download/internal/common/in_progress_download_manager.cc +++ b/chromium/components/download/internal/common/in_progress_download_manager.cc @@ -68,7 +68,7 @@ std::unique_ptr<DownloadItemImpl> CreateDownloadItemImpl( in_progress_info->interrupt_reason, in_progress_info->paused, in_progress_info->metered, false, base::Time(), in_progress_info->transient, in_progress_info->received_slices, - std::move(download_entry)); + in_progress_info->download_schedule, std::move(download_entry)); } void OnUrlDownloadHandlerCreated( @@ -157,6 +157,7 @@ void OnPathReserved(DownloadItemImplDelegate::DownloadTargetCallback callback, const InProgressDownloadManager::IntermediatePathCallback& intermediate_path_cb, const base::FilePath& forced_file_path, + base::Optional<DownloadSchedule> download_schedule, PathValidationResult result, const base::FilePath& target_path) { base::FilePath intermediate_path; @@ -177,7 +178,7 @@ void OnPathReserved(DownloadItemImplDelegate::DownloadTargetCallback callback, : BackgroudTargetDeterminationResultTypes::kSuccess); std::move(callback).Run( target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, danger_type, - mixed_content_status, intermediate_path, + mixed_content_status, intermediate_path, std::move(download_schedule), intermediate_path.empty() ? DOWNLOAD_INTERRUPT_REASON_FILE_FAILED : DOWNLOAD_INTERRUPT_REASON_NONE); } @@ -391,7 +392,8 @@ void InProgressDownloadManager::DetermineDownloadTarget( std::move(callback).Run( target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, download->GetDangerType(), download->GetMixedContentStatus(), - target_path, DOWNLOAD_INTERRUPT_REASON_FILE_FAILED); + target_path, download->GetDownloadSchedule(), + DOWNLOAD_INTERRUPT_REASON_FILE_FAILED); RecordBackgroundTargetDeterminationResult( BackgroudTargetDeterminationResultTypes::kTargetPathMissing); return; @@ -403,7 +405,8 @@ void InProgressDownloadManager::DetermineDownloadTarget( std::move(callback).Run( target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, download->GetDangerType(), download->GetMixedContentStatus(), - target_path, DOWNLOAD_INTERRUPT_REASON_NONE); + target_path, download->GetDownloadSchedule(), + DOWNLOAD_INTERRUPT_REASON_NONE); RecordBackgroundTargetDeterminationResult( BackgroudTargetDeterminationResultTypes::kSuccess); return; @@ -415,10 +418,10 @@ void InProgressDownloadManager::DetermineDownloadTarget( download->GetForcedFilePath().empty() ? DownloadPathReservationTracker::UNIQUIFY : DownloadPathReservationTracker::OVERWRITE, - base::BindOnce(&OnPathReserved, std::move(callback), - download->GetDangerType(), - download->GetMixedContentStatus(), intermediate_path_cb_, - download->GetForcedFilePath())); + base::BindOnce( + &OnPathReserved, std::move(callback), download->GetDangerType(), + download->GetMixedContentStatus(), intermediate_path_cb_, + download->GetForcedFilePath(), download->GetDownloadSchedule())); #else // For non-android, the code below is only used by tests. base::FilePath intermediate_path = @@ -426,7 +429,8 @@ void InProgressDownloadManager::DetermineDownloadTarget( std::move(callback).Run( target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE, download->GetDangerType(), download->GetMixedContentStatus(), - intermediate_path, DOWNLOAD_INTERRUPT_REASON_NONE); + intermediate_path, download->GetDownloadSchedule(), + DOWNLOAD_INTERRUPT_REASON_NONE); #endif // defined(OS_ANDROID) } diff --git a/chromium/components/download/internal/common/simple_download_manager_coordinator.cc b/chromium/components/download/internal/common/simple_download_manager_coordinator.cc index dfbf5ae1f04..2ae0e442b5b 100644 --- a/chromium/components/download/internal/common/simple_download_manager_coordinator.cc +++ b/chromium/components/download/internal/common/simple_download_manager_coordinator.cc @@ -55,11 +55,10 @@ void SimpleDownloadManagerCoordinator::SetSimpleDownloadManager( } void SimpleDownloadManagerCoordinator::AddObserver(Observer* observer) { + DCHECK(observer); observers_.AddObserver(observer); - if (initialized_) { - for (auto& observer : observers_) - observer.OnDownloadsInitialized(!has_all_history_downloads_); - } + if (initialized_) + observer->OnDownloadsInitialized(!has_all_history_downloads_); } void SimpleDownloadManagerCoordinator::RemoveObserver(Observer* observer) { 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); |