diff options
Diffstat (limited to 'chromium/components/download/internal')
13 files changed, 362 insertions, 103 deletions
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) { |