diff options
-rw-r--r-- | content/browser/download/download_browsertest.cc | 83 | ||||
-rw-r--r-- | content/browser/download/download_create_info.cc | 37 | ||||
-rw-r--r-- | content/browser/download/download_create_info.h | 4 | ||||
-rw-r--r-- | content/browser/download/download_manager_impl.cc | 34 | ||||
-rw-r--r-- | content/browser/download/download_manager_impl_unittest.cc | 2 | ||||
-rw-r--r-- | content/browser/download/download_request_core.cc | 120 | ||||
-rw-r--r-- | content/browser/download/download_request_core.h | 89 | ||||
-rw-r--r-- | content/browser/download/download_request_handle.cc | 5 | ||||
-rw-r--r-- | content/browser/download/download_request_handle.h | 2 | ||||
-rw-r--r-- | content/browser/download/download_resource_handler.cc | 393 | ||||
-rw-r--r-- | content/browser/download/download_resource_handler.h | 39 | ||||
-rw-r--r-- | content/browser/download/url_downloader.cc | 145 | ||||
-rw-r--r-- | content/browser/download/url_downloader.h | 25 | ||||
-rw-r--r-- | content/test/data/download/gzip-content.gz | bin | 0 -> 33 bytes | |||
-rw-r--r-- | content/test/data/download/gzip-content.gz.mock-http-headers | 3 |
15 files changed, 380 insertions, 601 deletions
diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc index c763b87..77e438c 100644 --- a/content/browser/download/download_browsertest.cc +++ b/content/browser/download/download_browsertest.cc @@ -1521,7 +1521,7 @@ IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, CancelResumingDownload) { ASSERT_FALSE(intermediate_path.empty()); EXPECT_TRUE(base::PathExists(intermediate_path)); - // Resume and remove download. We expect only a single OnDownloadCreated() + // Resume and cancel download. We expect only a single OnDownloadCreated() // call, and that's for the second download created below. MockDownloadManagerObserver dm_observer( DownloadManagerForShell(initiator_shell_for_resumption())); @@ -1560,6 +1560,76 @@ IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, CancelResumingDownload) { EXPECT_TRUE(EnsureNoPendingDownloads()); } +IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RemoveResumedDownload) { + TestDownloadRequestHandler::Parameters parameters = + TestDownloadRequestHandler::Parameters::WithSingleInterruption(); + TestDownloadRequestHandler request_handler; + request_handler.StartServing(parameters); + + DownloadItem* download = StartDownloadAndReturnItem( + initiator_shell_for_resumption(), request_handler.url()); + WaitForInterrupt(download); + + base::FilePath intermediate_path(download->GetFullPath()); + base::FilePath target_path(download->GetTargetFilePath()); + ASSERT_FALSE(intermediate_path.empty()); + EXPECT_TRUE(base::PathExists(intermediate_path)); + EXPECT_FALSE(base::PathExists(target_path)); + + // Resume and remove download. We don't expect OnDownloadCreated() calls. + MockDownloadManagerObserver dm_observer( + DownloadManagerForShell(initiator_shell_for_resumption())); + EXPECT_CALL(dm_observer, OnDownloadCreated(_, _)).Times(0); + + PrepareToResume(); + download->Resume(); + WaitForInProgress(download); + + download->Remove(); + + // The intermediate file should now be gone. + RunAllPendingInMessageLoop(BrowserThread::FILE); + RunAllPendingInMessageLoop(); + EXPECT_FALSE(base::PathExists(intermediate_path)); + EXPECT_FALSE(base::PathExists(target_path)); + EXPECT_TRUE(EnsureNoPendingDownloads()); +} + +IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, CancelResumedDownload) { + TestDownloadRequestHandler::Parameters parameters = + TestDownloadRequestHandler::Parameters::WithSingleInterruption(); + TestDownloadRequestHandler request_handler; + request_handler.StartServing(parameters); + + DownloadItem* download = StartDownloadAndReturnItem( + initiator_shell_for_resumption(), request_handler.url()); + WaitForInterrupt(download); + + base::FilePath intermediate_path(download->GetFullPath()); + base::FilePath target_path(download->GetTargetFilePath()); + ASSERT_FALSE(intermediate_path.empty()); + EXPECT_TRUE(base::PathExists(intermediate_path)); + EXPECT_FALSE(base::PathExists(target_path)); + + // Resume and remove download. We don't expect OnDownloadCreated() calls. + MockDownloadManagerObserver dm_observer( + DownloadManagerForShell(initiator_shell_for_resumption())); + EXPECT_CALL(dm_observer, OnDownloadCreated(_, _)).Times(0); + + PrepareToResume(); + download->Resume(); + WaitForInProgress(download); + + download->Cancel(true); + + // The intermediate file should now be gone. + RunAllPendingInMessageLoop(BrowserThread::FILE); + RunAllPendingInMessageLoop(); + EXPECT_FALSE(base::PathExists(intermediate_path)); + EXPECT_FALSE(base::PathExists(target_path)); + EXPECT_TRUE(EnsureNoPendingDownloads()); +} + // Check that the cookie policy is correctly updated when downloading a file // that redirects cross origin. IN_PROC_BROWSER_TEST_F(DownloadContentTest, CookiePolicy) { @@ -1705,6 +1775,17 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadGZipWithNoContent) { // That's it. This should work without crashing. } +// Make sure that sniffed MIME types are correctly passed through to the +// download item. +IN_PROC_BROWSER_TEST_F(DownloadContentTest, SniffedMimeType) { + GURL url = net::URLRequestMockHTTPJob::GetMockUrl("gzip-content.gz"); + DownloadItem* item = StartDownloadAndReturnItem(shell(), url); + WaitForCompletion(item); + + EXPECT_STREQ("application/x-gzip", item->GetMimeType().c_str()); + EXPECT_TRUE(item->GetOriginalMimeType().empty()); +} + IN_PROC_BROWSER_TEST_F(DownloadContentTest, Spam) { ASSERT_TRUE(embedded_test_server()->Start()); diff --git a/content/browser/download/download_create_info.cc b/content/browser/download/download_create_info.cc index 05b0992..901d3d8 100644 --- a/content/browser/download/download_create_info.cc +++ b/content/browser/download/download_create_info.cc @@ -14,39 +14,32 @@ namespace content { DownloadCreateInfo::DownloadCreateInfo(const base::Time& start_time, int64 total_bytes, const net::BoundNetLog& bound_net_log, - bool has_user_gesture, - ui::PageTransition transition_type, scoped_ptr<DownloadSaveInfo> save_info) : start_time(start_time), total_bytes(total_bytes), download_id(DownloadItem::kInvalidId), - has_user_gesture(has_user_gesture), - transition_type(transition_type), + has_user_gesture(false), + transition_type(ui::PAGE_TRANSITION_LINK), save_info(save_info.Pass()), request_bound_net_log(bound_net_log) {} DownloadCreateInfo::DownloadCreateInfo() - : total_bytes(0), - download_id(DownloadItem::kInvalidId), - has_user_gesture(false), - transition_type(ui::PAGE_TRANSITION_LINK), - save_info(new DownloadSaveInfo()) { -} + : DownloadCreateInfo(base::Time(), + 0, + net::BoundNetLog(), + make_scoped_ptr(new DownloadSaveInfo)) {} -DownloadCreateInfo::~DownloadCreateInfo() { -} +DownloadCreateInfo::~DownloadCreateInfo() {} std::string DownloadCreateInfo::DebugString() const { - return base::StringPrintf("{" - " download_id = %u" - " url = \"%s\"" - " request_handle = %s" - " total_bytes = %" PRId64 - " }", - download_id, - url().spec().c_str(), - request_handle.DebugString().c_str(), - total_bytes); + return base::StringPrintf( + "{" + " download_id = %u" + " url = \"%s\"" + " request_handle = %s" + " total_bytes = %" PRId64 " }", + download_id, url().spec().c_str(), request_handle->DebugString().c_str(), + total_bytes); } const GURL& DownloadCreateInfo::url() const { diff --git a/content/browser/download/download_create_info.h b/content/browser/download/download_create_info.h index 7bf03ab..406f4a1 100644 --- a/content/browser/download/download_create_info.h +++ b/content/browser/download/download_create_info.h @@ -27,8 +27,6 @@ struct CONTENT_EXPORT DownloadCreateInfo { DownloadCreateInfo(const base::Time& start_time, int64 total_bytes, const net::BoundNetLog& bound_net_log, - bool has_user_gesture, - ui::PageTransition transition_type, scoped_ptr<DownloadSaveInfo> save_info); DownloadCreateInfo(); ~DownloadCreateInfo(); @@ -92,7 +90,7 @@ struct CONTENT_EXPORT DownloadCreateInfo { std::string remote_address; // The handle to the URLRequest sourcing this download. - DownloadRequestHandle request_handle; + scoped_ptr<DownloadRequestHandleInterface> request_handle; // The request's |BoundNetLog|, for "source_dependency" linking with the // download item's. diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc index 89c63df..75763de 100644 --- a/content/browser/download/download_manager_impl.cc +++ b/content/browser/download/download_manager_impl.cc @@ -125,19 +125,19 @@ scoped_ptr<UrlDownloader, BrowserThread::DeleteOnIOThread> BeginDownload( if (params->render_process_host_id() != -1) { ResourceDispatcherHost::Get()->BeginDownload( - request.Pass(), params->referrer(), params->content_initiated(), + std::move(request), params->referrer(), params->content_initiated(), params->resource_context(), params->render_process_host_id(), params->render_view_host_routing_id(), params->render_frame_host_routing_id(), params->prefer_cache(), - params->do_not_prompt_for_login(), save_info.Pass(), download_id, + params->do_not_prompt_for_login(), std::move(save_info), download_id, params->callback()); return nullptr; } return scoped_ptr<UrlDownloader, BrowserThread::DeleteOnIOThread>( - UrlDownloader::BeginDownload( - download_manager, request.Pass(), params->referrer(), false, - params->prefer_cache(), true, save_info.Pass(), download_id, - params->callback()) + UrlDownloader::BeginDownload(download_manager, std::move(request), + params->referrer(), params->prefer_cache(), + std::move(save_info), download_id, + params->callback()) .release()); } @@ -348,8 +348,8 @@ void DownloadManagerImpl::StartDownload( base::Callback<void(uint32)> got_id(base::Bind( &DownloadManagerImpl::StartDownloadWithId, weak_factory_.GetWeakPtr(), - base::Passed(info.Pass()), - base::Passed(stream.Pass()), + base::Passed(&info), + base::Passed(&stream), on_started, new_download)); if (new_download) { @@ -378,7 +378,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(); if (!on_started.is_null()) on_started.Run(NULL, DOWNLOAD_INTERRUPT_REASON_USER_CANCELED); return; @@ -397,13 +397,11 @@ void DownloadManagerImpl::StartDownloadWithId( } // Create the download file and start the download. - scoped_ptr<DownloadFile> download_file( - file_factory_->CreateFile( - info->save_info.Pass(), default_download_directory, - info->url(), info->referrer_url, - delegate_ && delegate_->GenerateFileHash(), - stream.Pass(), download->GetBoundNetLog(), - download->DestinationObserverAsWeakPtr())); + scoped_ptr<DownloadFile> download_file(file_factory_->CreateFile( + std::move(info->save_info), default_download_directory, info->url(), + info->referrer_url, delegate_ && delegate_->GenerateFileHash(), + std::move(stream), download->GetBoundNetLog(), + download->DestinationObserverAsWeakPtr())); // Attach the client ID identifying the app to the AV system. if (download_file.get() && delegate_) { @@ -411,9 +409,7 @@ void DownloadManagerImpl::StartDownloadWithId( delegate_->ApplicationClientIdForFileScanning()); } - scoped_ptr<DownloadRequestHandleInterface> req_handle( - new DownloadRequestHandle(info->request_handle)); - download->Start(download_file.Pass(), req_handle.Pass()); + download->Start(std::move(download_file), std::move(info->request_handle)); // For interrupted downloads, Start() will transition the state to // IN_PROGRESS and consumers will be notified via OnDownloadUpdated(). diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc index 36c074b..2878cda 100644 --- a/content/browser/download/download_manager_impl_unittest.cc +++ b/content/browser/download/download_manager_impl_unittest.cc @@ -519,7 +519,7 @@ class DownloadManagerTest : public testing::Test { // null. uint32 id = next_download_id_; ++next_download_id_; - info.request_handle = DownloadRequestHandle(); + info.request_handle.reset(new DownloadRequestHandle); download_manager_->CreateActiveItem(id, info); DCHECK(mock_download_item_factory_->GetItem(id)); MockDownloadItemImpl& item(*mock_download_item_factory_->GetItem(id)); diff --git a/content/browser/download/download_request_core.cc b/content/browser/download/download_request_core.cc index 0de82b1..577d9f4 100644 --- a/content/browser/download/download_request_core.cc +++ b/content/browser/download/download_request_core.cc @@ -7,6 +7,7 @@ #include <string> #include "base/bind.h" +#include "base/callback_helpers.h" #include "base/location.h" #include "base/logging.h" #include "base/metrics/histogram_macros.h" @@ -18,6 +19,7 @@ #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_interrupt_reasons_impl.h" #include "content/browser/download/download_manager_impl.h" +#include "content/browser/download/download_request_handle.h" #include "content/browser/download/download_stats.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/download_interrupt_reasons.h" @@ -34,65 +36,23 @@ namespace content { -namespace { - -void CallStartedCBOnUIThread( - const DownloadUrlParameters::OnStartedCallback& started_cb, - DownloadItem* item, - DownloadInterruptReason interrupt_reason) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - if (started_cb.is_null()) - return; - started_cb.Run(item, interrupt_reason); -} - -// Static function in order to prevent any accidental accesses to -// DownloadRequestCore members from the UI thread. -static void StartOnUIThread( - scoped_ptr<DownloadCreateInfo> info, - scoped_ptr<ByteStreamReader> stream, - base::WeakPtr<DownloadManager> download_manager, - const DownloadUrlParameters::OnStartedCallback& started_cb) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - if (!download_manager) { - // nullptr in unittests or if the page closed right after starting the - // download. - if (!started_cb.is_null()) - started_cb.Run(nullptr, DOWNLOAD_INTERRUPT_REASON_USER_CANCELED); - - // |stream| gets deleted on non-FILE thread, but it's ok since - // we're not using stream_writer_ yet. - - return; - } - - download_manager->StartDownload(info.Pass(), stream.Pass(), started_cb); -} - -} // namespace - const int DownloadRequestCore::kDownloadByteStreamSize = 100 * 1024; DownloadRequestCore::DownloadRequestCore( - uint32 id, net::URLRequest* request, - const DownloadUrlParameters::OnStartedCallback& started_cb, scoped_ptr<DownloadSaveInfo> save_info, - base::WeakPtr<DownloadManagerImpl> download_manager) - : request_(request), - download_id_(id), - started_cb_(started_cb), - save_info_(save_info.Pass()), + const base::Closure& on_ready_to_read_callback) + : on_ready_to_read_callback_(on_ready_to_read_callback), + request_(request), + save_info_(std::move(save_info)), last_buffer_size_(0), bytes_read_(0), pause_count_(0), - was_deferred_(false), - on_response_started_called_(false), - download_manager_(download_manager) { + was_deferred_(false) { + DCHECK(request_); + DCHECK(save_info_); + DCHECK(!on_ready_to_read_callback_.is_null()); RecordDownloadCount(UNTHROTTLED_COUNT); - power_save_blocker_ = PowerSaveBlocker::Create( PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension, PowerSaveBlocker::kReasonOther, "Download in progress"); @@ -100,12 +60,6 @@ DownloadRequestCore::DownloadRequestCore( DownloadRequestCore::~DownloadRequestCore() { DCHECK_CURRENTLY_ON(BrowserThread::IO); - - // This won't do anything if the callback was called before. - // If it goes through, it will likely be because OnWillStart() returned - // false somewhere in the chain of resource handlers. - CallStartedCB(nullptr, DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED); - // Remove output stream callback if a stream exists. if (stream_writer_) stream_writer_->RegisterCallback(base::Closure()); @@ -115,12 +69,11 @@ DownloadRequestCore::~DownloadRequestCore() { } // Send the download creation information to the download thread. -bool DownloadRequestCore::OnResponseStarted() { +void DownloadRequestCore::OnResponseStarted( + scoped_ptr<DownloadCreateInfo>* create_info, + scoped_ptr<ByteStreamReader>* stream_reader) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - // There can be only one (call) - DCHECK(!on_response_started_called_); - on_response_started_called_ = true; - + DCHECK(save_info_); DVLOG(20) << __FUNCTION__ << "()" << DebugString(); download_start_time_ = base::TimeTicks::Now(); @@ -139,20 +92,18 @@ bool DownloadRequestCore::OnResponseStarted() { : 0; // Deleted in DownloadManager. - scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo( - base::Time::Now(), content_length, request()->net_log(), false, - ui::PAGE_TRANSITION_LINK, save_info_.Pass())); + scoped_ptr<DownloadCreateInfo> info( + new DownloadCreateInfo(base::Time::Now(), content_length, + request()->net_log(), std::move(save_info_))); // Create the ByteStream for sending data to the download sink. - scoped_ptr<ByteStreamReader> stream_reader; CreateByteStream( base::ThreadTaskRunnerHandle::Get(), BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE), - kDownloadByteStreamSize, &stream_writer_, &stream_reader); + kDownloadByteStreamSize, &stream_writer_, stream_reader); stream_writer_->RegisterCallback( base::Bind(&DownloadRequestCore::ResumeRequest, AsWeakPtr())); - info->download_id = download_id_; info->url_chain = request()->url_chain(); info->referrer_url = GURL(request()->referrer()); string mime_type; @@ -164,7 +115,7 @@ bool DownloadRequestCore::OnResponseStarted() { // though as of this writing, the network stack ensures if there are, they // are all duplicates. request()->response_headers()->EnumerateHeader( - nullptr, "content-disposition", &info->content_disposition); + nullptr, "Content-Disposition", &info->content_disposition); } RecordDownloadMimeType(info->mime_type); RecordDownloadContentDisposition(info->content_disposition); @@ -202,30 +153,7 @@ bool DownloadRequestCore::OnResponseStarted() { info->url_chain.front().GetOrigin() != info->url_chain.back().GetOrigin()) info->save_info->suggested_name.clear(); - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&StartOnUIThread, base::Passed(&info), - base::Passed(&stream_reader), download_manager_, - // Pass to StartOnUIThread so that variable - // access is always on IO thread but function - // is called on UI thread. - started_cb_)); - // Guaranteed to be called in StartOnUIThread - started_cb_.Reset(); - - return true; -} - -void DownloadRequestCore::CallStartedCB( - DownloadItem* item, - DownloadInterruptReason interrupt_reason) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - if (started_cb_.is_null()) - return; - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&CallStartedCBOnUIThread, started_cb_, - item, interrupt_reason)); - started_cb_.Reset(); + info.swap(*create_info); } // Create a new buffer, which will be handed to the download thread for file @@ -284,7 +212,7 @@ bool DownloadRequestCore::OnReadCompleted(int bytes_read, bool* defer) { return true; } -void DownloadRequestCore::OnResponseCompleted( +DownloadInterruptReason DownloadRequestCore::OnResponseCompleted( const net::URLRequestStatus& status) { DCHECK_CURRENTLY_ON(BrowserThread::IO); int response_code = status.is_success() ? request()->GetResponseCode() : 0; @@ -382,8 +310,6 @@ void DownloadRequestCore::OnResponseCompleted( RecordNetworkBlockage(base::TimeTicks::Now() - download_start_time_, total_pause_time_); - CallStartedCB(nullptr, reason); - // Send the info down the stream. Conditional is in case we get // OnResponseCompleted without OnResponseStarted. if (stream_writer_) @@ -398,6 +324,8 @@ void DownloadRequestCore::OnResponseCompleted( stream_writer_.reset(); // We no longer need the stream. read_buffer_ = nullptr; + + return reason; } void DownloadRequestCore::PauseRequest() { @@ -423,7 +351,7 @@ void DownloadRequestCore::ResumeRequest() { last_stream_pause_time_ = base::TimeTicks(); } - downloader_->ResumeReading(); + on_ready_to_read_callback_.Run(); } std::string DownloadRequestCore::DebugString() const { diff --git a/content/browser/download/download_request_core.h b/content/browser/download/download_request_core.h index c01fa98..6982f04 100644 --- a/content/browser/download/download_request_core.h +++ b/content/browser/download/download_request_core.h @@ -25,10 +25,13 @@ class DownloadManagerImpl; class ByteStreamReader; class ByteStreamWriter; class PowerSaveBlocker; -class UrlDownloader; struct DownloadCreateInfo; -// Forwards data to the download thread. +// This class encapsulates the core logic for reading data from a URLRequest and +// writing it into a ByteStream. It's common to both DownloadResourceHandler and +// UrlDownloader. +// +// Created, lives on and dies on the IO thread. class CONTENT_EXPORT DownloadRequestCore : public base::SupportsWeakPtr<DownloadRequestCore> { public: @@ -36,52 +39,68 @@ class CONTENT_EXPORT DownloadRequestCore // downstream receiver of its output. static const int kDownloadByteStreamSize; - // started_cb will be called exactly once on the UI thread. - // |id| should be invalid if the id should be automatically assigned. - DownloadRequestCore( - uint32 id, - net::URLRequest* request, - const DownloadUrlParameters::OnStartedCallback& started_cb, - scoped_ptr<DownloadSaveInfo> save_info, - base::WeakPtr<DownloadManagerImpl> download_manager); + // |request| *must* outlive the DownloadRequestCore. |save_info| must be + // valid. + // + // Invokes |on_ready_to_read_callback| if a previous call to OnReadCompleted() + // resulted in |defer| being set to true, and DownloadRequestCore is now ready + // to commence reading. + DownloadRequestCore(net::URLRequest* request, + scoped_ptr<DownloadSaveInfo> save_info, + const base::Closure& on_ready_to_read_callback); ~DownloadRequestCore(); - // Send the download creation information to the download thread. - bool OnResponseStarted(); - - // Create a new buffer, which will be handed to the download thread for file - // writing and deletion. + // Should be called when the URLRequest::Delegate receives OnResponseStarted. + // Constructs a DownloadCreateInfo and a ByteStreamReader that should be + // passed into DownloadManagerImpl::StartDownload(). + // + // Only populates the response derived fields of DownloadCreateInfo, with the + // exception of |save_info|. + void OnResponseStarted(scoped_ptr<DownloadCreateInfo>* info, + scoped_ptr<ByteStreamReader>* stream_reader); + + // Starts a read cycle. Creates a new IOBuffer which can be passed into + // URLRequest::Read(). Call OnReadCompleted() when the Read operation + // completes. bool OnWillRead(scoped_refptr<net::IOBuffer>* buf, int* buf_size, int min_size); + // Should be called when the Read() operation completes. |defer| will be set + // to true if reading is to be suspended. In the latter case, once more data + // can be read, invokes the |on_ready_to_read_callback|. bool OnReadCompleted(int bytes_read, bool* defer); - void OnResponseCompleted(const net::URLRequestStatus& status); - + // Called to signal that the response is complete. If the return value is + // something other than DOWNLOAD_INTERRUPT_REASON_NONE, then the download + // should be considered interrupted. + // + // It is expected that once this method is invoked, the DownloadRequestCore + // object will be destroyed in short order without invoking any other methods + // other than the destructor. + DownloadInterruptReason OnResponseCompleted( + const net::URLRequestStatus& status); + + // Called if the request should suspend reading. A subsequent + // OnReadCompleted() will result in |defer| being set to true. + // + // Each PauseRequest() must be balanced with a call to ResumeRequest(). void PauseRequest(); + + // Balances a call to PauseRequest(). If no more pauses are outstanding and + // the reader end of the ByteStream is ready to receive more data, + // DownloadRequestCore will invoke the |on_ready_to_read_callback| to signal + // to the caller that the read cycles should commence. void ResumeRequest(); std::string DebugString() const; - void set_downloader(UrlDownloader* downloader) { downloader_ = downloader; } - protected: net::URLRequest* request() const { return request_; } private: - // Arrange for started_cb_ to be called on the UI thread with the - // below values, nulling out started_cb_. Should only be called - // on the IO thread. - void CallStartedCB(DownloadItem* item, - DownloadInterruptReason interrupt_reason); - + base::Closure on_ready_to_read_callback_; net::URLRequest* request_; - uint32 download_id_; - - // This is read only on the IO thread, but may only - // be called on the UI thread. - DownloadUrlParameters::OnStartedCallback started_cb_; scoped_ptr<DownloadSaveInfo> save_info_; // Data flow @@ -104,16 +123,8 @@ class CONTENT_EXPORT DownloadRequestCore int pause_count_; bool was_deferred_; - // For DCHECKing - bool on_response_started_called_; - - UrlDownloader* downloader_; - - // DownloadManager passed in by the owner of DownloadRequestCore. - base::WeakPtr<DownloadManagerImpl> download_manager_; - + // Each successful OnWillRead will yield a buffer of this size. static const int kReadBufSize = 32768; // bytes - static const int kThrottleTimeMs = 200; // milliseconds DISALLOW_COPY_AND_ASSIGN(DownloadRequestCore); }; diff --git a/content/browser/download/download_request_handle.cc b/content/browser/download/download_request_handle.cc index 75f9d54..67dda4f 100644 --- a/content/browser/download/download_request_handle.cc +++ b/content/browser/download/download_request_handle.cc @@ -16,8 +16,9 @@ namespace content { -DownloadRequestHandle::~DownloadRequestHandle() { -} +DownloadRequestHandleInterface::~DownloadRequestHandleInterface() {} + +DownloadRequestHandle::~DownloadRequestHandle() {} DownloadRequestHandle::DownloadRequestHandle() : child_id_(-1), diff --git a/content/browser/download/download_request_handle.h b/content/browser/download/download_request_handle.h index 3e23775..5d047c7 100644 --- a/content/browser/download/download_request_handle.h +++ b/content/browser/download/download_request_handle.h @@ -24,7 +24,7 @@ class WebContents; // DownloadRequestHandleInterface is defined for mocking purposes. class CONTENT_EXPORT DownloadRequestHandleInterface { public: - virtual ~DownloadRequestHandleInterface() {} + virtual ~DownloadRequestHandleInterface(); // These functions must be called on the UI thread. virtual WebContents* GetWebContents() const = 0; diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc index 5cc4ee8..c3f69e2 100644 --- a/content/browser/download/download_resource_handler.cc +++ b/content/browser/download/download_resource_handler.cc @@ -7,34 +7,22 @@ #include <string> #include "base/bind.h" +#include "base/callback_helpers.h" #include "base/location.h" #include "base/logging.h" -#include "base/metrics/histogram_macros.h" -#include "base/metrics/sparse_histogram.h" -#include "base/single_thread_task_runner.h" #include "base/strings/stringprintf.h" -#include "base/thread_task_runner_handle.h" #include "content/browser/byte_stream.h" #include "content/browser/download/download_create_info.h" #include "content/browser/download/download_interrupt_reasons_impl.h" #include "content/browser/download/download_manager_impl.h" #include "content/browser/download/download_request_handle.h" -#include "content/browser/download/download_stats.h" #include "content/browser/loader/resource_dispatcher_host_impl.h" #include "content/browser/loader/resource_request_info_impl.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/download_interrupt_reasons.h" -#include "content/public/browser/download_item.h" -#include "content/public/browser/download_manager_delegate.h" #include "content/public/browser/navigation_entry.h" -#include "content/public/browser/power_save_blocker.h" #include "content/public/browser/web_contents.h" #include "content/public/common/resource_response.h" -#include "net/base/io_buffer.h" -#include "net/base/net_errors.h" -#include "net/http/http_response_headers.h" -#include "net/http/http_status_code.h" -#include "net/url_request/url_request_context.h" namespace content { @@ -45,17 +33,6 @@ struct DownloadResourceHandler::DownloadTabInfo { namespace { -void CallStartedCBOnUIThread( - const DownloadUrlParameters::OnStartedCallback& started_cb, - DownloadItem* item, - DownloadInterruptReason interrupt_reason) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - if (started_cb.is_null()) - return; - started_cb.Run(item, interrupt_reason); -} - // Static function in order to prevent any accidental accesses to // DownloadResourceHandler members from the UI thread. static void StartOnUIThread( @@ -65,7 +42,8 @@ static void StartOnUIThread( const DownloadUrlParameters::OnStartedCallback& started_cb) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DownloadManager* download_manager = info->request_handle.GetDownloadManager(); + DownloadManager* download_manager = + info->request_handle->GetDownloadManager(); if (!download_manager) { // NULL in unittests or if the page closed right after starting the // download. @@ -81,7 +59,8 @@ static void StartOnUIThread( info->tab_url = tab_info->tab_url; info->tab_referrer_url = tab_info->tab_referrer_url; - download_manager->StartDownload(info.Pass(), stream.Pass(), started_cb); + download_manager->StartDownload(std::move(info), std::move(stream), + started_cb); } void InitializeDownloadTabInfoOnUIThread( @@ -104,8 +83,6 @@ void DeleteOnUIThread( } // namespace -const int DownloadResourceHandler::kDownloadByteStreamSize = 100 * 1024; - DownloadResourceHandler::DownloadResourceHandler( uint32 id, net::URLRequest* request, @@ -114,15 +91,11 @@ DownloadResourceHandler::DownloadResourceHandler( : ResourceHandler(request), download_id_(id), started_cb_(started_cb), - save_info_(save_info.Pass()), tab_info_(new DownloadTabInfo()), - last_buffer_size_(0), - bytes_read_(0), - pause_count_(0), - was_deferred_(false), - on_response_started_called_(false) { - RecordDownloadCount(UNTHROTTLED_COUNT); - + core_(request, + std::move(save_info), + base::Bind(&DownloadResourceHandler::OnCoreReadyToRead, + base::Unretained(this))) { // Do UI thread initialization for tab_info_ asap after // DownloadResourceHandler creation since the tab could be navigated // before StartOnUIThread gets called. This is safe because deletion @@ -137,9 +110,19 @@ DownloadResourceHandler::DownloadResourceHandler( request_info->GetRequestID(), request_info->frame_tree_node_id()), tab_info_.get())); - power_save_blocker_ = PowerSaveBlocker::Create( - PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension, - PowerSaveBlocker::kReasonOther, "Download in progress"); +} + +DownloadResourceHandler::~DownloadResourceHandler() { + // This won't do anything if the callback was called before. + // If it goes through, it will likely be because OnWillStart() returned + // false somewhere in the chain of resource handlers. + CallStartedCB(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED); + + if (tab_info_) { + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&DeleteOnUIThread, base::Passed(&tab_info_))); + } } bool DownloadResourceHandler::OnRequestRedirected( @@ -153,127 +136,39 @@ bool DownloadResourceHandler::OnRequestRedirected( bool DownloadResourceHandler::OnResponseStarted( ResourceResponse* response, bool* defer) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - // There can be only one (call) - DCHECK(!on_response_started_called_); - on_response_started_called_ = true; - - DVLOG(20) << __FUNCTION__ << "()" << DebugString(); - download_start_time_ = base::TimeTicks::Now(); - - // If it's a download, we don't want to poison the cache with it. - request()->StopCaching(); - - // Lower priority as well, so downloads don't contend for resources - // with main frames. - request()->SetPriority(net::IDLE); + scoped_ptr<DownloadCreateInfo> create_info; + scoped_ptr<ByteStreamReader> stream_reader; - // If the content-length header is not present (or contains something other - // than numbers), the incoming content_length is -1 (unknown size). - // Set the content length to 0 to indicate unknown size to DownloadManager. - int64 content_length = - response->head.content_length > 0 ? response->head.content_length : 0; + core_.OnResponseStarted(&create_info, &stream_reader); const ResourceRequestInfoImpl* request_info = GetRequestInfo(); - - // Deleted in DownloadManager. - scoped_ptr<DownloadCreateInfo> info( - new DownloadCreateInfo(base::Time::Now(), - content_length, - request()->net_log(), - request_info->HasUserGesture(), - request_info->GetPageTransition(), - save_info_.Pass())); - - // Create the ByteStream for sending data to the download sink. - scoped_ptr<ByteStreamReader> stream_reader; - CreateByteStream( - base::ThreadTaskRunnerHandle::Get(), - BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE), - kDownloadByteStreamSize, &stream_writer_, &stream_reader); - stream_writer_->RegisterCallback( - base::Bind(&DownloadResourceHandler::ResumeRequest, AsWeakPtr())); - - info->download_id = download_id_; - info->url_chain = request()->url_chain(); - info->referrer_url = GURL(request()->referrer()); - info->mime_type = response->head.mime_type; - info->remote_address = request()->GetSocketAddress().host(); - if (request()->response_headers()) { - // Grab the first content-disposition header. There may be more than one, - // though as of this writing, the network stack ensures if there are, they - // are all duplicates. - request()->response_headers()->EnumerateHeader( - nullptr, "content-disposition", &info->content_disposition); - } - RecordDownloadMimeType(info->mime_type); - RecordDownloadContentDisposition(info->content_disposition); - - info->request_handle = DownloadRequestHandle( + create_info->download_id = download_id_; + create_info->has_user_gesture = request_info->HasUserGesture(); + create_info->transition_type = request_info->GetPageTransition(); + create_info->request_handle.reset(new DownloadRequestHandle( AsWeakPtr(), request_info->GetChildID(), request_info->GetRouteID(), - request_info->GetRequestID(), request_info->frame_tree_node_id()); - - // Get the last modified time and etag. - const net::HttpResponseHeaders* headers = request()->response_headers(); - if (headers) { - if (headers->HasStrongValidators()) { - // If we don't have strong validators as per RFC 2616 section 13.3.3, then - // we neither store nor use them for range requests. - if (!headers->EnumerateHeader(NULL, "Last-Modified", - &info->last_modified)) - info->last_modified.clear(); - if (!headers->EnumerateHeader(NULL, "ETag", &info->etag)) - info->etag.clear(); - } + request_info->GetRequestID(), request_info->frame_tree_node_id())); - int status = headers->response_code(); - if (2 == status / 100 && status != net::HTTP_PARTIAL_CONTENT) { - // Success & not range response; if we asked for a range, we didn't - // get it--reset the file pointers to reflect that. - info->save_info->offset = 0; - info->save_info->hash_state = ""; - } - - if (!headers->GetMimeType(&info->original_mime_type)) - info->original_mime_type.clear(); - } - - // Blink verifies that the requester of this download is allowed to set a - // suggested name for the security origin of the downlaod URL. However, this - // assumption doesn't hold if there were cross origin redirects. Therefore, - // clear the suggested_name for such requests. - if (info->url_chain.size() > 1 && - info->url_chain.front().GetOrigin() != info->url_chain.back().GetOrigin()) - info->save_info->suggested_name.clear(); + // The MIME type in ResourceResponse is the product of + // MimeTypeResourceHandler. + create_info->mime_type = response->head.mime_type; BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, - base::Bind(&StartOnUIThread, - base::Passed(&info), - base::Passed(&tab_info_), - base::Passed(&stream_reader), - // Pass to StartOnUIThread so that variable - // access is always on IO thread but function - // is called on UI thread. - started_cb_)); - // Guaranteed to be called in StartOnUIThread - started_cb_.Reset(); - + base::Bind(&StartOnUIThread, base::Passed(&create_info), + base::Passed(&tab_info_), base::Passed(&stream_reader), + base::ResetAndReturn(&started_cb_))); return true; } void DownloadResourceHandler::CallStartedCB( - DownloadItem* item, DownloadInterruptReason interrupt_reason) { DCHECK_CURRENTLY_ON(BrowserThread::IO); if (started_cb_.is_null()) return; - BrowserThread::PostTask( - BrowserThread::UI, - FROM_HERE, - base::Bind( - &CallStartedCBOnUIThread, started_cb_, item, interrupt_reason)); - started_cb_.Reset(); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(base::ResetAndReturn(&started_cb_), + nullptr, interrupt_reason)); } bool DownloadResourceHandler::OnWillStart(const GURL& url, bool* defer) { @@ -290,175 +185,20 @@ bool DownloadResourceHandler::OnBeforeNetworkStart(const GURL& url, bool DownloadResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf, int* buf_size, int min_size) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - DCHECK(buf && buf_size); - DCHECK(!read_buffer_.get()); - - *buf_size = min_size < 0 ? kReadBufSize : min_size; - last_buffer_size_ = *buf_size; - read_buffer_ = new net::IOBuffer(*buf_size); - *buf = read_buffer_.get(); - return true; + return core_.OnWillRead(buf, buf_size, min_size); } // Pass the buffer to the download file writer. bool DownloadResourceHandler::OnReadCompleted(int bytes_read, bool* defer) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - DCHECK(read_buffer_.get()); - - base::TimeTicks now(base::TimeTicks::Now()); - if (!last_read_time_.is_null()) { - double seconds_since_last_read = (now - last_read_time_).InSecondsF(); - if (now == last_read_time_) - // Use 1/10 ms as a "very small number" so that we avoid - // divide-by-zero error and still record a very high potential bandwidth. - seconds_since_last_read = 0.00001; - - double actual_bandwidth = (bytes_read)/seconds_since_last_read; - double potential_bandwidth = last_buffer_size_/seconds_since_last_read; - RecordBandwidth(actual_bandwidth, potential_bandwidth); - } - last_read_time_ = now; - - if (!bytes_read) - return true; - bytes_read_ += bytes_read; - DCHECK(read_buffer_.get()); - - // Take the data ship it down the stream. If the stream is full, pause the - // request; the stream callback will resume it. - if (!stream_writer_->Write(read_buffer_, bytes_read)) { - PauseRequest(); - *defer = was_deferred_ = true; - last_stream_pause_time_ = now; - } - - read_buffer_ = NULL; // Drop our reference. - - if (pause_count_ > 0) - *defer = was_deferred_ = true; - - return true; + return core_.OnReadCompleted(bytes_read, defer); } void DownloadResourceHandler::OnResponseCompleted( const net::URLRequestStatus& status, const std::string& security_info, bool* defer) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - int response_code = status.is_success() ? request()->GetResponseCode() : 0; - DVLOG(20) << __FUNCTION__ << "()" << DebugString() - << " status.status() = " << status.status() - << " status.error() = " << status.error() - << " response_code = " << response_code; - - net::Error error_code = net::OK; - if (status.status() == net::URLRequestStatus::FAILED || - // Note cancels as failures too. - status.status() == net::URLRequestStatus::CANCELED) { - error_code = static_cast<net::Error>(status.error()); // Normal case. - // Make sure that at least the fact of failure comes through. - if (error_code == net::OK) - error_code = net::ERR_FAILED; - } - - // ERR_CONTENT_LENGTH_MISMATCH and ERR_INCOMPLETE_CHUNKED_ENCODING are - // allowed since a number of servers in the wild close the connection too - // early by mistake. Other browsers - IE9, Firefox 11.0, and Safari 5.1.4 - - // treat downloads as complete in both cases, so we follow their lead. - if (error_code == net::ERR_CONTENT_LENGTH_MISMATCH || - error_code == net::ERR_INCOMPLETE_CHUNKED_ENCODING) { - error_code = net::OK; - } - DownloadInterruptReason reason = - ConvertNetErrorToInterruptReason( - error_code, DOWNLOAD_INTERRUPT_FROM_NETWORK); - - if (status.status() == net::URLRequestStatus::CANCELED && - status.error() == net::ERR_ABORTED) { - // CANCELED + ERR_ABORTED == something outside of the network - // stack cancelled the request. There aren't that many things that - // could do this to a download request (whose lifetime is separated from - // the tab from which it came). We map this to USER_CANCELLED as the - // case we know about (system suspend because of laptop close) corresponds - // to a user action. - // TODO(ahendrickson) -- Find a better set of codes to use here, as - // CANCELED/ERR_ABORTED can occur for reasons other than user cancel. - if (net::IsCertStatusError(request()->ssl_info().cert_status)) - reason = DOWNLOAD_INTERRUPT_REASON_SERVER_CERT_PROBLEM; - else - reason = DOWNLOAD_INTERRUPT_REASON_USER_CANCELED; - } - - if (status.is_success() && - reason == DOWNLOAD_INTERRUPT_REASON_NONE && - request()->response_headers()) { - // Handle server's response codes. - switch(response_code) { - case -1: // Non-HTTP request. - case net::HTTP_OK: - case net::HTTP_CREATED: - case net::HTTP_ACCEPTED: - case net::HTTP_NON_AUTHORITATIVE_INFORMATION: - case net::HTTP_RESET_CONTENT: - case net::HTTP_PARTIAL_CONTENT: - // Expected successful codes. - break; - case net::HTTP_NO_CONTENT: - case net::HTTP_NOT_FOUND: - reason = DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT; - break; - case net::HTTP_REQUESTED_RANGE_NOT_SATISFIABLE: - // Retry by downloading from the start automatically: - // If we haven't received data when we get this error, we won't. - reason = DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE; - break; - case net::HTTP_UNAUTHORIZED: - // Server didn't authorize this request. - reason = DOWNLOAD_INTERRUPT_REASON_SERVER_UNAUTHORIZED; - break; - case net::HTTP_FORBIDDEN: - // Server forbids access to this resource. - reason = DOWNLOAD_INTERRUPT_REASON_SERVER_FORBIDDEN; - break; - default: // All other errors. - // Redirection and informational codes should have been handled earlier - // in the stack. - DCHECK_NE(3, response_code / 100); - DCHECK_NE(1, response_code / 100); - reason = DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED; - break; - } - } - - std::string accept_ranges; - bool has_strong_validators = false; - if (request()->response_headers()) { - request()->response_headers()->EnumerateHeader( - NULL, "Accept-Ranges", &accept_ranges); - has_strong_validators = - request()->response_headers()->HasStrongValidators(); - } - RecordAcceptsRanges(accept_ranges, bytes_read_, has_strong_validators); - RecordNetworkBlockage(base::TimeTicks::Now() - download_start_time_, - total_pause_time_); - - CallStartedCB(NULL, reason); - - // Send the info down the stream. Conditional is in case we get - // OnResponseCompleted without OnResponseStarted. - if (stream_writer_) - stream_writer_->Close(reason); - - // If the error mapped to something unknown, record it so that - // we can drill down. - if (reason == DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED) { - UMA_HISTOGRAM_SPARSE_SLOWLY("Download.MapErrorNetworkFailed", - std::abs(status.error())); - } - - stream_writer_.reset(); // We no longer need the stream. - read_buffer_ = NULL; + DownloadInterruptReason result = core_.OnResponseCompleted(status); + CallStartedCB(result); } void DownloadResourceHandler::OnDataDownloaded(int bytes_downloaded) { @@ -466,28 +206,15 @@ void DownloadResourceHandler::OnDataDownloaded(int bytes_downloaded) { } void DownloadResourceHandler::PauseRequest() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - - ++pause_count_; + core_.PauseRequest(); } void DownloadResourceHandler::ResumeRequest() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - DCHECK_LT(0, pause_count_); - - --pause_count_; - - if (!was_deferred_) - return; - if (pause_count_ > 0) - return; - - was_deferred_ = false; - if (!last_stream_pause_time_.is_null()) { - total_pause_time_ += (base::TimeTicks::Now() - last_stream_pause_time_); - last_stream_pause_time_ = base::TimeTicks(); - } + core_.ResumeRequest(); +} +void DownloadResourceHandler::OnCoreReadyToRead() { + DCHECK_CURRENTLY_ON(BrowserThread::IO); controller()->Resume(); } @@ -519,28 +246,4 @@ std::string DownloadResourceHandler::DebugString() const { info->GetRouteID()); } -DownloadResourceHandler::~DownloadResourceHandler() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - - // This won't do anything if the callback was called before. - // If it goes through, it will likely be because OnWillStart() returned - // false somewhere in the chain of resource handlers. - CallStartedCB(NULL, DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED); - - // Remove output stream callback if a stream exists. - if (stream_writer_) - stream_writer_->RegisterCallback(base::Closure()); - - // tab_info_ must be destroyed on UI thread, since - // InitializeDownloadTabInfoOnUIThread might still be using it. - if (tab_info_.get()) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&DeleteOnUIThread, base::Passed(&tab_info_))); - } - - UMA_HISTOGRAM_TIMES("SB2.DownloadDuration", - base::TimeTicks::Now() - download_start_time_); -} - } // namespace content diff --git a/content/browser/download/download_resource_handler.h b/content/browser/download/download_resource_handler.h index 9c8f53d..a628926 100644 --- a/content/browser/download/download_resource_handler.h +++ b/content/browser/download/download_resource_handler.h @@ -9,6 +9,7 @@ #include "base/callback.h" #include "base/memory/scoped_ptr.h" +#include "content/browser/download/download_request_core.h" #include "content/browser/loader/resource_handler.h" #include "content/public/browser/download_interrupt_reasons.h" #include "content/public/browser/download_manager.h" @@ -33,10 +34,6 @@ class CONTENT_EXPORT DownloadResourceHandler public: struct DownloadTabInfo; - // Size of the buffer used between the DownloadResourceHandler and the - // downstream receiver of its output. - static const int kDownloadByteStreamSize; - // started_cb will be called exactly once on the UI thread. // |id| should be invalid if the id should be automatically assigned. DownloadResourceHandler( @@ -87,14 +84,15 @@ class CONTENT_EXPORT DownloadResourceHandler // Arrange for started_cb_ to be called on the UI thread with the // below values, nulling out started_cb_. Should only be called // on the IO thread. - void CallStartedCB(DownloadItem* item, - DownloadInterruptReason interrupt_reason); + void CallStartedCB(DownloadInterruptReason interrupt_reason); + + void OnCoreReadyToRead(); uint32 download_id_; + // This is read only on the IO thread, but may only // be called on the UI thread. DownloadUrlParameters::OnStartedCallback started_cb_; - scoped_ptr<DownloadSaveInfo> save_info_; // Stores information about the download that must be acquired on the UI // thread before StartOnUIThread is called. @@ -104,32 +102,7 @@ class CONTENT_EXPORT DownloadResourceHandler // deletion must occur on the IO thread. scoped_ptr<DownloadTabInfo> tab_info_; - // Data flow - scoped_refptr<net::IOBuffer> read_buffer_; // From URLRequest. - scoped_ptr<ByteStreamWriter> stream_writer_; // To rest of system. - - // Keeps the system from sleeping while this ResourceHandler is alive. If the - // system enters power saving mode while a request is alive, it can cause the - // request to fail and the associated download will be interrupted. - scoped_ptr<PowerSaveBlocker> power_save_blocker_; - - // The following are used to collect stats. - base::TimeTicks download_start_time_; - base::TimeTicks last_read_time_; - base::TimeTicks last_stream_pause_time_; - base::TimeDelta total_pause_time_; - size_t last_buffer_size_; - int64 bytes_read_; - - int pause_count_; - bool was_deferred_; - - // For DCHECKing - bool on_response_started_called_; - - static const int kReadBufSize = 32768; // bytes - static const int kThrottleTimeMs = 200; // milliseconds - + DownloadRequestCore core_; DISALLOW_COPY_AND_ASSIGN(DownloadResourceHandler); }; diff --git a/content/browser/download/url_downloader.cc b/content/browser/download/url_downloader.cc index ffaea71..1b05228 100644 --- a/content/browser/download/url_downloader.cc +++ b/content/browser/download/url_downloader.cc @@ -4,18 +4,17 @@ #include "content/browser/download/url_downloader.h" +#include "base/callback_helpers.h" #include "base/location.h" -#include "base/thread_task_runner_handle.h" -#include "content/browser/appcache/appcache_interceptor.h" +#include "base/macros.h" +#include "base/threading/sequenced_task_runner_handle.h" +#include "content/browser/byte_stream.h" +#include "content/browser/download/download_create_info.h" #include "content/browser/download/download_manager_impl.h" -#include "content/browser/service_worker/service_worker_request_handler.h" -#include "content/browser/ssl/ssl_policy.h" -#include "content/common/ssl_status_serialization.h" -#include "content/public/browser/cert_store.h" +#include "content/browser/download/download_request_handle.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/download_interrupt_reasons.h" #include "content/public/browser/download_save_info.h" -#include "content/public/browser/signed_certificate_timestamp_store.h" -#include "content/public/common/process_type.h" -#include "content/public/common/security_style.h" #include "net/base/io_buffer.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" @@ -25,14 +24,58 @@ namespace content { +class UrlDownloader::RequestHandle : public DownloadRequestHandleInterface { + public: + RequestHandle(base::WeakPtr<UrlDownloader> downloader, + base::WeakPtr<DownloadManagerImpl> download_manager_impl, + scoped_refptr<base::SequencedTaskRunner> downloader_task_runner) + : downloader_(downloader), + download_manager_impl_(download_manager_impl), + downloader_task_runner_(downloader_task_runner) {} + RequestHandle(RequestHandle&& other) + : downloader_(std::move(other.downloader_)), + download_manager_impl_(std::move(other.download_manager_impl_)), + downloader_task_runner_(std::move(other.downloader_task_runner_)) {} + RequestHandle& operator=(RequestHandle&& other) { + downloader_ = std::move(other.downloader_); + download_manager_impl_ = std::move(other.download_manager_impl_); + downloader_task_runner_ = std::move(other.downloader_task_runner_); + return *this; + } + + // DownloadRequestHandleInterface + WebContents* GetWebContents() const override { return nullptr; } + DownloadManager* GetDownloadManager() const override { + return download_manager_impl_ ? download_manager_impl_.get() : nullptr; + } + void PauseRequest() const override { + downloader_task_runner_->PostTask( + FROM_HERE, base::Bind(&UrlDownloader::PauseRequest, downloader_)); + } + void ResumeRequest() const override { + downloader_task_runner_->PostTask( + FROM_HERE, base::Bind(&UrlDownloader::ResumeRequest, downloader_)); + } + void CancelRequest() const override { + downloader_task_runner_->PostTask( + FROM_HERE, base::Bind(&UrlDownloader::CancelRequest, downloader_)); + } + std::string DebugString() const override { return std::string(); } + + private: + base::WeakPtr<UrlDownloader> downloader_; + base::WeakPtr<DownloadManagerImpl> download_manager_impl_; + scoped_refptr<base::SequencedTaskRunner> downloader_task_runner_; + + DISALLOW_COPY_AND_ASSIGN(RequestHandle); +}; + // static scoped_ptr<UrlDownloader> UrlDownloader::BeginDownload( base::WeakPtr<DownloadManagerImpl> download_manager, scoped_ptr<net::URLRequest> request, const Referrer& referrer, - bool is_content_initiated, bool prefer_cache, - bool do_not_prompt_for_login, scoped_ptr<DownloadSaveInfo> save_info, uint32 download_id, const DownloadUrlParameters::OnStartedCallback& started_callback) { @@ -59,32 +102,34 @@ scoped_ptr<UrlDownloader> UrlDownloader::BeginDownload( if (request->url().SchemeIs(url::kBlobScheme)) return nullptr; - scoped_ptr<DownloadRequestCore> handler( - new DownloadRequestCore(download_id, request.get(), started_callback, - save_info.Pass(), download_manager)); - // From this point forward, the |UrlDownloader| is responsible for // |started_callback|. scoped_ptr<UrlDownloader> downloader( - new UrlDownloader(request.Pass(), handler.Pass(), download_manager)); - + new UrlDownloader(std::move(request), download_manager, + std::move(save_info), download_id, started_callback)); downloader->Start(); return downloader; } -UrlDownloader::UrlDownloader(scoped_ptr<net::URLRequest> request, - scoped_ptr<DownloadRequestCore> handler, - base::WeakPtr<DownloadManagerImpl> manager) - : request_(request.Pass()), - handler_(handler.Pass()), +UrlDownloader::UrlDownloader( + scoped_ptr<net::URLRequest> request, + base::WeakPtr<DownloadManagerImpl> manager, + scoped_ptr<DownloadSaveInfo> save_info, + uint32 download_id, + const DownloadUrlParameters::OnStartedCallback& on_started_callback) + : request_(std::move(request)), manager_(manager), - weak_ptr_factory_(this) { - handler_->set_downloader(this); -} + download_id_(download_id), + on_started_callback_(on_started_callback), + handler_( + request_.get(), + std::move(save_info), + base::Bind(&UrlDownloader::ResumeReading, base::Unretained(this))), + weak_ptr_factory_(this) {} UrlDownloader::~UrlDownloader() { - handler_.reset(); + CallStartedCallbackOnFailure(DOWNLOAD_INTERRUPT_REASON_USER_CANCELED); } void UrlDownloader::Start() { @@ -112,7 +157,20 @@ void UrlDownloader::OnResponseStarted(net::URLRequest* request) { return; } - handler_->OnResponseStarted(); + scoped_ptr<DownloadCreateInfo> create_info; + scoped_ptr<ByteStreamReader> stream_reader; + + handler_.OnResponseStarted(&create_info, &stream_reader); + + create_info->download_id = download_id_; + create_info->request_handle.reset( + new RequestHandle(weak_ptr_factory_.GetWeakPtr(), manager_, + base::SequencedTaskRunnerHandle::Get())); + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&DownloadManagerImpl::StartDownload, manager_, + base::Passed(&create_info), base::Passed(&stream_reader), + base::ResetAndReturn(&on_started_callback_))); if (request_->status().is_success()) StartReading(false); // Read the first chunk. @@ -128,9 +186,9 @@ void UrlDownloader::StartReading(bool is_continuation) { // doesn't use the buffer. scoped_refptr<net::IOBuffer> buf; int buf_size; - if (!handler_->OnWillRead(&buf, &buf_size, -1)) { + if (!handler_.OnWillRead(&buf, &buf_size, -1)) { request_->CancelWithError(net::ERR_ABORTED); - base::ThreadTaskRunnerHandle::Get()->PostTask( + base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&UrlDownloader::ResponseCompleted, weak_ptr_factory_.GetWeakPtr())); return; @@ -150,7 +208,7 @@ void UrlDownloader::StartReading(bool is_continuation) { } else { // Else, trigger OnReadCompleted asynchronously to avoid starving the IO // thread in case the URLRequest can provide data synchronously. - base::ThreadTaskRunnerHandle::Get()->PostTask( + base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&UrlDownloader::OnReadCompleted, weak_ptr_factory_.GetWeakPtr(), request_.get(), bytes_read)); @@ -171,7 +229,7 @@ void UrlDownloader::OnReadCompleted(net::URLRequest* request, int bytes_read) { DCHECK(request_->status().is_success()); bool defer = false; - if (!handler_->OnReadCompleted(bytes_read, &defer)) { + if (!handler_.OnReadCompleted(bytes_read, &defer)) { request_->CancelWithError(net::ERR_ABORTED); return; } else if (defer) { @@ -193,7 +251,7 @@ void UrlDownloader::OnReadCompleted(net::URLRequest* request, int bytes_read) { void UrlDownloader::ResponseCompleted() { DVLOG(1) << "ResponseCompleted: " << request_->url().spec(); - handler_->OnResponseCompleted(request_->status()); + handler_.OnResponseCompleted(request_->status()); BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, base::Bind(&DownloadManagerImpl::RemoveUrlDownloader, manager_, this)); @@ -207,4 +265,27 @@ void UrlDownloader::ResumeReading() { } } +void UrlDownloader::CallStartedCallbackOnFailure( + DownloadInterruptReason result) { + if (on_started_callback_.is_null()) + return; + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(base::ResetAndReturn(&on_started_callback_), nullptr, result)); +} + +void UrlDownloader::PauseRequest() { + handler_.PauseRequest(); +} + +void UrlDownloader::ResumeRequest() { + handler_.ResumeRequest(); +} + +void UrlDownloader::CancelRequest() { + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&DownloadManagerImpl::RemoveUrlDownloader, manager_, this)); +} + } // namespace content diff --git a/content/browser/download/url_downloader.h b/content/browser/download/url_downloader.h index c985092..d9bc773 100644 --- a/content/browser/download/url_downloader.h +++ b/content/browser/download/url_downloader.h @@ -16,22 +16,22 @@ namespace content { class DownloadManagerImpl; -class DownloadRequestCore; class UrlDownloader : public net::URLRequest::Delegate { public: - UrlDownloader(scoped_ptr<net::URLRequest> request, - scoped_ptr<DownloadRequestCore> handler, - base::WeakPtr<DownloadManagerImpl> manager); + UrlDownloader( + scoped_ptr<net::URLRequest> request, + base::WeakPtr<DownloadManagerImpl> manager, + scoped_ptr<DownloadSaveInfo> save_info, + uint32 download_id, + const DownloadUrlParameters::OnStartedCallback& on_started_callback); ~UrlDownloader() override; static scoped_ptr<UrlDownloader> BeginDownload( base::WeakPtr<DownloadManagerImpl> download_manager, scoped_ptr<net::URLRequest> request, const Referrer& referrer, - bool is_content_initiated, bool prefer_cache, - bool do_not_prompt_for_login, scoped_ptr<DownloadSaveInfo> save_info, uint32 download_id, const DownloadUrlParameters::OnStartedCallback& started_callback); @@ -49,10 +49,21 @@ class UrlDownloader : public net::URLRequest::Delegate { void Start(); void ResumeReading(); + void CallStartedCallbackOnFailure(DownloadInterruptReason result); + private: + class RequestHandle; + + void PauseRequest(); + void ResumeRequest(); + void CancelRequest(); + scoped_ptr<net::URLRequest> request_; - scoped_ptr<DownloadRequestCore> handler_; base::WeakPtr<DownloadManagerImpl> manager_; + uint32 download_id_; + DownloadUrlParameters::OnStartedCallback on_started_callback_; + + DownloadRequestCore handler_; base::WeakPtrFactory<UrlDownloader> weak_ptr_factory_; }; diff --git a/content/test/data/download/gzip-content.gz b/content/test/data/download/gzip-content.gz Binary files differnew file mode 100644 index 0000000..eb478f7 --- /dev/null +++ b/content/test/data/download/gzip-content.gz diff --git a/content/test/data/download/gzip-content.gz.mock-http-headers b/content/test/data/download/gzip-content.gz.mock-http-headers new file mode 100644 index 0000000..8f48fd4 --- /dev/null +++ b/content/test/data/download/gzip-content.gz.mock-http-headers @@ -0,0 +1,3 @@ +HTTP/1.1 200 OK +Content-Length: 33 + |