diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2017-09-18 14:34:04 +0200 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2017-10-04 11:15:27 +0000 |
| commit | e6430e577f105ad8813c92e75c54660c4985026e (patch) | |
| tree | 88115e5d1fb471fea807111924dcccbeadbf9e4f /chromium/content/browser/download | |
| parent | 53d399fe6415a96ea6986ec0d402a9c07da72453 (diff) | |
| download | qtwebengine-chromium-e6430e577f105ad8813c92e75c54660c4985026e.tar.gz | |
BASELINE: Update Chromium to 61.0.3163.99
Change-Id: I8452f34574d88ca2b27af9bd56fc9ff3f16b1367
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Diffstat (limited to 'chromium/content/browser/download')
38 files changed, 950 insertions, 473 deletions
diff --git a/chromium/content/browser/download/download_browsertest.cc b/chromium/content/browser/download/download_browsertest.cc index a5debca6eb7..06db334784c 100644 --- a/chromium/content/browser/download/download_browsertest.cc +++ b/chromium/content/browser/download/download_browsertest.cc @@ -63,6 +63,7 @@ #include "net/test/embedded_test_server/http_response.h" #include "net/test/url_request/url_request_mock_http_job.h" #include "net/test/url_request/url_request_slow_download_job.h" +#include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "ppapi/features/features.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -581,9 +582,7 @@ class DownloadContentTest : public ContentBrowserTest { base::FilePath mock_base(GetTestFilePath("download", "")); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, - base::Bind( - &net::URLRequestMockHTTPJob::AddUrlHandlers, mock_base, - make_scoped_refptr(content::BrowserThread::GetBlockingPool()))); + base::Bind(&net::URLRequestMockHTTPJob::AddUrlHandlers, mock_base)); ASSERT_TRUE(embedded_test_server()->Start()); const std::string real_host = embedded_test_server()->host_port_pair().host(); @@ -1734,7 +1733,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, RemoveResumingDownload) { request_start_handler.WaitForCallback(); // At this point, the download resumption request has been sent out, but the - // reponse hasn't been received yet. + // response hasn't been received yet. download->Remove(); request_start_handler.RespondWith(std::string(), net::OK); @@ -2319,6 +2318,34 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ReferrerForPartialResumption) { EXPECT_EQ(document_url.spec(), requests.back()->referrer); } +// Test that the referrer header is dropped for HTTP downloads from HTTPS. +IN_PROC_BROWSER_TEST_F(DownloadContentTest, ReferrerForHTTPS) { + net::EmbeddedTestServer https_origin( + net::EmbeddedTestServer::Type::TYPE_HTTPS); + net::EmbeddedTestServer http_origin(net::EmbeddedTestServer::Type::TYPE_HTTP); + https_origin.ServeFilesFromDirectory(GetTestFilePath("download", "")); + http_origin.RegisterRequestHandler(CreateBasicResponseHandler( + "/download", base::StringPairs(), "application/octet-stream", "Hello")); + ASSERT_TRUE(https_origin.InitializeAndListen()); + ASSERT_TRUE(http_origin.InitializeAndListen()); + + GURL download_url = http_origin.GetURL("/download"); + GURL referrer_url = https_origin.GetURL( + std::string("/download-link.html?dl=") + download_url.spec()); + + https_origin.StartAcceptingConnections(); + http_origin.StartAcceptingConnections(); + + DownloadItem* download = StartDownloadAndReturnItem(shell(), referrer_url); + WaitForCompletion(download); + + ASSERT_EQ(5, download->GetReceivedBytes()); + EXPECT_EQ("", download->GetReferrerUrl().spec()); + + ASSERT_TRUE(https_origin.ShutdownAndWaitUntilComplete()); + ASSERT_TRUE(http_origin.ShutdownAndWaitUntilComplete()); +} + // Check that the cookie policy is correctly updated when downloading a file // that redirects cross origin. IN_PROC_BROWSER_TEST_F(DownloadContentTest, CookiePolicy) { @@ -2345,7 +2372,8 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, CookiePolicy) { SetupEnsureNoPendingDownloads(); std::unique_ptr<DownloadUrlParameters> download_parameters( DownloadUrlParameters::CreateForWebContentsMainFrame( - shell()->web_contents(), origin_two.GetURL("/bar"))); + shell()->web_contents(), origin_two.GetURL("/bar"), + TRAFFIC_ANNOTATION_FOR_TESTS)); std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(shell(), 1)); DownloadManagerForShell(shell())->DownloadUrl(std::move(download_parameters)); observer->WaitForFinished(); @@ -2462,6 +2490,27 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ASSERT_TRUE(origin_two.ShutdownAndWaitUntilComplete()); } +// Test that the suggested filename for data: URLs works. +IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeDataUrl) { + net::EmbeddedTestServer server; + ASSERT_TRUE(server.InitializeAndListen()); + + GURL url = server.GetURL(std::string( + "/download-attribute.html?target=data:application/octet-stream, ...")); + server.ServeFilesFromDirectory(GetTestFilePath("download", "")); + server.StartAcceptingConnections(); + + NavigateToURLAndWaitForDownload(shell(), url, DownloadItem::COMPLETE); + + std::vector<DownloadItem*> downloads; + DownloadManagerForShell(shell())->GetAllDownloads(&downloads); + ASSERT_EQ(1u, downloads.size()); + + EXPECT_EQ(FILE_PATH_LITERAL("suggested-filename"), + downloads[0]->GetTargetFilePath().BaseName().value()); + ASSERT_TRUE(server.ShutdownAndWaitUntilComplete()); +} + // A request for a non-existent resource should still result in a DownloadItem // that's created in an interrupted state. IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeServerError) { diff --git a/chromium/content/browser/download/download_item_impl.cc b/chromium/content/browser/download/download_item_impl.cc index dccaf11c831..a5d7db8cef5 100644 --- a/chromium/content/browser/download/download_item_impl.cc +++ b/chromium/content/browser/download/download_item_impl.cc @@ -62,6 +62,7 @@ #include "net/log/net_log_event_type.h" #include "net/log/net_log_parameters_callback.h" #include "net/log/net_log_source.h" +#include "net/traffic_annotation/network_traffic_annotation.h" namespace content { @@ -132,6 +133,65 @@ const uint32_t DownloadItem::kInvalidId = 0; // The maximum number of attempts we will make to resume automatically. const int DownloadItemImpl::kMaxAutoResumeAttempts = 5; +DownloadItemImpl::RequestInfo::RequestInfo( + const std::vector<GURL>& url_chain, + const GURL& referrer_url, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + const std::string& suggested_filename, + const base::FilePath& forced_file_path, + ui::PageTransition transition_type, + bool has_user_gesture, + const std::string& remote_address, + base::Time start_time) + : url_chain(url_chain), + referrer_url(referrer_url), + site_url(site_url), + tab_url(tab_url), + tab_referrer_url(tab_referrer_url), + suggested_filename(suggested_filename), + forced_file_path(forced_file_path), + transition_type(transition_type), + has_user_gesture(has_user_gesture), + remote_address(remote_address), + start_time(start_time) {} + +DownloadItemImpl::RequestInfo::RequestInfo(const GURL& url) + : url_chain(std::vector<GURL>(1, url)), start_time(base::Time::Now()) {} + +DownloadItemImpl::RequestInfo::RequestInfo() = default; + +DownloadItemImpl::RequestInfo::RequestInfo( + const DownloadItemImpl::RequestInfo& other) = default; + +DownloadItemImpl::RequestInfo::~RequestInfo() = default; + +DownloadItemImpl::DestinationInfo::DestinationInfo( + const base::FilePath& target_path, + const base::FilePath& current_path, + int64_t received_bytes, + bool all_data_saved, + const std::string& hash, + base::Time end_time) + : target_path(target_path), + current_path(current_path), + received_bytes(received_bytes), + all_data_saved(all_data_saved), + hash(hash), + end_time(end_time) {} + +DownloadItemImpl::DestinationInfo::DestinationInfo( + TargetDisposition target_disposition) + : target_disposition(target_disposition) {} + +DownloadItemImpl::DestinationInfo::DestinationInfo() = default; + +DownloadItemImpl::DestinationInfo::DestinationInfo( + const DownloadItemImpl::DestinationInfo& other) = default; + +DownloadItemImpl::DestinationInfo::~DestinationInfo() = default; + // Constructor for reading from the history service. DownloadItemImpl::DownloadItemImpl( DownloadItemImplDelegate* delegate, @@ -161,14 +221,19 @@ DownloadItemImpl::DownloadItemImpl( bool transient, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log) - : guid_(base::ToUpperASCII(guid)), + : request_info_(url_chain, + referrer_url, + site_url, + tab_url, + tab_refererr_url, + std::string(), + base::FilePath(), + ui::PAGE_TRANSITION_LINK, + false, + std::string(), + start_time), + guid_(base::ToUpperASCII(guid)), download_id_(download_id), - target_path_(target_path), - url_chain_(url_chain), - referrer_url_(referrer_url), - site_url_(site_url), - tab_url_(tab_url), - tab_referrer_url_(tab_refererr_url), mime_type_(mime_type), original_mime_type_(original_mime_type), total_bytes_(total_bytes), @@ -176,16 +241,16 @@ DownloadItemImpl::DownloadItemImpl( start_tick_(base::TimeTicks()), state_(ExternalToInternalState(state)), danger_type_(danger_type), - start_time_(start_time), - end_time_(end_time), delegate_(delegate), opened_(opened), last_access_time_(last_access_time), transient_(transient), - current_path_(current_path), - received_bytes_(received_bytes), - all_data_saved_(state == COMPLETE), - hash_(hash), + destination_info_(target_path, + current_path, + received_bytes, + state == COMPLETE, + hash, + end_time), last_modified_time_(last_modified), etag_(etag), received_slices_(received_slices), @@ -203,35 +268,35 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate, uint32_t download_id, const DownloadCreateInfo& info, const net::NetLogWithSource& net_log) - : guid_(info.guid.empty() ? base::ToUpperASCII(base::GenerateGUID()) + : request_info_(info.url_chain, + info.referrer_url, + info.site_url, + info.tab_url, + info.tab_referrer_url, + base::UTF16ToUTF8(info.save_info->suggested_name), + info.save_info->file_path, + info.transition_type ? info.transition_type.value() + : ui::PAGE_TRANSITION_LINK, + info.has_user_gesture, + info.remote_address, + info.start_time), + guid_(info.guid.empty() ? base::ToUpperASCII(base::GenerateGUID()) : info.guid), download_id_(download_id), - target_disposition_((info.save_info->prompt_for_save_location) - ? TARGET_DISPOSITION_PROMPT - : TARGET_DISPOSITION_OVERWRITE), - url_chain_(info.url_chain), - referrer_url_(info.referrer_url), - site_url_(info.site_url), - tab_url_(info.tab_url), - tab_referrer_url_(info.tab_referrer_url), - suggested_filename_(base::UTF16ToUTF8(info.save_info->suggested_name)), - forced_file_path_(info.save_info->file_path), - transition_type_(info.transition_type ? info.transition_type.value() - : ui::PAGE_TRANSITION_LINK), - has_user_gesture_(info.has_user_gesture), response_headers_(info.response_headers), content_disposition_(info.content_disposition), mime_type_(info.mime_type), original_mime_type_(info.original_mime_type), - remote_address_(info.remote_address), total_bytes_(info.total_bytes), last_reason_(info.result), start_tick_(base::TimeTicks::Now()), state_(INITIAL_INTERNAL), - start_time_(info.start_time), delegate_(delegate), is_temporary_(!info.transient && !info.save_info->file_path.empty()), transient_(info.transient), + destination_info_(info.save_info->prompt_for_save_location + ? TARGET_DISPOSITION_PROMPT + : TARGET_DISPOSITION_OVERWRITE), last_modified_time_(info.last_modified), etag_(info.etag), net_log_(net_log), @@ -258,22 +323,19 @@ DownloadItemImpl::DownloadItemImpl( const std::string& mime_type, std::unique_ptr<DownloadRequestHandleInterface> request_handle, const net::NetLogWithSource& net_log) - : is_save_package_download_(true), + : request_info_(url), guid_(base::ToUpperASCII(base::GenerateGUID())), download_id_(download_id), - target_path_(path), - url_chain_(1, url), mime_type_(mime_type), original_mime_type_(mime_type), start_tick_(base::TimeTicks::Now()), state_(IN_PROGRESS_INTERNAL), - start_time_(base::Time::Now()), delegate_(delegate), - current_path_(path), + destination_info_(path, path, 0, false, std::string(), base::Time()), net_log_(net_log), weak_ptr_factory_(this) { - job_ = - base::MakeUnique<DownloadJobImpl>(this, std::move(request_handle), false); + job_ = DownloadJobFactory::CreateJob(this, std::move(request_handle), + DownloadCreateInfo(), true); delegate_->Attach(); Init(true /* actively downloading */, SRC_SAVE_PAGE_AS); } @@ -352,9 +414,9 @@ void DownloadItemImpl::StealDangerousDownload( base::Bind(&DownloadFileDetach, base::Passed(&download_file_)), callback); } else { - callback.Run(current_path_); + callback.Run(GetFullPath()); } - current_path_.clear(); + destination_info_.current_path.clear(); Remove(); // Download item has now been deleted. } else if (download_file_) { @@ -362,7 +424,7 @@ void DownloadItemImpl::StealDangerousDownload( BrowserThread::FILE, FROM_HERE, base::Bind(&MakeCopyOfDownloadFile, download_file_.get()), callback); } else { - callback.Run(current_path_); + callback.Run(GetFullPath()); } } @@ -576,37 +638,39 @@ bool DownloadItemImpl::IsDone() const { } const GURL& DownloadItemImpl::GetURL() const { - return url_chain_.empty() ? GURL::EmptyGURL() : url_chain_.back(); + return request_info_.url_chain.empty() ? GURL::EmptyGURL() + : request_info_.url_chain.back(); } const std::vector<GURL>& DownloadItemImpl::GetUrlChain() const { - return url_chain_; + return request_info_.url_chain; } const GURL& DownloadItemImpl::GetOriginalUrl() const { // Be careful about taking the front() of possibly-empty vectors! // http://crbug.com/190096 - return url_chain_.empty() ? GURL::EmptyGURL() : url_chain_.front(); + return request_info_.url_chain.empty() ? GURL::EmptyGURL() + : request_info_.url_chain.front(); } const GURL& DownloadItemImpl::GetReferrerUrl() const { - return referrer_url_; + return request_info_.referrer_url; } const GURL& DownloadItemImpl::GetSiteUrl() const { - return site_url_; + return request_info_.site_url; } const GURL& DownloadItemImpl::GetTabUrl() const { - return tab_url_; + return request_info_.tab_url; } const GURL& DownloadItemImpl::GetTabReferrerUrl() const { - return tab_referrer_url_; + return request_info_.tab_referrer_url; } std::string DownloadItemImpl::GetSuggestedFilename() const { - return suggested_filename_; + return request_info_.suggested_filename; } const scoped_refptr<const net::HttpResponseHeaders>& @@ -627,16 +691,16 @@ std::string DownloadItemImpl::GetOriginalMimeType() const { } std::string DownloadItemImpl::GetRemoteAddress() const { - return remote_address_; + return request_info_.remote_address; } bool DownloadItemImpl::HasUserGesture() const { - return has_user_gesture_; -}; + return request_info_.has_user_gesture; +} ui::PageTransition DownloadItemImpl::GetTransitionType() const { - return transition_type_; -}; + return request_info_.transition_type; +} const std::string& DownloadItemImpl::GetLastModifiedTime() const { return last_modified_time_; @@ -647,35 +711,35 @@ const std::string& DownloadItemImpl::GetETag() const { } bool DownloadItemImpl::IsSavePackageDownload() const { - return is_save_package_download_; + return job_ && job_->IsSavePackageDownload(); } const base::FilePath& DownloadItemImpl::GetFullPath() const { - return current_path_; + return destination_info_.current_path; } const base::FilePath& DownloadItemImpl::GetTargetFilePath() const { - return target_path_; + return destination_info_.target_path; } const base::FilePath& DownloadItemImpl::GetForcedFilePath() const { // TODO(asanka): Get rid of GetForcedFilePath(). We should instead just // require that clients respect GetTargetFilePath() if it is already set. - return forced_file_path_; + return request_info_.forced_file_path; } base::FilePath DownloadItemImpl::GetFileNameToReportUser() const { if (!display_name_.empty()) return display_name_; - return target_path_.BaseName(); + return GetTargetFilePath().BaseName(); } DownloadItem::TargetDisposition DownloadItemImpl::GetTargetDisposition() const { - return target_disposition_; + return destination_info_.target_disposition; } const std::string& DownloadItemImpl::GetHash() const { - return hash_; + return destination_info_.hash; } bool DownloadItemImpl::GetFileExternallyRemoved() const { @@ -692,7 +756,7 @@ void DownloadItemImpl::DeleteFile(const base::Callback<void(bool)>& callback) { base::WeakPtr<DownloadItemImpl>(), callback, false)); return; } - if (current_path_.empty() || file_externally_removed_) { + if (GetFullPath().empty() || file_externally_removed_) { // Pass a null WeakPtr so it doesn't call OnDownloadedFileRemoved. BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, @@ -702,9 +766,9 @@ void DownloadItemImpl::DeleteFile(const base::Callback<void(bool)>& callback) { } BrowserThread::PostTaskAndReplyWithResult( BrowserThread::FILE, FROM_HERE, - base::Bind(&DeleteDownloadedFile, current_path_), - base::Bind(&DeleteDownloadedFileDone, - weak_ptr_factory_.GetWeakPtr(), callback)); + base::Bind(&DeleteDownloadedFile, GetFullPath()), + base::Bind(&DeleteDownloadedFileDone, weak_ptr_factory_.GetWeakPtr(), + callback)); } bool DownloadItemImpl::IsDangerous() const { @@ -728,8 +792,8 @@ bool DownloadItemImpl::TimeRemaining(base::TimeDelta* remaining) const { if (speed == 0) return false; - *remaining = base::TimeDelta::FromSeconds( - (total_bytes_ - received_bytes_) / speed); + *remaining = + base::TimeDelta::FromSeconds((total_bytes_ - GetReceivedBytes()) / speed); return true; } @@ -745,11 +809,11 @@ int DownloadItemImpl::PercentComplete() const { if (delegate_delayed_complete_ || total_bytes_ <= 0) return -1; - return static_cast<int>(received_bytes_ * 100.0 / total_bytes_); + return static_cast<int>(GetReceivedBytes() * 100.0 / total_bytes_); } bool DownloadItemImpl::AllDataSaved() const { - return all_data_saved_; + return destination_info_.all_data_saved; } int64_t DownloadItemImpl::GetTotalBytes() const { @@ -757,7 +821,7 @@ int64_t DownloadItemImpl::GetTotalBytes() const { } int64_t DownloadItemImpl::GetReceivedBytes() const { - return received_bytes_; + return destination_info_.received_bytes; } const std::vector<DownloadItem::ReceivedSlice>& @@ -766,11 +830,11 @@ DownloadItemImpl::GetReceivedSlices() const { } base::Time DownloadItemImpl::GetStartTime() const { - return start_time_; + return request_info_.start_time; } base::Time DownloadItemImpl::GetEndTime() const { - return end_time_; + return destination_info_.end_time; } bool DownloadItemImpl::CanShowInFolder() { @@ -826,7 +890,8 @@ WebContents* DownloadItemImpl::GetWebContents() const { return nullptr; } -void DownloadItemImpl::OnContentCheckCompleted(DownloadDangerType danger_type) { +void DownloadItemImpl::OnContentCheckCompleted(DownloadDangerType danger_type, + DownloadInterruptReason reason) { DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(AllDataSaved()); @@ -840,6 +905,10 @@ void DownloadItemImpl::OnContentCheckCompleted(DownloadDangerType danger_type) { DVLOG(20) << __func__ << "() danger_type=" << danger_type << " download=" << DebugString(true); SetDangerType(danger_type); + if (reason != DOWNLOAD_INTERRUPT_REASON_NONE) { + InterruptAndDiscardPartialState(reason); + DCHECK_EQ(RESUME_MODE_INVALID, GetResumeMode()); + } UpdateObservers(); } @@ -869,9 +938,9 @@ std::string DownloadItemImpl::DebugString(bool verbose) const { // Construct a string of the URL chain. std::string url_list("<none>"); - if (!url_chain_.empty()) { - std::vector<GURL>::const_iterator iter = url_chain_.begin(); - std::vector<GURL>::const_iterator last = url_chain_.end(); + if (!request_info_.url_chain.empty()) { + std::vector<GURL>::const_iterator iter = request_info_.url_chain.begin(); + std::vector<GURL>::const_iterator last = request_info_.url_chain.end(); url_list = (*iter).is_valid() ? (*iter).spec() : "<invalid>"; ++iter; for ( ; verbose && (iter != last); ++iter) { @@ -928,14 +997,14 @@ DownloadItemImpl::ResumeMode DownloadItemImpl::GetResumeMode() const { // We also can't continue if we don't have some verifier to make sure // we're getting the same file. bool restart_required = - (current_path_.empty() || (etag_.empty() && last_modified_time_.empty())); + (GetFullPath().empty() || (etag_.empty() && last_modified_time_.empty())); // We won't auto-restart if we've used up our attempts or the // download has been paused by user action. bool user_action_required = (auto_resume_count_ >= kMaxAutoResumeAttempts || IsPaused()); - switch(last_reason_) { + switch (last_reason_) { case DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR: case DOWNLOAD_INTERRUPT_REASON_NETWORK_TIMEOUT: case DOWNLOAD_INTERRUPT_REASON_SERVER_CONTENT_LENGTH_MISMATCH: @@ -1029,28 +1098,28 @@ void DownloadItemImpl::UpdateValidatorsOnResumption( // download since the initial request, in order. std::vector<GURL>::const_iterator chain_iter = new_create_info.url_chain.begin(); - if (*chain_iter == url_chain_.back()) + if (*chain_iter == request_info_.url_chain.back()) ++chain_iter; // Record some stats. If the precondition failed (the server returned // HTTP_PRECONDITION_FAILED), then the download will automatically retried as // a full request rather than a partial. Full restarts clobber validators. int origin_state = 0; - bool is_partial = received_bytes_ > 0; + bool is_partial = GetReceivedBytes() > 0; if (chain_iter != new_create_info.url_chain.end()) origin_state |= ORIGIN_STATE_ON_RESUMPTION_ADDITIONAL_REDIRECTS; if (etag_ != new_create_info.etag || last_modified_time_ != new_create_info.last_modified) { received_slices_.clear(); - received_bytes_ = 0; + destination_info_.received_bytes = 0; origin_state |= ORIGIN_STATE_ON_RESUMPTION_VALIDATORS_CHANGED; } if (content_disposition_ != new_create_info.content_disposition) origin_state |= ORIGIN_STATE_ON_RESUMPTION_CONTENT_DISPOSITION_CHANGED; RecordOriginStateOnResumption(is_partial, origin_state); - url_chain_.insert( - url_chain_.end(), chain_iter, new_create_info.url_chain.end()); + request_info_.url_chain.insert(request_info_.url_chain.end(), chain_iter, + new_create_info.url_chain.end()); etag_ = new_create_info.etag; last_modified_time_ = new_create_info.last_modified; response_headers_ = new_create_info.response_headers; @@ -1064,12 +1133,6 @@ void DownloadItemImpl::UpdateValidatorsOnResumption( // notified when the download transitions to the IN_PROGRESS state. } -void DownloadItemImpl::CancelRequestWithOffset(int64_t offset) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - if (job_) - job_->CancelRequestWithOffset(offset); -} - void DownloadItemImpl::NotifyRemoved() { for (auto& observer : observers_) observer.OnDownloadRemoved(this); @@ -1098,8 +1161,8 @@ void DownloadItemImpl::OnAllDataSaved( int64_t total_bytes, std::unique_ptr<crypto::SecureHash> hash_state) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(!all_data_saved_); - all_data_saved_ = true; + DCHECK(!AllDataSaved()); + destination_info_.all_data_saved = true; SetTotalBytes(total_bytes); UpdateProgress(total_bytes, 0); received_slices_.clear(); @@ -1108,11 +1171,11 @@ void DownloadItemImpl::OnAllDataSaved( // the download and don't expect to receive any more // data. - if (received_bytes_at_length_mismatch > 0) { - if (total_bytes > received_bytes_at_length_mismatch) { + if (received_bytes_at_length_mismatch_ > 0) { + if (total_bytes > received_bytes_at_length_mismatch_) { RecordDownloadCount( MORE_BYTES_RECEIVED_AFTER_CONTENT_LENGTH_MISMATCH_COUNT); - } else if (total_bytes == received_bytes_at_length_mismatch) { + } else if (total_bytes == received_bytes_at_length_mismatch_) { RecordDownloadCount( NO_BYTES_RECEIVED_AFTER_CONTENT_LENGTH_MISMATCH_COUNT); } else { @@ -1126,8 +1189,8 @@ void DownloadItemImpl::OnAllDataSaved( void DownloadItemImpl::MarkAsComplete() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(all_data_saved_); - end_time_ = base::Time::Now(); + DCHECK(AllDataSaved()); + destination_info_.end_time = base::Time::Now(); TransitionTo(COMPLETE_INTERNAL); UpdateObservers(); } @@ -1156,7 +1219,7 @@ void DownloadItemImpl::DestinationUpdate( if (net_log_.IsCapturing()) { net_log_.AddEvent( net::NetLogEventType::DOWNLOAD_ITEM_UPDATED, - net::NetLog::Int64Callback("bytes_so_far", received_bytes_)); + net::NetLog::Int64Callback("bytes_so_far", GetReceivedBytes())); } UpdateObservers(); @@ -1206,13 +1269,13 @@ void DownloadItemImpl::Init(bool active, std::string file_name; if (download_type == SRC_HISTORY_IMPORT) { // target_path_ works for History and Save As versions. - file_name = target_path_.AsUTF8Unsafe(); + file_name = GetTargetFilePath().AsUTF8Unsafe(); } else { // See if it's set programmatically. - file_name = forced_file_path_.AsUTF8Unsafe(); + file_name = GetForcedFilePath().AsUTF8Unsafe(); // Possibly has a 'download' attribute for the anchor. if (file_name.empty()) - file_name = suggested_filename_; + file_name = GetSuggestedFilename(); // From the URL file name. if (file_name.empty()) file_name = GetURL().ExtractFileName(); @@ -1243,7 +1306,7 @@ void DownloadItemImpl::Start( download_file_ = std::move(file); job_ = DownloadJobFactory::CreateJob(this, std::move(req_handle), - new_create_info); + new_create_info, false); if (job_->IsParallelizable()) { RecordParallelizableDownloadCount(START_COUNT, IsParallelDownloadEnabled()); } @@ -1267,7 +1330,7 @@ void DownloadItemImpl::Start( DCHECK(state_ == INITIAL_INTERNAL || state_ == RESUMING_INTERNAL); // If the state_ is INITIAL_INTERNAL, then the target path must be empty. - DCHECK(state_ != INITIAL_INTERNAL || target_path_.empty()); + DCHECK(state_ != INITIAL_INTERNAL || GetTargetFilePath().empty()); // If a resumption attempted failed, or if the download was DOA, then the // download should go back to being interrupted. @@ -1284,10 +1347,9 @@ void DownloadItemImpl::Start( ? new_create_info.save_info->hash_state->Clone() : nullptr; - current_path_ = new_create_info.save_info->file_path; - received_bytes_ = offset; + destination_info_.received_bytes = offset; hash_state_ = std::move(hash_state); - hash_.clear(); + destination_info_.hash.clear(); deferred_interrupt_reason_ = new_create_info.result; received_slices_.clear(); TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL); @@ -1318,27 +1380,17 @@ void DownloadItemImpl::Start( // If the download uses parallel requests, and choose not to create parallel // request during resumption, clear the received_slices_ vector. if (!IsParallelDownloadEnabled() && !received_slices_.empty()) { - received_bytes_ = + destination_info_.received_bytes = GetMaxContiguousDataBlockSizeFromBeginning(received_slices_); received_slices_.clear(); } TransitionTo(TARGET_PENDING_INTERNAL); - job_->Start(); -} - -void DownloadItemImpl::StartDownload() { - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - base::Bind(&DownloadFile::Initialize, - // Safe because we control download file lifetime. - base::Unretained(download_file_.get()), - base::Bind(&DownloadItemImpl::OnDownloadFileInitialized, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&DownloadItemImpl::CancelRequestWithOffset, - weak_ptr_factory_.GetWeakPtr()), - received_slices_, job_ && job_->IsParallelizable())); + job_->Start(download_file_.get(), + base::Bind(&DownloadItemImpl::OnDownloadFileInitialized, + weak_ptr_factory_.GetWeakPtr()), + GetReceivedSlices()); } void DownloadItemImpl::OnDownloadFileInitialized( @@ -1402,8 +1454,8 @@ void DownloadItemImpl::OnDownloadTargetDetermined( return; } - target_path_ = target_path; - target_disposition_ = disposition; + destination_info_.target_path = target_path; + destination_info_.target_disposition = disposition; SetDangerType(danger_type); // This was an interrupted download that was looking for a filename. Resolve @@ -1427,7 +1479,7 @@ void DownloadItemImpl::OnDownloadTargetDetermined( // The intermediate name may change from its original value during filename // determination on resumption, for example if the reason for the interruption // was the download target running out space, resulting in a user prompt. - if (intermediate_path == current_path_) { + if (intermediate_path == GetFullPath()) { OnDownloadRenamedToIntermediateName(DOWNLOAD_INTERRUPT_REASON_NONE, intermediate_path); return; @@ -1438,7 +1490,7 @@ void DownloadItemImpl::OnDownloadTargetDetermined( // spurious rename when we can just rename to the final // filename. Unnecessary renames may cause bugs like // http://crbug.com/74187. - DCHECK(!is_save_package_download_); + DCHECK(!IsSavePackageDownload()); DownloadFile::RenameCompletionCallback callback = base::Bind(&DownloadItemImpl::OnDownloadRenamedToIntermediateName, weak_ptr_factory_.GetWeakPtr()); @@ -1494,7 +1546,7 @@ void DownloadItemImpl::OnTargetResolved() { TransitionTo(TARGET_RESOLVED_INTERNAL); if (DOWNLOAD_INTERRUPT_REASON_NONE != deferred_interrupt_reason_) { - InterruptWithPartialState(received_bytes_, std::move(hash_state_), + InterruptWithPartialState(GetReceivedBytes(), std::move(hash_state_), deferred_interrupt_reason_); deferred_interrupt_reason_ = DOWNLOAD_INTERRUPT_REASON_NONE; UpdateObservers(); @@ -1520,7 +1572,7 @@ void DownloadItemImpl::OnTargetResolved() { // complete. void DownloadItemImpl::MaybeCompleteDownload() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(!is_save_package_download_); + DCHECK(!IsSavePackageDownload()); if (!IsDownloadReadyForCompletion( base::Bind(&DownloadItemImpl::MaybeCompleteDownload, @@ -1530,7 +1582,7 @@ void DownloadItemImpl::MaybeCompleteDownload() { // history handle, (validated or safe). DCHECK_EQ(IN_PROGRESS_INTERNAL, state_); DCHECK(!IsDangerous()); - DCHECK(all_data_saved_); + DCHECK(AllDataSaved()); OnDownloadCompleting(); } @@ -1547,16 +1599,6 @@ void DownloadItemImpl::OnDownloadCompleting() { DCHECK(!GetTargetFilePath().empty()); DCHECK(!IsDangerous()); - // TODO(rdsmith/benjhayden): Remove as part of SavePackage integration. - if (is_save_package_download_) { - // Avoid doing anything on the file thread; there's nothing we control - // there. Strictly speaking, this skips giving the embedder a chance to - // open the download. But on a save package download, there's no real - // concept of opening. - Completed(); - return; - } - DCHECK(download_file_.get()); // Unilaterally rename; even if it already has the right name, // we need theannotation. @@ -1579,7 +1621,7 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName( DownloadInterruptReason reason, const base::FilePath& full_path) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(!is_save_package_download_); + DCHECK(!IsSavePackageDownload()); // If a cancel or interrupt hit, we'll cancel the DownloadFile, which // will result in deleting the file on the file thread. So we don't @@ -1599,9 +1641,9 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName( return; } - DCHECK(target_path_ == full_path); + DCHECK(GetTargetFilePath() == full_path); - if (full_path != current_path_) { + if (full_path != GetFullPath()) { // full_path is now the current and target file path. DCHECK(!full_path.empty()); SetFullPath(full_path); @@ -1639,10 +1681,10 @@ void DownloadItemImpl::Completed() { DVLOG(20) << __func__ << "() " << DebugString(false); - DCHECK(all_data_saved_); - end_time_ = base::Time::Now(); + DCHECK(AllDataSaved()); + destination_info_.end_time = base::Time::Now(); TransitionTo(COMPLETE_INTERNAL); - RecordDownloadCompleted(start_tick_, received_bytes_); + RecordDownloadCompleted(start_tick_, GetReceivedBytes()); if (!GetBrowserContext()->IsOffTheRecord()) { RecordDownloadCount(COMPLETED_COUNT_NORMAL_PROFILE); } @@ -1750,15 +1792,15 @@ void DownloadItemImpl::InterruptWithPartialState( return; last_reason_ = reason; - if (!current_path_.empty()) { + if (!GetFullPath().empty()) { // There is no download file and this is transitioning from INTERRUPTED // to CANCELLED. The intermediate file is no longer usable, and should // be deleted. BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, base::Bind(base::IgnoreResult(&DeleteDownloadedFile), - current_path_)); - current_path_.clear(); + GetFullPath())); + destination_info_.current_path.clear(); } break; } @@ -1772,12 +1814,12 @@ void DownloadItemImpl::InterruptWithPartialState( // simply result in a null range request, which would generate a // DestinationCompleted() notification from the DownloadFile, which would // behave properly with setting all_data_saved_ to false here. - all_data_saved_ = false; + destination_info_.all_data_saved = false; - if (current_path_.empty()) { + if (GetFullPath().empty()) { hash_state_.reset(); - hash_.clear(); - received_bytes_ = 0; + destination_info_.hash.clear(); + destination_info_.received_bytes = 0; received_slices_.clear(); } else { UpdateProgress(bytes_so_far, 0); @@ -1806,11 +1848,11 @@ void DownloadItemImpl::InterruptWithPartialState( return; } - RecordDownloadInterrupted(reason, received_bytes_, total_bytes_, + RecordDownloadInterrupted(reason, GetReceivedBytes(), total_bytes_, job_ && job_->IsParallelizable(), IsParallelDownloadEnabled()); if (reason == DOWNLOAD_INTERRUPT_REASON_SERVER_CONTENT_LENGTH_MISMATCH) - received_bytes_at_length_mismatch = received_bytes_; + received_bytes_at_length_mismatch_ = GetReceivedBytes(); if (!GetWebContents()) RecordDownloadCount(INTERRUPTED_WITHOUT_WEBCONTENTS); @@ -1826,12 +1868,12 @@ void DownloadItemImpl::InterruptWithPartialState( void DownloadItemImpl::UpdateProgress( int64_t bytes_so_far, int64_t bytes_per_sec) { - received_bytes_ = bytes_so_far; + destination_info_.received_bytes = bytes_so_far; bytes_per_sec_ = bytes_per_sec; // If we've received more data than we were expecting (bad server info?), // revert to 'unknown size mode'. - if (received_bytes_ > total_bytes_) + if (bytes_so_far > total_bytes_) total_bytes_ = 0; } @@ -1839,14 +1881,14 @@ void DownloadItemImpl::SetHashState( std::unique_ptr<crypto::SecureHash> hash_state) { hash_state_ = std::move(hash_state); if (!hash_state_) { - hash_.clear(); + destination_info_.hash.clear(); return; } std::unique_ptr<crypto::SecureHash> clone_of_hash_state(hash_state_->Clone()); std::vector<char> hash_value(clone_of_hash_state->GetHashLength()); clone_of_hash_state->Finish(&hash_value.front(), hash_value.size()); - hash_.assign(hash_value.begin(), hash_value.end()); + destination_info_.hash.assign(hash_value.begin(), hash_value.end()); } void DownloadItemImpl::ReleaseDownloadFile(bool destroy_file) { @@ -1860,7 +1902,7 @@ void DownloadItemImpl::ReleaseDownloadFile(bool destroy_file) { base::Bind(&DownloadFileCancel, base::Passed(&download_file_))); // Avoid attempting to reuse the intermediate file by clearing out // current_path_ and received slices. - current_path_.clear(); + destination_info_.current_path.clear(); received_slices_.clear(); } else { BrowserThread::PostTask( @@ -1897,9 +1939,9 @@ bool DownloadItemImpl::IsDownloadReadyForCompletion( // target determination calls and the download is in progress, both the target // and current paths should be non-empty and they should point to the same // directory. - DCHECK(!target_path_.empty()); - DCHECK(!current_path_.empty()); - DCHECK(target_path_.DirName() == current_path_.DirName()); + DCHECK(!GetTargetFilePath().empty()); + DCHECK(!GetFullPath().empty()); + DCHECK(GetTargetFilePath().DirName() == GetFullPath().DirName()); // Give the delegate a chance to hold up a stop sign. It'll call // use back through the passed callback if it does and that state changes. @@ -1918,7 +1960,7 @@ void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) { DownloadInternalState old_state = state_; state_ = new_state; - DCHECK(is_save_package_download_ + DCHECK(IsSavePackageDownload() ? IsValidSavePackageStateTransition(old_state, new_state) : IsValidStateTransition(old_state, new_state)) << "Invalid state transition from:" << DebugDownloadStateString(old_state) @@ -1940,9 +1982,9 @@ void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) { break; case IN_PROGRESS_INTERNAL: - DCHECK(!current_path_.empty()) << "Current output path must be known."; - DCHECK(!target_path_.empty()) << "Target path must be known."; - DCHECK(current_path_.DirName() == target_path_.DirName()) + DCHECK(!GetFullPath().empty()) << "Current output path must be known."; + DCHECK(!GetTargetFilePath().empty()) << "Target path must be known."; + DCHECK(GetFullPath().DirName() == GetTargetFilePath().DirName()) << "Current output directory must match target directory."; DCHECK(download_file_) << "Output file must be owned by download item."; DCHECK(job_) << "Must have active download job."; @@ -1952,16 +1994,17 @@ void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) { break; case COMPLETING_INTERNAL: - DCHECK(all_data_saved_) << "All data must be saved prior to completion."; + DCHECK(AllDataSaved()) << "All data must be saved prior to completion."; DCHECK(!download_file_) << "Download file must be released prior to completion."; - DCHECK(!target_path_.empty()) << "Target path must be known."; - DCHECK(current_path_ == target_path_) + DCHECK(!GetTargetFilePath().empty()) << "Target path must be known."; + DCHECK(GetFullPath() == GetTargetFilePath()) << "Current output path must match target path."; net_log_.AddEvent( net::NetLogEventType::DOWNLOAD_ITEM_COMPLETING, - base::Bind(&ItemCompletingNetLogCallback, received_bytes_, &hash_)); + base::Bind(&ItemCompletingNetLogCallback, GetReceivedBytes(), + &destination_info_.hash)); break; case COMPLETE_INTERNAL: @@ -1975,20 +2018,20 @@ void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) { << "Download file must be released prior to interruption."; DCHECK_NE(last_reason_, DOWNLOAD_INTERRUPT_REASON_NONE); net_log_.AddEvent(net::NetLogEventType::DOWNLOAD_ITEM_INTERRUPTED, - base::Bind(&ItemInterruptedNetLogCallback, - last_reason_, received_bytes_)); + base::Bind(&ItemInterruptedNetLogCallback, last_reason_, + GetReceivedBytes())); break; case RESUMING_INTERNAL: net_log_.AddEvent(net::NetLogEventType::DOWNLOAD_ITEM_RESUMED, - base::Bind(&ItemResumingNetLogCallback, false, - last_reason_, received_bytes_)); + base::Bind(&ItemResumingNetLogCallback, false, + last_reason_, GetReceivedBytes())); break; case CANCELLED_INTERNAL: net_log_.AddEvent( net::NetLogEventType::DOWNLOAD_ITEM_CANCELED, - base::Bind(&ItemCanceledNetLogCallback, received_bytes_)); + base::Bind(&ItemCanceledNetLogCallback, GetReceivedBytes())); break; case MAX_DOWNLOAD_INTERNAL_STATE: @@ -2012,7 +2055,7 @@ void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) { // Resumption if (was_done && !is_done) { - std::string file_name(target_path_.BaseName().AsUTF8Unsafe()); + std::string file_name(GetTargetFilePath().BaseName().AsUTF8Unsafe()); net_log_.BeginEvent(net::NetLogEventType::DOWNLOAD_ITEM_ACTIVE, base::Bind(&ItemActivatedNetLogCallback, this, SRC_ACTIVE_DOWNLOAD, &file_name)); @@ -2046,11 +2089,11 @@ void DownloadItemImpl::SetFullPath(const base::FilePath& new_path) { << DebugString(true); DCHECK(!new_path.empty()); - net_log_.AddEvent( - net::NetLogEventType::DOWNLOAD_ITEM_RENAMED, - base::Bind(&ItemRenamedNetLogCallback, ¤t_path_, &new_path)); + net_log_.AddEvent(net::NetLogEventType::DOWNLOAD_ITEM_RENAMED, + base::Bind(&ItemRenamedNetLogCallback, + &destination_info_.current_path, &new_path)); - current_path_ = new_path; + destination_info_.current_path = new_path; } void DownloadItemImpl::AutoResumeIfValid() { @@ -2083,26 +2126,51 @@ void DownloadItemImpl::ResumeInterruptedDownload( ResumeMode mode = GetResumeMode(); if (mode == RESUME_MODE_IMMEDIATE_RESTART || mode == RESUME_MODE_USER_RESTART) { - DCHECK(current_path_.empty()); - received_bytes_ = 0; + DCHECK(GetFullPath().empty()); + destination_info_.received_bytes = 0; last_modified_time_.clear(); etag_.clear(); - hash_.clear(); + destination_info_.hash.clear(); hash_state_.reset(); received_slices_.clear(); } StoragePartition* storage_partition = BrowserContext::GetStoragePartitionForSite(GetBrowserContext(), - site_url_); - + request_info_.site_url); + + net::NetworkTrafficAnnotationTag traffic_annotation = + net::DefineNetworkTrafficAnnotation("download_manager_resume", R"( + semantics { + sender: "Download Manager" + description: + "When user resumes downloading a file, a network request is made " + "to fetch it." + trigger: + "User resumes a download." + data: "None." + destination: WEBSITE + } + policy { + cookies_allowed: true + cookies_store: "user" + setting: + "This feature cannot be disabled in settings, but it is activated " + "by direct user action." + chrome_policy { + DownloadRestrictions { + DownloadRestrictions: 3 + } + } + })"); // Avoid using the WebContents even if it's still around. Resumption requests // are consistently routed through the no-renderer code paths so that the // request will not be dropped if the WebContents (and by extension, the // associated renderer) goes away before a response is received. std::unique_ptr<DownloadUrlParameters> download_params( new DownloadUrlParameters(GetURL(), - storage_partition->GetURLRequestContext())); + storage_partition->GetURLRequestContext(), + traffic_annotation)); download_params->set_file_path(GetFullPath()); if (received_slices_.size() > 0) { std::vector<DownloadItem::ReceivedSlice> slices_to_download @@ -2113,7 +2181,7 @@ void DownloadItemImpl::ResumeInterruptedDownload( } download_params->set_last_modified(GetLastModifiedTime()); download_params->set_etag(GetETag()); - download_params->set_hash_of_partial_file(hash_); + download_params->set_hash_of_partial_file(GetHash()); download_params->set_hash_state(std::move(hash_state_)); // Note that resumed downloads disallow redirects. Hence the referrer URL diff --git a/chromium/content/browser/download/download_item_impl.h b/chromium/content/browser/download/download_item_impl.h index 90dda1d3d38..6d91f344d12 100644 --- a/chromium/content/browser/download/download_item_impl.h +++ b/chromium/content/browser/download/download_item_impl.h @@ -9,6 +9,7 @@ #include <memory> #include <string> +#include <vector> #include "base/callback_forward.h" #include "base/files/file_path.h" @@ -35,12 +36,111 @@ class CONTENT_EXPORT DownloadItemImpl : public DownloadItem, public DownloadDestinationObserver { public: - enum ResumeMode { - RESUME_MODE_INVALID = 0, - RESUME_MODE_IMMEDIATE_CONTINUE, - RESUME_MODE_IMMEDIATE_RESTART, - RESUME_MODE_USER_CONTINUE, - RESUME_MODE_USER_RESTART + // Information about the initial request that triggers the download. Most of + // the fields are immutable after the DownloadItem is successfully created. + // However, it is possible that the url chain is changed when resuming an + // interrupted download. In that case, the download will restart from the + // beginning. + struct CONTENT_EXPORT RequestInfo { + RequestInfo(const std::vector<GURL>& url_chain, + const GURL& referrer_url, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + const std::string& suggested_filename, + const base::FilePath& forced_file_path, + ui::PageTransition transition_type, + bool has_user_gesture, + const std::string& remote_address, + base::Time start_time); + RequestInfo(); + explicit RequestInfo(const RequestInfo& other); + explicit RequestInfo(const GURL& url); + ~RequestInfo(); + + // The chain of redirects that leading up to and including the final URL. + std::vector<GURL> url_chain; + + // The URL of the page that initiated the download. + GURL referrer_url; + + // Site URL for the site instance that initiated this download. + GURL site_url; + + // The URL of the tab that initiated the download. + GURL tab_url; + + // The URL of the referrer of the tab that initiated the download. + GURL tab_referrer_url; + + // Filename suggestion from DownloadSaveInfo. It could, among others, be the + // suggested filename in 'download' attribute of an anchor. Details: + // http://www.whatwg.org/specs/web-apps/current-work/#downloading-hyperlinks + std::string suggested_filename; + + // If non-empty, contains an externally supplied path that should be used as + // the target path. + base::FilePath forced_file_path; + + // Page transition that triggerred the download. + ui::PageTransition transition_type = ui::PAGE_TRANSITION_LINK; + + // Whether the download was triggered with a user gesture. + bool has_user_gesture = false; + + // The remote IP address where the download was fetched from. + std::string remote_address; + + // Time the download was started. + base::Time start_time; + }; + + // Information about the current state of the download destination. + struct CONTENT_EXPORT DestinationInfo { + DestinationInfo(const base::FilePath& target_path, + const base::FilePath& current_path, + int64_t received_bytes, + bool all_data_saved, + const std::string& hash, + base::Time end_time); + DestinationInfo(); + explicit DestinationInfo(TargetDisposition target_disposition); + explicit DestinationInfo(const DestinationInfo& other); + ~DestinationInfo(); + + // Whether the target should be overwritten, uniquified or prompted for. + TargetDisposition target_disposition = TARGET_DISPOSITION_OVERWRITE; + + // Target path of an in-progress download. We may be downloading to a + // temporary or intermediate file (specified by |current_path|). Once the + // download completes, we will rename the file to |target_path|. + base::FilePath target_path; + + // Full path to the downloaded or downloading file. This is the path to the + // physical file, if one exists. The final target path is specified by + // |target_path|. |current_path| can be empty if the in-progress path + // hasn't been determined. + base::FilePath current_path; + + // Current received bytes. + int64_t received_bytes = 0; + + // True if we've saved all the data for the download. If true, then the file + // at |current_path| contains |received_bytes|, which constitute the + // entirety of what we expect to save there. A digest of its contents can be + // found at |hash|. + bool all_data_saved = false; + + // SHA256 hash of the possibly partial content. The hash is updated each + // time the download is interrupted, and when the all the data has been + // transferred. |hash| contains the raw binary hash and is not hex encoded. + // + // While the download is in progress, and while resuming, |hash| will be + // empty. + std::string hash; + + // Time last update was written to target file. + base::Time end_time; }; // The maximum number of attempts we will make to resume automatically. @@ -171,7 +271,8 @@ class CONTENT_EXPORT DownloadItemImpl bool IsTransient() const override; BrowserContext* GetBrowserContext() const override; WebContents* GetWebContents() const override; - void OnContentCheckCompleted(DownloadDangerType danger_type) override; + void OnContentCheckCompleted(DownloadDangerType danger_type, + DownloadInterruptReason reason) override; void SetOpenWhenComplete(bool open) override; void SetOpened(bool opened) override; void SetLastAccessTime(base::Time last_access_time) override; @@ -181,11 +282,6 @@ class CONTENT_EXPORT DownloadItemImpl // All remaining public interfaces virtual to allow for DownloadItemImpl // mocks. - // Determines the resume mode for an interrupted download. Requires - // last_reason_ to be set, but doesn't require the download to be in - // INTERRUPTED state. - virtual ResumeMode GetResumeMode() const; - // State transition operations on regular downloads -------------------------- // Start the download. @@ -204,9 +300,6 @@ class CONTENT_EXPORT DownloadItemImpl // TODO(rdsmith): Unwind DownloadManagerImpl and DownloadItemImpl, // removing these from the public interface. - // Notify observers that this item is being removed by the user. - virtual void NotifyRemoved(); - virtual void OnDownloadedFileRemoved(); // Provide a weak pointer reference to a DownloadDestinationObserver @@ -408,9 +501,6 @@ class CONTENT_EXPORT DownloadItemImpl // this is. void Init(bool active, DownloadType download_type); - // Start a series of events that result in the file being downloaded. - void StartDownload(); - // Callback from file thread when we initialize the DownloadFile. void OnDownloadFileInitialized(DownloadInterruptReason result); @@ -507,8 +597,21 @@ class CONTENT_EXPORT DownloadItemImpl virtual void UpdateValidatorsOnResumption( const DownloadCreateInfo& new_create_info); - // Cancel a particular request that starts from |offset|. - void CancelRequestWithOffset(int64_t offset); + // Notify observers that this item is being removed by the user. + void NotifyRemoved(); + + enum ResumeMode { + RESUME_MODE_INVALID = 0, + RESUME_MODE_IMMEDIATE_CONTINUE, + RESUME_MODE_IMMEDIATE_RESTART, + RESUME_MODE_USER_CONTINUE, + RESUME_MODE_USER_RESTART + }; + + // Determines the resume mode for an interrupted download. Requires + // last_reason_ to be set, but doesn't require the download to be in + // INTERRUPTED state. + ResumeMode GetResumeMode() const; static DownloadState InternalToExternalState( DownloadInternalState internal_state); @@ -523,56 +626,16 @@ class CONTENT_EXPORT DownloadItemImpl static bool IsValidStateTransition(DownloadInternalState from, DownloadInternalState to); - // Will be false for save package downloads retrieved from the history. - // TODO(rdsmith): Replace with a generalized enum for "download source". - const bool is_save_package_download_ = false; + RequestInfo request_info_; std::string guid_; uint32_t download_id_ = kInvalidId; // Display name for the download. If this is empty, then the display name is - // considered to be |target_path_.BaseName()|. + // considered to be |GetTargetFilePath().BaseName()|. base::FilePath display_name_; - // Target path of an in-progress download. We may be downloading to a - // temporary or intermediate file (specified by |current_path_|. Once the - // download completes, we will rename the file to |target_path_|. - base::FilePath target_path_; - - // Whether the target should be overwritten, uniquified or prompted for. - TargetDisposition target_disposition_ = TARGET_DISPOSITION_OVERWRITE; - - // The chain of redirects that leading up to and including the final URL. - std::vector<GURL> url_chain_; - - // The URL of the page that initiated the download. - GURL referrer_url_; - - // Site URL for the site instance that initiated this download. - GURL site_url_; - - // The URL of the tab that initiated the download. - GURL tab_url_; - - // The URL of the referrer of the tab that initiated the download. - GURL tab_referrer_url_; - - // Filename suggestion from DownloadSaveInfo. It could, among others, be the - // suggested filename in 'download' attribute of an anchor. Details: - // http://www.whatwg.org/specs/web-apps/current-work/#downloading-hyperlinks - std::string suggested_filename_; - - // If non-empty, contains an externally supplied path that should be used as - // the target path. - base::FilePath forced_file_path_; - - // Page transition that triggerred the download. - ui::PageTransition transition_type_ = ui::PAGE_TRANSITION_LINK; - - // Whether the download was triggered with a user gesture. - bool has_user_gesture_ = false; - // Information from the response. // The HTTP response headers. This contains a nullptr when the response has @@ -590,10 +653,6 @@ class CONTENT_EXPORT DownloadItemImpl // which may look at the file extension and first few bytes of the file. std::string original_mime_type_; - // The remote IP address where the download was fetched from. Copied from - // DownloadCreateInfo::remote_address. - std::string remote_address_; - // Total bytes expected. int64_t total_bytes_ = 0; @@ -612,12 +671,6 @@ class CONTENT_EXPORT DownloadItemImpl // The views of this item in the download shelf and download contents. base::ObserverList<Observer> observers_; - // Time the download was started. - base::Time start_time_; - - // Time the download completed. - base::Time end_time_; - // Our delegate. DownloadItemImplDelegate* delegate_ = nullptr; @@ -664,36 +717,16 @@ class CONTENT_EXPORT DownloadItemImpl // the IN_PROGRESS state. std::unique_ptr<DownloadFile> download_file_; - // Full path to the downloaded or downloading file. This is the path to the - // physical file, if one exists. The final target path is specified by - // |target_path_|. |current_path_| can be empty if the in-progress path hasn't - // been determined. - base::FilePath current_path_; - - // Current received bytes. - int64_t received_bytes_ = 0; + // Information about |download_file_|. + DestinationInfo destination_info_; // Current speed. Calculated by the DownloadFile. int64_t bytes_per_sec_ = 0; - // True if we've saved all the data for the download. If true, then the file - // at |current_path_| contains |received_bytes_|, which constitute the - // entirety of what we expect to save there. A digest of its contents can be - // found at |hash_|. - bool all_data_saved_ = false; - // The number of times this download has been resumed automatically. Will be // reset to 0 if a resumption is performed in response to a Resume() call. int auto_resume_count_ = 0; - // SHA256 hash of the possibly partial content. The hash is updated each time - // the download is interrupted, and when the all the data has been - // transferred. |hash_| contains the raw binary hash and is not hex encoded. - // - // While the download is in progress, and while resuming, |hash_| will be - // empty. - std::string hash_; - // In the event of an interruption, the DownloadDestinationObserver interface // exposes the partial hash state. This state can be held by the download item // in case it's needed for resumption. @@ -715,7 +748,7 @@ class CONTENT_EXPORT DownloadItemImpl // Value of |received_bytes_| at the time the download was interrupted with // CONTENT_LENGTH_MISMATCH. - int64_t received_bytes_at_length_mismatch = -1; + int64_t received_bytes_at_length_mismatch_ = -1; base::WeakPtrFactory<DownloadItemImpl> weak_ptr_factory_; diff --git a/chromium/content/browser/download/download_item_impl_unittest.cc b/chromium/content/browser/download/download_item_impl_unittest.cc index 3303320887d..4431bdd4c00 100644 --- a/chromium/content/browser/download/download_item_impl_unittest.cc +++ b/chromium/content/browser/download/download_item_impl_unittest.cc @@ -12,6 +12,7 @@ #include <memory> #include <queue> #include <utility> +#include <vector> #include "base/callback.h" #include "base/callback_helpers.h" @@ -119,7 +120,7 @@ class MockRequestHandle : public DownloadRequestHandleInterface { MOCK_CONST_METHOD0(GetDownloadManager, DownloadManager*()); MOCK_CONST_METHOD0(PauseRequest, void()); MOCK_CONST_METHOD0(ResumeRequest, void()); - MOCK_CONST_METHOD0(CancelRequest, void()); + MOCK_CONST_METHOD1(CancelRequest, void(bool)); MOCK_CONST_METHOD0(DebugString, std::string()); }; @@ -523,7 +524,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) { safe_item->OnAllDataSaved(0, std::unique_ptr<crypto::SecureHash>()); EXPECT_TRUE(safe_observer.CheckAndResetDownloadUpdated()); - safe_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS); + safe_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_TRUE(safe_observer.CheckAndResetDownloadUpdated()); CleanupItem(safe_item, download_file, DownloadItem::IN_PROGRESS); @@ -535,7 +537,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) { unsafeurl_item->OnAllDataSaved(0, std::unique_ptr<crypto::SecureHash>()); EXPECT_TRUE(unsafeurl_observer.CheckAndResetDownloadUpdated()); - unsafeurl_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_URL); + unsafeurl_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_URL, + DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_TRUE(unsafeurl_observer.CheckAndResetDownloadUpdated()); EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _)) @@ -552,7 +555,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) { unsafefile_item->OnAllDataSaved(0, std::unique_ptr<crypto::SecureHash>()); EXPECT_TRUE(unsafefile_observer.CheckAndResetDownloadUpdated()); - unsafefile_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE); + unsafefile_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE, + DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_TRUE(unsafefile_observer.CheckAndResetDownloadUpdated()); EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _)) @@ -1127,7 +1131,7 @@ TEST_F(DownloadItemTest, InitDownloadFileFails) { base::MakeUnique<MockRequestHandle>(); EXPECT_CALL(*file, Cancel()); - EXPECT_CALL(*request_handle, CancelRequest()); + EXPECT_CALL(*request_handle, CancelRequest(_)); EXPECT_CALL(*file, Initialize(_, _, _, _)) .WillOnce(ScheduleCallbackWithParam( DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED)); @@ -1758,8 +1762,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_SetDanger) { copy_delegate_callback = delegate_callback; delegate_callback.Reset(); EXPECT_FALSE(item->IsDangerous()); - item->OnContentCheckCompleted( - content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE); + item->OnContentCheckCompleted(content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE, + DOWNLOAD_INTERRUPT_REASON_NONE); EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState()); copy_delegate_callback.Run(); ASSERT_TRUE(delegate_callback.is_null()); @@ -2161,7 +2165,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) { // Expect that the download file and the request will be cancelled as a // result. EXPECT_CALL(*file_, Cancel()); - EXPECT_CALL(*request_handle_, CancelRequest()); + EXPECT_CALL(*request_handle_, CancelRequest(_)); base::RunLoop download_start_loop; DownloadFile::InitializeCallback initialize_callback; @@ -2205,7 +2209,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) { // Expect that the download file and the request will be cancelled as a // result. EXPECT_CALL(*file_, Cancel()); - EXPECT_CALL(*request_handle_, CancelRequest()); + EXPECT_CALL(*request_handle_, CancelRequest(_)); // Intermediate rename loop is not used immediately, but let's set up the // DownloadFile expectations since we are about to transfer its ownership to @@ -2271,7 +2275,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) { // DownloadFile will Detach()). It depends on the list of observations that // are given to us. EXPECT_CALL(*file_, Cancel()).Times(::testing::AnyNumber()); - EXPECT_CALL(*request_handle_, CancelRequest()).Times(::testing::AnyNumber()); + EXPECT_CALL(*request_handle_, CancelRequest(_)).Times(::testing::AnyNumber()); EXPECT_CALL(*file_, Detach()).Times(::testing::AnyNumber()); EXPECT_CALL(*file_, FullPath()) diff --git a/chromium/content/browser/download/download_job.cc b/chromium/content/browser/download/download_job.cc index b7f5dfc5075..5bd23a4291b 100644 --- a/chromium/content/browser/download/download_job.cc +++ b/chromium/content/browser/download/download_job.cc @@ -5,27 +5,65 @@ #include "content/browser/download/download_job.h" #include "base/bind_helpers.h" -#include "content/browser/download/download_file.h" #include "content/browser/download/download_item_impl.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/web_contents.h" namespace content { -DownloadJob::DownloadJob(DownloadItemImpl* download_item) - : download_item_(download_item), is_paused_(false) {} +DownloadJob::DownloadJob( + DownloadItemImpl* download_item, + std::unique_ptr<DownloadRequestHandleInterface> request_handle) + : download_item_(download_item), + request_handle_(std::move(request_handle)), + is_paused_(false), + weak_ptr_factory_(this) {} DownloadJob::~DownloadJob() = default; +void DownloadJob::Cancel(bool user_cancel) { + if (request_handle_) + request_handle_->CancelRequest(user_cancel); +} + void DownloadJob::Pause() { + if (request_handle_) + request_handle_->PauseRequest(); is_paused_ = true; } void DownloadJob::Resume(bool resume_request) { is_paused_ = false; + if (!resume_request) + return; + + if (request_handle_) + request_handle_->ResumeRequest(); +} + +WebContents* DownloadJob::GetWebContents() const { + return request_handle_ ? request_handle_->GetWebContents() : nullptr; +} + +void DownloadJob::Start(DownloadFile* download_file_, + const DownloadFile::InitializeCallback& callback, + const DownloadItem::ReceivedSlices& received_slices) { + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, + base::Bind(&DownloadFile::Initialize, + // Safe because we control download file lifetime. + base::Unretained(download_file_), + base::Bind(&DownloadJob::OnDownloadFileInitialized, + weak_ptr_factory_.GetWeakPtr(), callback), + base::Bind(&DownloadJob::CancelRequestWithOffset, + weak_ptr_factory_.GetWeakPtr()), + received_slices, IsParallelizable())); } -void DownloadJob::StartDownload() const { - download_item_->StartDownload(); +void DownloadJob::OnDownloadFileInitialized( + const DownloadFile::InitializeCallback& callback, + DownloadInterruptReason result) { + callback.Run(result); } bool DownloadJob::AddByteStream(std::unique_ptr<ByteStreamReader> stream_reader, @@ -54,4 +92,8 @@ bool DownloadJob::IsParallelizable() const { return false; } +bool DownloadJob::IsSavePackageDownload() const { + return false; +} + } // namespace content diff --git a/chromium/content/browser/download/download_job.h b/chromium/content/browser/download/download_job.h index cef1c47f4d0..1398ea7883e 100644 --- a/chromium/content/browser/download/download_job.h +++ b/chromium/content/browser/download/download_job.h @@ -6,8 +6,11 @@ #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_JOB_H_ #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "base/time/time.h" #include "content/browser/byte_stream.h" +#include "content/browser/download/download_file.h" +#include "content/browser/download/download_request_handle.h" #include "content/common/content_export.h" #include "content/public/browser/download_danger_type.h" #include "content/public/browser/download_interrupt_reasons.h" @@ -22,12 +25,17 @@ class WebContents; // The base class is a friend class of DownloadItemImpl. class CONTENT_EXPORT DownloadJob { public: - explicit DownloadJob(DownloadItemImpl* download_item); + DownloadJob(DownloadItemImpl* download_item, + std::unique_ptr<DownloadRequestHandleInterface> request_handle); virtual ~DownloadJob(); // Download operations. - virtual void Start() = 0; - virtual void Cancel(bool user_cancel) = 0; + // TODO(qinmin): Remove the |callback| and |download_file_| parameter if + // DownloadJob owns download file. + void Start(DownloadFile* download_file_, + const DownloadFile::InitializeCallback& callback, + const DownloadItem::ReceivedSlices& received_slices); + virtual void Cancel(bool user_cancel); virtual void Pause(); virtual void Resume(bool resume_request); @@ -38,7 +46,7 @@ class CONTENT_EXPORT DownloadJob { // user. // Or return nullptr if the download is not associated with an active // WebContents. - virtual WebContents* GetWebContents() const = 0; + WebContents* GetWebContents() const; // Returns whether the download is parallelizable. The download may not send // parallel requests as it can be disabled through flags. @@ -49,8 +57,14 @@ class CONTENT_EXPORT DownloadJob { // TODO(xingliu): Remove this function if download job owns download file. virtual void CancelRequestWithOffset(int64_t offset); + // Whether the download is save package. + virtual bool IsSavePackageDownload() const; + protected: - void StartDownload() const; + // Callback from file thread when we initialize the DownloadFile. + virtual void OnDownloadFileInitialized( + const DownloadFile::InitializeCallback& callback, + DownloadInterruptReason result); // Add a byte stream to the download sink. Return false if we start to // destroy download file. @@ -60,10 +74,16 @@ class CONTENT_EXPORT DownloadJob { DownloadItemImpl* download_item_; + // Used to perform operations on network request. + // Can be null on interrupted download. + std::unique_ptr<DownloadRequestHandleInterface> request_handle_; + private: // If the download progress is paused by the user. bool is_paused_; + base::WeakPtrFactory<DownloadJob> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(DownloadJob); }; diff --git a/chromium/content/browser/download/download_job_factory.cc b/chromium/content/browser/download/download_job_factory.cc index e68d8000525..a530ccf4254 100644 --- a/chromium/content/browser/download/download_job_factory.cc +++ b/chromium/content/browser/download/download_job_factory.cc @@ -13,6 +13,7 @@ #include "content/browser/download/download_stats.h" #include "content/browser/download/parallel_download_job.h" #include "content/browser/download/parallel_download_utils.h" +#include "content/browser/download/save_package_download_job.h" #include "content/public/common/content_features.h" namespace content { @@ -89,7 +90,13 @@ bool IsParallelizableDownload(const DownloadCreateInfo& create_info) { std::unique_ptr<DownloadJob> DownloadJobFactory::CreateJob( DownloadItemImpl* download_item, std::unique_ptr<DownloadRequestHandleInterface> req_handle, - const DownloadCreateInfo& create_info) { + const DownloadCreateInfo& create_info, + bool is_save_package_download) { + if (is_save_package_download) { + return base::MakeUnique<SavePackageDownloadJob>(download_item, + std::move(req_handle)); + } + bool is_parallelizable = IsParallelizableDownload(create_info); // Build parallel download job. if (IsParallelDownloadEnabled() && is_parallelizable) { diff --git a/chromium/content/browser/download/download_job_factory.h b/chromium/content/browser/download/download_job_factory.h index 10eaa21aafa..fba48956a33 100644 --- a/chromium/content/browser/download/download_job_factory.h +++ b/chromium/content/browser/download/download_job_factory.h @@ -22,7 +22,8 @@ class DownloadJobFactory { static std::unique_ptr<DownloadJob> CreateJob( DownloadItemImpl* download_item, std::unique_ptr<DownloadRequestHandleInterface> req_handle, - const DownloadCreateInfo& create_info); + const DownloadCreateInfo& create_info, + bool is_save_package_download); private: DISALLOW_COPY_AND_ASSIGN(DownloadJobFactory); diff --git a/chromium/content/browser/download/download_job_impl.cc b/chromium/content/browser/download/download_job_impl.cc index 82b6ed86c64..0697da3de9a 100644 --- a/chromium/content/browser/download/download_job_impl.cc +++ b/chromium/content/browser/download/download_job_impl.cc @@ -4,48 +4,17 @@ #include "content/browser/download/download_job_impl.h" -#include "content/public/browser/web_contents.h" - namespace content { DownloadJobImpl::DownloadJobImpl( DownloadItemImpl* download_item, std::unique_ptr<DownloadRequestHandleInterface> request_handle, bool is_parallizable) - : DownloadJob(download_item), - request_handle_(std::move(request_handle)), + : DownloadJob(download_item, std::move(request_handle)), is_parallizable_(is_parallizable) {} DownloadJobImpl::~DownloadJobImpl() = default; -void DownloadJobImpl::Start() { - DownloadJob::StartDownload(); -} - -void DownloadJobImpl::Cancel(bool user_cancel) { - if (request_handle_) - request_handle_->CancelRequest(); -} - -void DownloadJobImpl::Pause() { - DownloadJob::Pause(); - if (request_handle_) - request_handle_->PauseRequest(); -} - -void DownloadJobImpl::Resume(bool resume_request) { - DownloadJob::Resume(resume_request); - if (!resume_request) - return; - - if (request_handle_) - request_handle_->ResumeRequest(); -} - -WebContents* DownloadJobImpl::GetWebContents() const { - return request_handle_ ? request_handle_->GetWebContents() : nullptr; -} - bool DownloadJobImpl::IsParallelizable() const { return is_parallizable_; } diff --git a/chromium/content/browser/download/download_job_impl.h b/chromium/content/browser/download/download_job_impl.h index 59522a7193f..0f5585f996a 100644 --- a/chromium/content/browser/download/download_job_impl.h +++ b/chromium/content/browser/download/download_job_impl.h @@ -8,7 +8,6 @@ #include "base/macros.h" #include "content/browser/download/download_item_impl.h" #include "content/browser/download/download_job.h" -#include "content/browser/download/download_request_handle.h" #include "content/common/content_export.h" namespace content { @@ -22,18 +21,9 @@ class CONTENT_EXPORT DownloadJobImpl : public DownloadJob { ~DownloadJobImpl() override; // DownloadJob implementation. - void Start() override; - void Cancel(bool user_cancel) override; - void Pause() override; - void Resume(bool resume_request) override; - WebContents* GetWebContents() const override; bool IsParallelizable() const override; private: - // Used to perform operations on network request. - // Can be null on interrupted download. - std::unique_ptr<DownloadRequestHandleInterface> request_handle_; - // Whether the download can be parallized. bool is_parallizable_; diff --git a/chromium/content/browser/download/download_manager_impl.cc b/chromium/content/browser/download/download_manager_impl.cc index 02c7f12da9f..d69a4312464 100644 --- a/chromium/content/browser/download/download_manager_impl.cc +++ b/chromium/content/browser/download/download_manager_impl.cc @@ -55,7 +55,7 @@ #include "storage/browser/blob/blob_url_request_job_factory.h" #include "url/origin.h" -#if defined(OS_LINUX) && !defined(OS_CHROMEOS) +#if defined(USE_X11) && !defined(OS_CHROMEOS) #include "base/nix/xdg_util.h" #endif @@ -178,7 +178,7 @@ class DownloadItemFactoryImpl : public DownloadItemFactory { } }; -#if defined(OS_LINUX) && !defined(OS_CHROMEOS) +#if defined(USE_X11) && !defined(OS_CHROMEOS) base::FilePath GetTemporaryDownloadDirectory() { std::unique_ptr<base::Environment> env(base::Environment::Create()); return base::nix::GetXDGDirectory(env.get(), "XDG_DATA_HOME", ".local/share"); @@ -357,7 +357,7 @@ void DownloadManagerImpl::StartDownloadWithId( // If the download is no longer known to the DownloadManager, then it was // removed after it was resumed. Ignore. If the download is cancelled // while resuming, then also ignore the request. - info->request_handle->CancelRequest(); + info->request_handle->CancelRequest(true); if (!on_started.is_null()) on_started.Run(nullptr, DOWNLOAD_INTERRUPT_REASON_USER_CANCELED); // The ByteStreamReader lives and dies on the FILE thread. @@ -370,7 +370,7 @@ void DownloadManagerImpl::StartDownloadWithId( } base::FilePath default_download_directory; -#if defined(OS_LINUX) && !defined(OS_CHROMEOS) +#if defined(USE_X11) && !defined(OS_CHROMEOS) // TODO(thomasanderson): Remove this when all Linux distros with // versions of GTK lower than 3.14.7 are no longer supported. This // should happen when support for Ubuntu Trusty and Debian Jessie @@ -491,12 +491,6 @@ void DownloadManagerImpl::CreateSavePackageDownloadItemWithId( item_created.Run(download_item); } -void DownloadManagerImpl::OnSavePackageSuccessfullyFinished( - DownloadItem* download_item) { - for (auto& observer : observers_) - observer.OnSavePackageSuccessfullyFinished(this, download_item); -} - // Resume a download of a specific URL. We send the request to the // ResourceDispatcherHost, and let it send us responses like a regular // download. diff --git a/chromium/content/browser/download/download_manager_impl.h b/chromium/content/browser/download/download_manager_impl.h index e9b13f69e70..67c48e31692 100644 --- a/chromium/content/browser/download/download_manager_impl.h +++ b/chromium/content/browser/download/download_manager_impl.h @@ -62,9 +62,6 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager, std::unique_ptr<DownloadRequestHandleInterface> request_handle, const DownloadItemImplCreated& item_created); - // Notifies DownloadManager about a successful completion of |download_item|. - void OnSavePackageSuccessfullyFinished(DownloadItem* download_item); - // DownloadManager functions. void SetDelegate(DownloadManagerDelegate* delegate) override; DownloadManagerDelegate* GetDelegate() const override; diff --git a/chromium/content/browser/download/download_manager_impl_unittest.cc b/chromium/content/browser/download/download_manager_impl_unittest.cc index 81adae77821..ad9dd844c80 100644 --- a/chromium/content/browser/download/download_manager_impl_unittest.cc +++ b/chromium/content/browser/download/download_manager_impl_unittest.cc @@ -73,6 +73,12 @@ class ByteStreamReader; namespace { +// Matches a DownloadCreateInfo* that points to the same object as |info| and +// has a |default_download_directory| that matches |download_directory|. +MATCHER_P2(DownloadCreateInfoWithDefaultPath, info, download_directory, "") { + return arg == info && arg->default_download_directory == download_directory; +} + class MockDownloadManagerDelegate : public DownloadManagerDelegate { public: MockDownloadManagerDelegate(); @@ -320,6 +326,7 @@ class MockBrowserContext : public BrowserContext { MOCK_METHOD0(GetSSLHostStateDelegate, SSLHostStateDelegate*()); MOCK_METHOD0(GetPermissionManager, PermissionManager*()); MOCK_METHOD0(GetBackgroundSyncController, BackgroundSyncController*()); + MOCK_METHOD0(GetBrowsingDataRemoverDelegate, BrowsingDataRemoverDelegate*()); MOCK_METHOD0(CreateMediaRequestContext, net::URLRequestContextGetter*()); MOCK_METHOD2(CreateMediaRequestContextForStoragePartition, @@ -546,7 +553,7 @@ TEST_F(DownloadManagerTest, StartDownload) { EXPECT_CALL(GetMockDownloadManagerDelegate(), GetNextId(_)) .WillOnce(RunCallback<0>(local_id)); -#if !defined(OS_LINUX) || defined(OS_CHROMEOS) +#if !defined(USE_X11) || defined(OS_CHROMEOS) // Doing nothing will set the default download directory to null. EXPECT_CALL(GetMockDownloadManagerDelegate(), GetSaveDir(_, _, _, _)); #endif diff --git a/chromium/content/browser/download/download_request_core.cc b/chromium/content/browser/download/download_request_core.cc index 2fd62bc0f7a..ded98502547 100644 --- a/chromium/content/browser/download/download_request_core.cc +++ b/chromium/content/browser/download/download_request_core.cc @@ -32,7 +32,6 @@ #include "content/public/browser/navigation_entry.h" #include "content/public/browser/resource_context.h" #include "content/public/browser/web_contents.h" -#include "device/wake_lock/public/interfaces/wake_lock_provider.mojom.h" #include "net/base/elements_upload_data_stream.h" #include "net/base/io_buffer.h" #include "net/base/load_flags.h" @@ -43,6 +42,7 @@ #include "net/http/http_status_code.h" #include "net/url_request/url_request_context.h" #include "services/device/public/interfaces/constants.mojom.h" +#include "services/device/public/interfaces/wake_lock_provider.mojom.h" #include "services/service_manager/public/cpp/connector.h" namespace content { @@ -128,7 +128,8 @@ std::unique_ptr<net::URLRequest> DownloadRequestCore::CreateRequestOnIOThread( std::unique_ptr<net::URLRequest> request( params->url_request_context_getter() ->GetURLRequestContext() - ->CreateRequest(params->url(), net::DEFAULT_PRIORITY, nullptr)); + ->CreateRequest(params->url(), net::DEFAULT_PRIORITY, nullptr, + params->GetNetworkTrafficAnnotation())); request->set_method(params->method()); if (!params->post_body().empty()) { diff --git a/chromium/content/browser/download/download_request_core.h b/chromium/content/browser/download/download_request_core.h index fc2ccd2ea08..95d31672d72 100644 --- a/chromium/content/browser/download/download_request_core.h +++ b/chromium/content/browser/download/download_request_core.h @@ -18,7 +18,7 @@ #include "content/public/browser/download_interrupt_reasons.h" #include "content/public/browser/download_save_info.h" #include "content/public/browser/download_url_parameters.h" -#include "device/wake_lock/public/interfaces/wake_lock_service.mojom.h" +#include "services/device/public/interfaces/wake_lock.mojom.h" namespace net { class HttpResponseHeaders; @@ -146,7 +146,7 @@ class CONTENT_EXPORT DownloadRequestCore // Used to keep the system from sleeping while a download is ongoing. If the // system enters power saving mode while a URLRequest is alive, it can cause // URLRequest to fail and the associated download will be interrupted. - device::mojom::WakeLockServicePtr wake_lock_; + device::mojom::WakeLockPtr wake_lock_; // The following are used to collect stats. base::TimeTicks download_start_time_; diff --git a/chromium/content/browser/download/download_request_core_unittest.cc b/chromium/content/browser/download/download_request_core_unittest.cc index f2eba00872c..d94c5b3fb01 100644 --- a/chromium/content/browser/download/download_request_core_unittest.cc +++ b/chromium/content/browser/download/download_request_core_unittest.cc @@ -12,6 +12,7 @@ #include "content/public/browser/download_url_parameters.h" #include "content/public/test/test_browser_thread_bundle.h" #include "net/http/http_request_headers.h" +#include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "net/url_request/url_request_test_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -29,7 +30,7 @@ class DownloadRequestCoreTest : public testing::Test { const std::string& url) const { GURL gurl(url); return base::MakeUnique<DownloadUrlParameters>( - gurl, request_context_getter_.get()); + gurl, request_context_getter_.get(), TRAFFIC_ANNOTATION_FOR_TESTS); } void CheckRequestHeaders(const std::string& name, diff --git a/chromium/content/browser/download/download_request_handle.cc b/chromium/content/browser/download/download_request_handle.cc index f5979b945ad..0cde1f84e97 100644 --- a/chromium/content/browser/download/download_request_handle.cc +++ b/chromium/content/browser/download/download_request_handle.cc @@ -54,7 +54,7 @@ void DownloadRequestHandle::ResumeRequest() const { base::Bind(&DownloadResourceHandler::ResumeRequest, handler_)); } -void DownloadRequestHandle::CancelRequest() const { +void DownloadRequestHandle::CancelRequest(bool user_cancel) const { BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, base::Bind(&DownloadResourceHandler::CancelRequest, handler_)); diff --git a/chromium/content/browser/download/download_request_handle.h b/chromium/content/browser/download/download_request_handle.h index 090a25bb5a6..8fbbc101295 100644 --- a/chromium/content/browser/download/download_request_handle.h +++ b/chromium/content/browser/download/download_request_handle.h @@ -34,7 +34,7 @@ class CONTENT_EXPORT DownloadRequestHandleInterface { virtual void ResumeRequest() const = 0; // Cancels the request. - virtual void CancelRequest() const = 0; + virtual void CancelRequest(bool user_cancel) const = 0; }; @@ -62,7 +62,7 @@ class CONTENT_EXPORT DownloadRequestHandle DownloadManager* GetDownloadManager() const override; void PauseRequest() const override; void ResumeRequest() const override; - void CancelRequest() const override; + void CancelRequest(bool user_cancel) const override; private: base::WeakPtr<DownloadResourceHandler> handler_; diff --git a/chromium/content/browser/download/download_stats.cc b/chromium/content/browser/download/download_stats.cc index d008996ce14..1289d61ad30 100644 --- a/chromium/content/browser/download/download_stats.cc +++ b/chromium/content/browser/download/download_stats.cc @@ -640,8 +640,8 @@ void RecordDownloadImageType(const std::string& mime_type_string) { DOWNLOAD_IMAGE_MAX); } -DownloadContent DownloadContentFromMimeType( - const std::string& mime_type_string) { +DownloadContent DownloadContentFromMimeType(const std::string& mime_type_string, + bool record_image_type) { DownloadContent download_content = DOWNLOAD_CONTENT_UNRECOGNIZED; // Look up exact matches. @@ -661,7 +661,8 @@ DownloadContent DownloadContentFromMimeType( } else if (base::StartsWith(mime_type_string, "image/", base::CompareCase::SENSITIVE)) { download_content = DOWNLOAD_CONTENT_IMAGE; - RecordDownloadImageType(mime_type_string); + if (record_image_type) + RecordDownloadImageType(mime_type_string); } else if (base::StartsWith(mime_type_string, "audio/", base::CompareCase::SENSITIVE)) { download_content = DOWNLOAD_CONTENT_AUDIO; @@ -678,15 +679,16 @@ DownloadContent DownloadContentFromMimeType( void RecordDownloadMimeType(const std::string& mime_type_string) { UMA_HISTOGRAM_ENUMERATION("Download.Start.ContentType", - DownloadContentFromMimeType(mime_type_string), + DownloadContentFromMimeType(mime_type_string, true), DOWNLOAD_CONTENT_MAX); } void RecordDownloadMimeTypeForNormalProfile( const std::string& mime_type_string) { - UMA_HISTOGRAM_ENUMERATION("Download.Start.ContentType.NormalProfile", - DownloadContentFromMimeType(mime_type_string), - DOWNLOAD_CONTENT_MAX); + UMA_HISTOGRAM_ENUMERATION( + "Download.Start.ContentType.NormalProfile", + DownloadContentFromMimeType(mime_type_string, false), + DOWNLOAD_CONTENT_MAX); } void RecordDownloadContentDisposition( diff --git a/chromium/content/browser/download/download_worker.cc b/chromium/content/browser/download/download_worker.cc index 5eff1cf3b38..2f564e5f42e 100644 --- a/chromium/content/browser/download/download_worker.cc +++ b/chromium/content/browser/download/download_worker.cc @@ -24,7 +24,7 @@ class CompletedByteStreamReader : public ByteStreamReader { return ByteStreamReader::STREAM_COMPLETE; } int GetStatus() const override { return status_; } - void RegisterCallback(const base::Closure& sink_callback) override {}; + void RegisterCallback(const base::Closure& sink_callback) override {} private: int status_; @@ -57,6 +57,7 @@ DownloadWorker::DownloadWorker(DownloadWorker::Delegate* delegate, length_(length), is_paused_(false), is_canceled_(false), + is_user_cancel_(false), weak_factory_(this) { DCHECK(delegate_); } @@ -86,10 +87,11 @@ void DownloadWorker::Resume() { request_handle_->ResumeRequest(); } -void DownloadWorker::Cancel() { +void DownloadWorker::Cancel(bool user_cancel) { is_canceled_ = true; + is_user_cancel_ = user_cancel; if (request_handle_) - request_handle_->CancelRequest(); + request_handle_->CancelRequest(user_cancel); } void DownloadWorker::OnUrlDownloaderStarted( @@ -102,7 +104,7 @@ void DownloadWorker::OnUrlDownloaderStarted( // Destroy the request if user canceled. if (is_canceled_) { VLOG(kVerboseLevel) << "Byte stream arrived after user cancel the request."; - create_info->request_handle->CancelRequest(); + create_info->request_handle->CancelRequest(is_user_cancel_); return; } diff --git a/chromium/content/browser/download/download_worker.h b/chromium/content/browser/download/download_worker.h index ecfb18266bb..4fd3a951058 100644 --- a/chromium/content/browser/download/download_worker.h +++ b/chromium/content/browser/download/download_worker.h @@ -48,7 +48,7 @@ class CONTENT_EXPORT DownloadWorker : public UrlDownloader::Delegate { // Download operations. void Pause(); void Resume(); - void Cancel(); + void Cancel(bool user_cancel); private: // UrlDownloader::Delegate implementation. @@ -73,6 +73,7 @@ class CONTENT_EXPORT DownloadWorker : public UrlDownloader::Delegate { // States of the worker. bool is_paused_; bool is_canceled_; + bool is_user_cancel_; // Used to control the network request. Live on UI thread. std::unique_ptr<DownloadRequestHandleInterface> request_handle_; diff --git a/chromium/content/browser/download/drag_download_file.cc b/chromium/content/browser/download/drag_download_file.cc index 40c76535530..78611a1b0cd 100644 --- a/chromium/content/browser/download/drag_download_file.cc +++ b/chromium/content/browser/download/drag_download_file.cc @@ -20,6 +20,7 @@ #include "content/public/browser/download_item.h" #include "content/public/browser/download_save_info.h" #include "content/public/browser/download_url_parameters.h" +#include "net/traffic_annotation/network_traffic_annotation.h" namespace content { @@ -67,9 +68,32 @@ class DragDownloadFile::DragDownloadFileUI : public DownloadItem::Observer { RecordDownloadSource(INITIATED_BY_DRAG_N_DROP); // TODO(https://crbug.com/614134) This should use the frame actually // containing the link being dragged rather than the main frame of the tab. + net::NetworkTrafficAnnotationTag traffic_annotation = + net::DefineNetworkTrafficAnnotation("drag_download_file", R"( + semantics { + sender: "Drag To Download" + description: + "Users can download files by dragging them out of browser and into " + "a disk related area." + trigger: "When user drags a file from the browser." + data: "None." + destination: WEBSITE + } + policy { + cookies_allowed: true + cookies_store: "user" + setting: + "This feature cannot be disabled in settings, but it is only " + "activated by direct user action." + chrome_policy { + DownloadRestrictions { + DownloadRestrictions: 3 + } + } + })"); std::unique_ptr<content::DownloadUrlParameters> params( DownloadUrlParameters::CreateForWebContentsMainFrame( - web_contents_, url_)); + web_contents_, url_, traffic_annotation)); params->set_referrer(referrer_); params->set_referrer_encoding(referrer_encoding_); params->set_callback(base::Bind(&DragDownloadFileUI::OnDownloadStarted, diff --git a/chromium/content/browser/download/drag_download_file_browsertest.cc b/chromium/content/browser/download/drag_download_file_browsertest.cc index 6753598964f..93e2a58d24e 100644 --- a/chromium/content/browser/download/drag_download_file_browsertest.cc +++ b/chromium/content/browser/download/drag_download_file_browsertest.cc @@ -7,7 +7,6 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/message_loop/message_loop.h" -#include "base/threading/sequenced_worker_pool.h" #include "content/browser/download/download_file_factory.h" #include "content/browser/download/download_file_impl.h" #include "content/browser/download/download_item_impl.h" @@ -24,7 +23,6 @@ #include "content/shell/browser/shell.h" #include "content/shell/browser/shell_browser_context.h" #include "content/shell/browser/shell_download_manager_delegate.h" -#include "device/power_save_blocker/power_save_blocker.h" #include "net/test/url_request/url_request_mock_http_job.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -77,9 +75,7 @@ class DragDownloadFileTest : public ContentBrowserTest { base::FilePath mock_base(GetTestFilePath("download", "")); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, - base::Bind( - &net::URLRequestMockHTTPJob::AddUrlHandlers, mock_base, - make_scoped_refptr(content::BrowserThread::GetBlockingPool()))); + base::Bind(&net::URLRequestMockHTTPJob::AddUrlHandlers, mock_base)); } const base::FilePath& downloads_directory() const { diff --git a/chromium/content/browser/download/mhtml_generation_browsertest.cc b/chromium/content/browser/download/mhtml_generation_browsertest.cc index e0bc3a25288..e13a93d2d6e 100644 --- a/chromium/content/browser/download/mhtml_generation_browsertest.cc +++ b/chromium/content/browser/download/mhtml_generation_browsertest.cc @@ -376,7 +376,13 @@ IN_PROC_BROWSER_TEST_F(MHTMLGenerationTest, GenerateMHTMLAndExitRenderer) { EXPECT_GT(ReadFileSizeFromDisk(path), 100); // Verify the actual file size. } -IN_PROC_BROWSER_TEST_F(MHTMLGenerationTest, InvalidPath) { +// TODO(crbug.com/672313): Flaky on Windows. +#if defined(OS_WIN) +#define MAYBE_InvalidPath DISABLED_InvalidPath +#else +#define MAYBE_InvalidPath InvalidPath +#endif +IN_PROC_BROWSER_TEST_F(MHTMLGenerationTest, MAYBE_InvalidPath) { base::FilePath path(FILE_PATH_LITERAL("/invalid/file/path")); GenerateMHTML(path, embedded_test_server()->GetURL( diff --git a/chromium/content/browser/download/mhtml_generation_manager.cc b/chromium/content/browser/download/mhtml_generation_manager.cc index 001d30d59f4..df825ba7cac 100644 --- a/chromium/content/browser/download/mhtml_generation_manager.cc +++ b/chromium/content/browser/download/mhtml_generation_manager.cc @@ -253,6 +253,7 @@ MhtmlSaveStatus MHTMLGenerationManager::Job::SendToNextRenderFrame() { ipc_params.mhtml_binary_encoding = params_.use_binary_encoding; ipc_params.mhtml_cache_control_policy = params_.cache_control_policy; ipc_params.mhtml_popup_overlay_removal = params_.remove_popup_overlay; + ipc_params.mhtml_problem_detection = params_.use_page_problem_detectors; int frame_tree_node_id = pending_frame_tree_node_ids_.front(); pending_frame_tree_node_ids_.pop(); @@ -608,8 +609,8 @@ base::File MHTMLGenerationManager::CreateFile(const base::FilePath& file_path) { base::File browser_file(file_path, file_flags); if (!browser_file.IsValid()) { - LOG(ERROR) << "Failed to create file to save MHTML at: " << - file_path.value(); + LOG(ERROR) << "Failed to create file to save MHTML at: " + << file_path.value(); } return browser_file; } diff --git a/chromium/content/browser/download/mock_download_item_impl.h b/chromium/content/browser/download/mock_download_item_impl.h index f0bedb0a6d9..33e4b45a86d 100644 --- a/chromium/content/browser/download/mock_download_item_impl.h +++ b/chromium/content/browser/download/mock_download_item_impl.h @@ -5,6 +5,10 @@ #ifndef CONTENT_BROWSER_DOWNLOAD_MOCK_DOWNLOAD_ITEM_IMPL_H_ #define CONTENT_BROWSER_DOWNLOAD_MOCK_DOWNLOAD_ITEM_IMPL_H_ +#include <memory> +#include <string> +#include <vector> + #include "base/macros.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_file.h" @@ -67,7 +71,8 @@ class MockDownloadItemImpl : public DownloadItemImpl { MOCK_CONST_METHOD0(GetFullPath, const base::FilePath&()); MOCK_CONST_METHOD0(GetTargetFilePath, const base::FilePath&()); MOCK_CONST_METHOD0(GetTargetDisposition, TargetDisposition()); - MOCK_METHOD1(OnContentCheckCompleted, void(DownloadDangerType)); + MOCK_METHOD2(OnContentCheckCompleted, + void(DownloadDangerType, DownloadInterruptReason)); MOCK_CONST_METHOD0(GetState, DownloadState()); MOCK_CONST_METHOD0(GetUrlChain, const std::vector<GURL>&()); MOCK_METHOD1(SetTotalBytes, void(int64_t)); @@ -112,7 +117,6 @@ class MockDownloadItemImpl : public DownloadItemImpl { MOCK_CONST_METHOD0(GetWebContents, WebContents*()); MOCK_CONST_METHOD0(GetFileNameToReportUser, base::FilePath()); MOCK_METHOD1(SetDisplayName, void(const base::FilePath&)); - MOCK_METHOD0(NotifyRemoved, void()); // May be called when vlog is on. std::string DebugString(bool verbose) const override { return std::string(); } }; diff --git a/chromium/content/browser/download/mock_download_job.cc b/chromium/content/browser/download/mock_download_job.cc index fd94c33a958..02f927df6cc 100644 --- a/chromium/content/browser/download/mock_download_job.cc +++ b/chromium/content/browser/download/mock_download_job.cc @@ -7,7 +7,7 @@ namespace content { MockDownloadJob::MockDownloadJob(DownloadItemImpl* download_item) - : DownloadJob(download_item) {} + : DownloadJob(download_item, nullptr) {} MockDownloadJob::~MockDownloadJob() = default; diff --git a/chromium/content/browser/download/mock_download_job.h b/chromium/content/browser/download/mock_download_job.h index 25a4ad0378b..52435616df8 100644 --- a/chromium/content/browser/download/mock_download_job.h +++ b/chromium/content/browser/download/mock_download_job.h @@ -24,7 +24,9 @@ class MockDownloadJob : public DownloadJob { DownloadItemImpl* download_item() { return download_item_; } // DownloadJob implementation. - MOCK_METHOD0(Start, void()); + MOCK_METHOD2(Start, + void(const DownloadFile::InitializeCallback&, + const DownloadItem::ReceivedSlices&)); MOCK_METHOD1(Cancel, void(bool)); MOCK_METHOD0(Pause, void()); MOCK_METHOD1(Resume, void(bool)); diff --git a/chromium/content/browser/download/parallel_download_job.cc b/chromium/content/browser/download/parallel_download_job.cc index 20885bd02e0..4ce373f3bf8 100644 --- a/chromium/content/browser/download/parallel_download_job.cc +++ b/chromium/content/browser/download/parallel_download_job.cc @@ -14,6 +14,7 @@ #include "content/browser/download/parallel_download_utils.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/storage_partition.h" +#include "net/traffic_annotation/network_traffic_annotation.h" namespace content { namespace { @@ -34,10 +35,12 @@ ParallelDownloadJob::ParallelDownloadJob( ParallelDownloadJob::~ParallelDownloadJob() = default; -void ParallelDownloadJob::Start() { - DownloadJobImpl::Start(); - - BuildParallelRequestAfterDelay(); +void ParallelDownloadJob::OnDownloadFileInitialized( + const DownloadFile::InitializeCallback& callback, + DownloadInterruptReason result) { + DownloadJobImpl::OnDownloadFileInitialized(callback, result); + if (result == DOWNLOAD_INTERRUPT_REASON_NONE) + BuildParallelRequestAfterDelay(); } void ParallelDownloadJob::Cancel(bool user_cancel) { @@ -50,7 +53,7 @@ void ParallelDownloadJob::Cancel(bool user_cancel) { } for (auto& worker : workers_) - worker.second->Cancel(); + worker.second->Cancel(user_cancel); } void ParallelDownloadJob::Pause() { @@ -101,7 +104,7 @@ void ParallelDownloadJob::CancelRequestWithOffset(int64_t offset) { auto it = workers_.find(offset); DCHECK(it != workers_.end()); - it->second->Cancel(); + it->second->Cancel(false); } void ParallelDownloadJob::BuildParallelRequestAfterDelay() { @@ -124,7 +127,7 @@ void ParallelDownloadJob::OnByteStreamReady( if (!success) { VLOG(kVerboseLevel) << "Byte stream arrived after download file is released."; - worker->Cancel(); + worker->Cancel(false); } } @@ -209,10 +212,33 @@ void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) { BrowserContext::GetStoragePartitionForSite( download_item_->GetBrowserContext(), download_item_->GetSiteUrl()); + net::NetworkTrafficAnnotationTag traffic_annotation = + net::DefineNetworkTrafficAnnotation("parallel_download_job", R"( + semantics { + sender: "Parallel Download" + description: + "Chrome makes parallel request to speed up download of a file." + trigger: + "When user starts a download request, if it would be technically " + "possible, Chrome starts parallel downloading." + data: "None." + destination: WEBSITE + } + policy { + cookies_allowed: true + cookies_store: "user" + setting: "This feature cannot be disabled in settings." + chrome_policy { + DownloadRestrictions { + DownloadRestrictions: 3 + } + } + })"); // The parallel requests only use GET method. std::unique_ptr<DownloadUrlParameters> download_params( new DownloadUrlParameters(download_item_->GetURL(), - storage_partition->GetURLRequestContext())); + storage_partition->GetURLRequestContext(), + traffic_annotation)); download_params->set_file_path(download_item_->GetFullPath()); download_params->set_last_modified(download_item_->GetLastModifiedTime()); download_params->set_etag(download_item_->GetETag()); diff --git a/chromium/content/browser/download/parallel_download_job.h b/chromium/content/browser/download/parallel_download_job.h index 907bd5df64c..68c648f149d 100644 --- a/chromium/content/browser/download/parallel_download_job.h +++ b/chromium/content/browser/download/parallel_download_job.h @@ -30,13 +30,17 @@ class CONTENT_EXPORT ParallelDownloadJob : public DownloadJobImpl, ~ParallelDownloadJob() override; // DownloadJobImpl implementation. - void Start() override; void Cancel(bool user_cancel) override; void Pause() override; void Resume(bool resume_request) override; void CancelRequestWithOffset(int64_t offset) override; protected: + // DownloadJobImpl implementation. + void OnDownloadFileInitialized( + const DownloadFile::InitializeCallback& callback, + DownloadInterruptReason result) override; + // Virtual for testing. virtual int GetParallelRequestCount() const; virtual int64_t GetMinSliceSize() const; diff --git a/chromium/content/browser/download/parallel_download_job_unittest.cc b/chromium/content/browser/download/parallel_download_job_unittest.cc index 0c730c3f1a5..2690ea4739b 100644 --- a/chromium/content/browser/download/parallel_download_job_unittest.cc +++ b/chromium/content/browser/download/parallel_download_job_unittest.cc @@ -8,6 +8,9 @@ #include <vector> #include "base/memory/ptr_util.h" +#include "base/run_loop.h" +#include "content/browser/download/download_destination_observer.h" +#include "content/browser/download/download_file_impl.h" #include "content/browser/download/download_item_impl_delegate.h" #include "content/browser/download/mock_download_item_impl.h" #include "content/browser/download/parallel_download_utils.h" @@ -15,7 +18,9 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +using ::testing::_; using ::testing::NiceMock; +using ::testing::StrictMock; namespace content { @@ -27,10 +32,35 @@ class MockDownloadRequestHandle : public DownloadRequestHandleInterface { MOCK_CONST_METHOD0(GetDownloadManager, DownloadManager*()); MOCK_CONST_METHOD0(PauseRequest, void()); MOCK_CONST_METHOD0(ResumeRequest, void()); - MOCK_CONST_METHOD0(CancelRequest, void()); + MOCK_CONST_METHOD1(CancelRequest, void(bool)); MOCK_CONST_METHOD0(DebugString, std::string()); }; +class MockDownloadDestinationObserver : public DownloadDestinationObserver { + public: + MOCK_METHOD3(DestinationUpdate, + void(int64_t, + int64_t, + const std::vector<DownloadItem::ReceivedSlice>&)); + void DestinationError( + DownloadInterruptReason reason, + int64_t bytes_so_far, + std::unique_ptr<crypto::SecureHash> hash_state) override {} + void DestinationCompleted( + int64_t total_bytes, + std::unique_ptr<crypto::SecureHash> hash_state) override {} + MOCK_METHOD2(CurrentUpdateStatus, void(int64_t, int64_t)); +}; + +class MockByteStreamReader : public ByteStreamReader { + public: + MOCK_METHOD2(Read, + ByteStreamReader::StreamState(scoped_refptr<net::IOBuffer>*, + size_t*)); + MOCK_CONST_METHOD0(GetStatus, int()); + MOCK_METHOD1(RegisterCallback, void(const base::Closure&)); +}; + } // namespace class ParallelDownloadJobForTest : public ParallelDownloadJob { @@ -106,6 +136,7 @@ class ParallelDownloadJobTest : public testing::Test { job_ = base::MakeUnique<ParallelDownloadJobForTest>( download_item_.get(), std::move(request_handle), info, request_count, min_slice_size, min_remaining_time); + file_initialized_ = false; } void DestroyParallelJob() { @@ -138,10 +169,15 @@ class ParallelDownloadJobTest : public testing::Test { EXPECT_EQ(length, job_->workers_[offset]->length()); } + void OnFileInitialized(DownloadInterruptReason result) { + file_initialized_ = true; + } + content::TestBrowserThreadBundle browser_threads_; std::unique_ptr<DownloadItemImplDelegate> item_delegate_; std::unique_ptr<MockDownloadItemImpl> download_item_; std::unique_ptr<ParallelDownloadJobForTest> job_; + bool file_initialized_; // Request handle for the original request. MockDownloadRequestHandle* mock_request_handle_; }; @@ -248,7 +284,7 @@ TEST_F(ParallelDownloadJobTest, CreateNewDownloadRequestsWithSlices) { // built. TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeBuildRequests) { CreateParallelJob(0, 100, DownloadItem::ReceivedSlices(), 2, 1, 10); - EXPECT_CALL(*mock_request_handle_, CancelRequest()); + EXPECT_CALL(*mock_request_handle_, CancelRequest(_)); // Job is canceled before building parallel requests. job_->Cancel(true); @@ -264,7 +300,7 @@ TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeBuildRequests) { // canceled. TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeByteStreamReady) { CreateParallelJob(0, 100, DownloadItem::ReceivedSlices(), 2, 1, 10); - EXPECT_CALL(*mock_request_handle_, CancelRequest()); + EXPECT_CALL(*mock_request_handle_, CancelRequest(_)); BuildParallelRequests(); VerifyWorker(50, 0); @@ -277,7 +313,7 @@ TEST_F(ParallelDownloadJobTest, EarlyCancelBeforeByteStreamReady) { for (auto& worker : job_->workers()) { std::unique_ptr<MockDownloadRequestHandle> mock_handle = base::MakeUnique<MockDownloadRequestHandle>(); - EXPECT_CALL(*mock_handle.get(), CancelRequest()); + EXPECT_CALL(*mock_handle.get(), CancelRequest(_)); MakeWorkerReady(worker.second.get(), std::move(mock_handle)); } @@ -320,4 +356,33 @@ TEST_F(ParallelDownloadJobTest, RemainingContentWillFinishSoon) { DestroyParallelJob(); } +// Test that parallel request is not created until download file is initialized. +TEST_F(ParallelDownloadJobTest, ParallelRequestNotCreatedUntilFileInitialized) { + auto save_info = base::MakeUnique<DownloadSaveInfo>(); + StrictMock<MockByteStreamReader>* input_stream = + new StrictMock<MockByteStreamReader>(); + auto observer = + base::MakeUnique<StrictMock<MockDownloadDestinationObserver>>(); + base::WeakPtrFactory<DownloadDestinationObserver> observer_factory( + observer.get()); + auto download_file = base::MakeUnique<DownloadFileImpl>( + std::move(save_info), base::FilePath(), + std::unique_ptr<ByteStreamReader>(input_stream), net::NetLogWithSource(), + observer_factory.GetWeakPtr()); + CreateParallelJob(0, 100, DownloadItem::ReceivedSlices(), 2, 0, 0); + job_->Start(download_file.get(), + base::Bind(&ParallelDownloadJobTest::OnFileInitialized, + base::Unretained(this)), + DownloadItem::ReceivedSlices()); + EXPECT_FALSE(file_initialized_); + EXPECT_EQ(0u, job_->workers().size()); + EXPECT_CALL(*input_stream, RegisterCallback(_)); + EXPECT_CALL(*input_stream, Read(_, _)); + EXPECT_CALL(*(observer.get()), DestinationUpdate(_, _, _)); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(file_initialized_); + EXPECT_EQ(1u, job_->workers().size()); + DestroyParallelJob(); +} + } // namespace content diff --git a/chromium/content/browser/download/save_file_manager.cc b/chromium/content/browser/download/save_file_manager.cc index 256ef281137..982eeac812a 100644 --- a/chromium/content/browser/download/save_file_manager.cc +++ b/chromium/content/browser/download/save_file_manager.cc @@ -25,6 +25,7 @@ #include "content/public/common/previews_state.h" #include "net/base/io_buffer.h" #include "net/base/load_flags.h" +#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_request.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_job_factory.h" @@ -307,8 +308,27 @@ void SaveFileManager::OnSaveURL(const GURL& url, return; } - std::unique_ptr<net::URLRequest> request( - request_context->CreateRequest(url, net::DEFAULT_PRIORITY, NULL)); + net::NetworkTrafficAnnotationTag traffic_annotation = + net::DefineNetworkTrafficAnnotation("save_file_manager", R"( + semantics { + sender: "Save File" + description: "Saving url to local file." + trigger: + "User clicks on 'Save link as...' context menu command to save a " + "link." + data: "None." + destination: WEBSITE + } + policy { + cookies_allowed: true + cookies_store: "user" + setting: + "This feature cannot be disable by settings. The request is made " + "only if user chooses 'Save link as...' in context menu." + policy_exception_justification: "Not implemented." + })"); + std::unique_ptr<net::URLRequest> request(request_context->CreateRequest( + url, net::DEFAULT_PRIORITY, NULL, traffic_annotation)); request->set_method("GET"); // The URLRequest needs to be initialized with the referrer and other diff --git a/chromium/content/browser/download/save_package.cc b/chromium/content/browser/download/save_package.cc index 3f9a643627e..b6b6bd454ff 100644 --- a/chromium/content/browser/download/save_package.cc +++ b/chromium/content/browser/download/save_package.cc @@ -117,7 +117,7 @@ bool CanSaveAsComplete(const std::string& contents_mime_type) { } // Request handle for SavePackage downloads. Currently doesn't support -// pause/resume/cancel, but returns a WebContents. +// pause/resume, but returns a WebContents. class SavePackageRequestHandle : public DownloadRequestHandleInterface { public: explicit SavePackageRequestHandle(base::WeakPtr<SavePackage> save_package) @@ -130,7 +130,10 @@ class SavePackageRequestHandle : public DownloadRequestHandleInterface { DownloadManager* GetDownloadManager() const override { return nullptr; } void PauseRequest() const override {} void ResumeRequest() const override {} - void CancelRequest() const override {} + void CancelRequest(bool user_cancel) const override { + if (save_package_.get() && !save_package_->canceled()) + save_package_->Cancel(user_cancel, false); + } private: base::WeakPtr<SavePackage> save_package_; @@ -189,8 +192,11 @@ SavePackage::~SavePackage() { // We should no longer be observing the DownloadItem at this point. CHECK(!download_); - DCHECK_EQ(all_save_items_count_, waiting_item_queue_.size() + - completed_count() + in_process_count()); + DCHECK_EQ(all_save_items_count_, + waiting_item_queue_.size() + completed_count() + in_process_count()) + << "waiting: " << waiting_item_queue_.size() + << " completed: " << completed_count() + << " in_progress: " << in_process_count(); // Free all SaveItems. waiting_item_queue_.clear(); @@ -219,14 +225,14 @@ GURL SavePackage::GetUrlToBeSaved(WebContents* web_contents) { return visible_entry ? visible_entry->GetURL() : GURL::EmptyGURL(); } -void SavePackage::Cancel(bool user_action) { +void SavePackage::Cancel(bool user_action, bool cancel_download_item) { DCHECK_CURRENTLY_ON(BrowserThread::UI); if (!canceled()) { if (user_action) user_canceled_ = true; else disk_error_occurred_ = true; - Stop(); + Stop(cancel_download_item); } RecordSavePackageEvent(SAVE_PACKAGE_CANCELLED); } @@ -283,7 +289,6 @@ void SavePackage::InitWithDownloadItem( DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(item); download_ = item; - download_->AddObserver(this); // Confirm above didn't delete the tab out from under us. if (!download_created_callback.is_null()) download_created_callback.Run(download_); @@ -315,21 +320,17 @@ void SavePackage::InitWithDownloadItem( void SavePackage::OnMHTMLGenerated(int64_t size) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + if (!download_) + return; + + CHECK_EQ(download_->GetState(), DownloadItem::IN_PROGRESS); if (size <= 0) { Cancel(false); return; } wrote_to_completed_file_ = true; - // Hack to avoid touching download_ after user cancel. - // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem - // with SavePackage flow. - if (download_->GetState() == DownloadItem::IN_PROGRESS) { - // Must call OnAllDataSaved here in order for - // GDataDownloadObserver::ShouldUpload() to return true. - // ShouldCompleteDownload() may depend on the gdata uploader to finish. - download_->OnAllDataSaved(size, std::unique_ptr<crypto::SecureHash>()); - } + download_->OnAllDataSaved(size, std::unique_ptr<crypto::SecureHash>()); if (!download_manager_->GetDelegate()) { Finish(); @@ -601,7 +602,7 @@ bool SavePackage::UpdateSaveProgress(SaveItemId save_item_id, // Stop all page saving jobs that are in progress and instruct the FILE thread // to delete all saved files. -void SavePackage::Stop() { +void SavePackage::Stop(bool cancel_download_item) { DCHECK_CURRENTLY_ON(BrowserThread::UI); // If we haven't moved out of the initial state, there's nothing to cancel and // there won't be valid pointers for |file_manager_| or |download_|. @@ -641,7 +642,8 @@ void SavePackage::Stop() { // Inform the DownloadItem we have canceled whole save page job. if (download_) { - download_->Cancel(false); + if (cancel_download_item) + download_->Cancel(false); FinalizeDownloadEntry(); } } @@ -705,19 +707,15 @@ void SavePackage::Finish() { list_of_failed_save_item_ids)); if (download_) { - // Hack to avoid touching |download_| after user cancel. - // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem - // with SavePackage flow. - if (download_->GetState() == DownloadItem::IN_PROGRESS) { - if (save_type_ != SAVE_PAGE_TYPE_AS_MHTML) { - download_->DestinationUpdate( - all_save_items_count_, CurrentSpeed(), - std::vector<DownloadItem::ReceivedSlice>()); - download_->OnAllDataSaved(all_save_items_count_, - std::unique_ptr<crypto::SecureHash>()); - } - download_->MarkAsComplete(); + if (save_type_ != SAVE_PAGE_TYPE_AS_MHTML) { + CHECK_EQ(download_->GetState(), DownloadItem::IN_PROGRESS); + download_->DestinationUpdate( + all_save_items_count_, CurrentSpeed(), + std::vector<DownloadItem::ReceivedSlice>()); + download_->OnAllDataSaved(all_save_items_count_, + std::unique_ptr<crypto::SecureHash>()); } + download_->MarkAsComplete(); FinalizeDownloadEntry(); } } @@ -741,10 +739,8 @@ void SavePackage::SaveFinished(SaveItemId save_item_id, // Inform the DownloadItem to update UI. // We use the received bytes as number of saved files. - // Hack to avoid touching download_ after user cancel. - // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem - // with SavePackage flow. - if (download_ && (download_->GetState() == DownloadItem::IN_PROGRESS)) { + if (download_) { + CHECK_EQ(download_->GetState(), DownloadItem::IN_PROGRESS); download_->DestinationUpdate( completed_count(), CurrentSpeed(), std::vector<DownloadItem::ReceivedSlice>()); @@ -1089,6 +1085,7 @@ void SavePackage::GetSavableResourceLinks() { EnqueueFrame(FrameTreeNode::kFrameTreeNodeInvalidId, // No container. main_frame_tree_node->frame_tree_node_id(), main_frame_tree_node->current_url()); + all_save_items_count_ = 1; } void SavePackage::OnSavableResourceLinksResponse( @@ -1227,10 +1224,7 @@ void SavePackage::CompleteSavableResourceLinksResponse() { all_save_items_count_ = static_cast<int>(waiting_item_queue_.size()); // We use total bytes as the total number of files we want to save. - // Hack to avoid touching download_ after user cancel. - // TODO(rdsmith/benjhayden): Integrate canceling on DownloadItem - // with SavePackage flow. - if (download_ && (download_->GetState() == DownloadItem::IN_PROGRESS)) + if (download_) download_->SetTotalBytes(all_save_items_count_); if (all_save_items_count_) { @@ -1470,25 +1464,11 @@ void SavePackage::OnPathPicked( Init(download_created_callback); } -void SavePackage::RemoveObservers() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); +void SavePackage::FinalizeDownloadEntry() { DCHECK(download_); DCHECK(download_manager_); - - download_->RemoveObserver(this); download_ = nullptr; download_manager_ = nullptr; } -void SavePackage::OnDownloadDestroyed(DownloadItem* download) { - RemoveObservers(); -} - -void SavePackage::FinalizeDownloadEntry() { - DCHECK(download_); - - download_manager_->OnSavePackageSuccessfullyFinished(download_); - RemoveObservers(); -} - } // namespace content diff --git a/chromium/content/browser/download/save_package.h b/chromium/content/browser/download/save_package.h index 74ea31fc96e..7a4fb8ce2fa 100644 --- a/chromium/content/browser/download/save_package.h +++ b/chromium/content/browser/download/save_package.h @@ -62,7 +62,6 @@ class WebContents; class CONTENT_EXPORT SavePackage : public base::RefCountedThreadSafe<SavePackage>, public WebContentsObserver, - public DownloadItem::Observer, public base::SupportsWeakPtr<SavePackage> { public: enum WaitState { @@ -97,7 +96,7 @@ class CONTENT_EXPORT SavePackage bool Init(const SavePackageDownloadCreatedCallback& cb); // Cancel all in progress request, might be called by user or internal error. - void Cancel(bool user_action); + void Cancel(bool user_action, bool cancel_download_item = true); void Finish(); @@ -133,6 +132,7 @@ class CONTENT_EXPORT SavePackage FRIEND_TEST_ALL_PREFIXES(SavePackageTest, TestLongSafePureFilename); FRIEND_TEST_ALL_PREFIXES(SavePackageBrowserTest, ImplicitCancel); FRIEND_TEST_ALL_PREFIXES(SavePackageBrowserTest, ExplicitCancel); + FRIEND_TEST_ALL_PREFIXES(SavePackageBrowserTest, DownloadItemDestroyed); // Map from SaveItem::id() (aka save_item_id) into a SaveItem. using SaveItemIdMap = std:: @@ -164,7 +164,7 @@ class CONTENT_EXPORT SavePackage // Notes from Init() above applies here as well. void InternalInit(); - void Stop(); + void Stop(bool cancel_download_item); void CheckFinish(); // Initiate a saving job of a specific URL. We send the request to @@ -180,15 +180,9 @@ class CONTENT_EXPORT SavePackage bool OnMessageReceived(const IPC::Message& message, RenderFrameHost* render_frame_host) override; - // DownloadItem::Observer implementation. - void OnDownloadDestroyed(DownloadItem* download) override; - // Update the download history of this item upon completion. void FinalizeDownloadEntry(); - // Detach from DownloadManager. - void RemoveObservers(); - // Return max length of a path for a specific base directory. // This is needed on POSIX, which restrict the length of file names in // addition to the restriction on the length of path names. diff --git a/chromium/content/browser/download/save_package_browsertest.cc b/chromium/content/browser/download/save_package_browsertest.cc index 2e3fa59ba33..c8ad8b418e7 100644 --- a/chromium/content/browser/download/save_package_browsertest.cc +++ b/chromium/content/browser/download/save_package_browsertest.cc @@ -2,17 +2,77 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/bind.h" #include "base/files/scoped_temp_dir.h" +#include "base/run_loop.h" +#include "base/threading/sequenced_task_runner_handle.h" +#include "content/browser/download/download_manager_impl.h" #include "content/browser/download/save_package.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/download_manager.h" +#include "content/public/browser/web_contents.h" #include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test_utils.h" +#include "content/public/test/download_test_observer.h" #include "content/shell/browser/shell.h" +#include "content/shell/browser/shell_download_manager_delegate.h" #include "net/test/embedded_test_server/embedded_test_server.h" namespace content { const char kTestFile[] = "/simple_page.html"; +class TestShellDownloadManagerDelegate : public ShellDownloadManagerDelegate { + public: + explicit TestShellDownloadManagerDelegate(SavePageType save_page_type) + : save_page_type_(save_page_type) {} + + void ChooseSavePath(WebContents* web_contents, + const base::FilePath& suggested_path, + const base::FilePath::StringType& default_extension, + bool can_save_as_complete, + const SavePackagePathPickedCallback& callback) override { + callback.Run(suggested_path, save_page_type_, + SavePackageDownloadCreatedCallback()); + } + + void GetSaveDir(BrowserContext* context, + base::FilePath* website_save_dir, + base::FilePath* download_save_dir, + bool* skip_dir_check) override { + *website_save_dir = download_dir_; + *download_save_dir = download_dir_; + *skip_dir_check = false; + } + + bool ShouldCompleteDownload(DownloadItem* download, + const base::Closure& closure) override { + return true; + } + + base::FilePath download_dir_; + SavePageType save_page_type_; +}; + +class DownloadicidalObserver : public DownloadManager::Observer { + public: + explicit DownloadicidalObserver(bool remove_download) + : remove_download_(remove_download) {} + void OnDownloadCreated(DownloadManager* manager, + DownloadItem* item) override { + base::SequencedTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind( + [](bool remove_download, const base::Closure& closure, + DownloadItem* item) { + remove_download ? item->Remove() : item->Cancel(true); + closure.Run(); + }, + remove_download_, quit_closure_, item)); + } + base::Closure quit_closure_; + bool remove_download_; +}; + class SavePackageBrowserTest : public ContentBrowserTest { protected: void SetUp() override { @@ -28,6 +88,51 @@ class SavePackageBrowserTest : public ContentBrowserTest { *dir = save_dir_.GetPath().AppendASCII(prefix + "_files"); } + // Start a SavePackage download and then cancels it. If |remove_download| is + // true, the download item will be removed while page is being saved. + // Otherwise, the download item will be canceled. + void RunAndCancelSavePackageDownload(SavePageType save_page_type, + bool remove_download) { + ASSERT_TRUE(embedded_test_server()->Start()); + GURL url = embedded_test_server()->GetURL("/page_with_iframe.html"); + NavigateToURL(shell(), url); + auto* download_manager = + static_cast<DownloadManagerImpl*>(BrowserContext::GetDownloadManager( + shell()->web_contents()->GetBrowserContext())); + auto delegate = + base::MakeUnique<TestShellDownloadManagerDelegate>(save_page_type); + delegate->download_dir_ = save_dir_.GetPath(); + auto* old_delegate = download_manager->GetDelegate(); + download_manager->SetDelegate(delegate.get()); + + { + base::RunLoop run_loop; + DownloadicidalObserver download_item_killer(false); + download_manager->AddObserver(&download_item_killer); + download_item_killer.quit_closure_ = run_loop.QuitClosure(); + + scoped_refptr<SavePackage> save_package( + new SavePackage(shell()->web_contents())); + save_package->GetSaveInfo(); + run_loop.Run(); + download_manager->RemoveObserver(&download_item_killer); + EXPECT_TRUE(save_package->canceled()); + } + + // Run a second download to completion so that any pending tasks will get + // flushed out. If the previous SavePackage operation didn't cleanup after + // itself, then there could be stray tasks that invoke the now defunct + // download item. + { + base::RunLoop run_loop; + SavePackageFinishedObserver finished_observer(download_manager, + run_loop.QuitClosure()); + shell()->web_contents()->OnSavePage(); + run_loop.Run(); + } + download_manager->SetDelegate(old_delegate); + } + // Temporary directory we will save pages to. base::ScopedTempDir save_dir_; }; @@ -59,4 +164,12 @@ IN_PROC_BROWSER_TEST_F(SavePackageBrowserTest, ExplicitCancel) { save_package->Cancel(true); } +IN_PROC_BROWSER_TEST_F(SavePackageBrowserTest, DownloadItemDestroyed) { + RunAndCancelSavePackageDownload(SAVE_PAGE_TYPE_AS_COMPLETE_HTML, true); +} + +IN_PROC_BROWSER_TEST_F(SavePackageBrowserTest, DownloadItemCanceled) { + RunAndCancelSavePackageDownload(SAVE_PAGE_TYPE_AS_MHTML, false); +} + } // namespace content diff --git a/chromium/content/browser/download/save_package_download_job.cc b/chromium/content/browser/download/save_package_download_job.cc new file mode 100644 index 00000000000..c5a9b539067 --- /dev/null +++ b/chromium/content/browser/download/save_package_download_job.cc @@ -0,0 +1,20 @@ +// Copyright 2017 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 "content/browser/download/save_package_download_job.h" + +namespace content { + +SavePackageDownloadJob::SavePackageDownloadJob( + DownloadItemImpl* download_item, + std::unique_ptr<DownloadRequestHandleInterface> request_handle) + : DownloadJob(download_item, std::move(request_handle)) {} + +SavePackageDownloadJob::~SavePackageDownloadJob() = default; + +bool SavePackageDownloadJob::IsSavePackageDownload() const { + return true; +} + +} // namespace content diff --git a/chromium/content/browser/download/save_package_download_job.h b/chromium/content/browser/download/save_package_download_job.h new file mode 100644 index 00000000000..fb1bf7f6f2e --- /dev/null +++ b/chromium/content/browser/download/save_package_download_job.h @@ -0,0 +1,32 @@ +// Copyright 2017 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 CONTENT_BROWSER_DOWNLOAD_SAVE_PACKAGE_DOWNLOAD_JOB_H_ +#define CONTENT_BROWSER_DOWNLOAD_SAVE_PACKAGE_DOWNLOAD_JOB_H_ + +#include "base/macros.h" +#include "content/browser/download/download_item_impl.h" +#include "content/browser/download/download_job.h" +#include "content/browser/download/download_request_handle.h" +#include "content/common/content_export.h" + +namespace content { + +class CONTENT_EXPORT SavePackageDownloadJob : public DownloadJob { + public: + SavePackageDownloadJob( + DownloadItemImpl* download_item, + std::unique_ptr<DownloadRequestHandleInterface> request_handle); + ~SavePackageDownloadJob() override; + + // DownloadJob implementation. + bool IsSavePackageDownload() const override; + + private: + DISALLOW_COPY_AND_ASSIGN(SavePackageDownloadJob); +}; + +} // namespace content + +#endif // CONTENT_BROWSER_DOWNLOAD_SAVE_PACKAGE_DOWNLOAD_JOB_H_ diff --git a/chromium/content/browser/download/url_downloader.cc b/chromium/content/browser/download/url_downloader.cc index ee70480eb2d..be20bb891de 100644 --- a/chromium/content/browser/download/url_downloader.cc +++ b/chromium/content/browser/download/url_downloader.cc @@ -50,7 +50,7 @@ class UrlDownloader::RequestHandle : public DownloadRequestHandleInterface { downloader_task_runner_->PostTask( FROM_HERE, base::Bind(&UrlDownloader::ResumeRequest, downloader_)); } - void CancelRequest() const override { + void CancelRequest(bool user_cancel) const override { downloader_task_runner_->PostTask( FROM_HERE, base::Bind(&UrlDownloader::CancelRequest, downloader_)); } @@ -68,7 +68,9 @@ std::unique_ptr<UrlDownloader> UrlDownloader::BeginDownload( std::unique_ptr<net::URLRequest> request, const Referrer& referrer, bool is_parallel_request) { - Referrer::SetReferrerForRequest(request.get(), referrer); + Referrer sanitized_referrer = + Referrer::SanitizeForRequest(request->url(), referrer); + Referrer::SetReferrerForRequest(request.get(), sanitized_referrer); if (request->url().SchemeIs(url::kBlobScheme)) return nullptr; |