Index: content/browser/download/download_resource_handler.cc |
diff --git a/content/browser/download/download_resource_handler.cc b/content/browser/download/download_resource_handler.cc |
index bdcd1d7bfcaaea14eef61c04b2ff42051eb62c78..3caee71c6ccd45b2504bb5480820193c8d0bf519 100644 |
--- a/content/browser/download/download_resource_handler.cc |
+++ b/content/browser/download/download_resource_handler.cc |
@@ -8,15 +8,16 @@ |
#include "base/bind.h" |
#include "base/logging.h" |
+#include "base/message_loop_proxy.h" |
#include "base/metrics/histogram.h" |
#include "base/metrics/stats_counters.h" |
#include "base/stringprintf.h" |
-#include "content/browser/download/download_buffer.h" |
#include "content/browser/download/download_create_info.h" |
#include "content/browser/download/download_file_manager.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/byte_stream.h" |
#include "content/browser/download/download_stats.h" |
#include "content/browser/renderer_host/resource_dispatcher_host_impl.h" |
#include "content/browser/renderer_host/resource_request_info_impl.h" |
@@ -68,10 +69,10 @@ DownloadResourceHandler::DownloadResourceHandler( |
request_(request), |
started_cb_(started_cb), |
save_info_(save_info), |
- buffer_(new content::DownloadBuffer), |
is_paused_(false), |
last_buffer_size_(0), |
- bytes_read_(0) { |
+ bytes_read_(0), |
+ ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) { |
download_stats::RecordDownloadCount(download_stats::UNTHROTTLED_COUNT); |
} |
@@ -115,6 +116,14 @@ bool DownloadResourceHandler::OnResponseStarted( |
const ResourceRequestInfoImpl* request_info = |
ResourceRequestInfoImpl::ForRequest(request_); |
+ // Create the ByteStream pipe for sending data to the download sink. |
+ output_pipe_ = new content::ByteStream(); |
+ output_pipe_->RegisterSourceCallback( |
+ base::MessageLoopProxy::current(), |
+ base::Bind(&DownloadResourceHandler::ThrottleRequest, |
+ weak_factory_.GetWeakPtr()), |
+ 33); |
+ |
// Deleted in DownloadManager. |
scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo( |
base::Time::Now(), 0, content_length_, DownloadItem::IN_PROGRESS, |
@@ -163,20 +172,13 @@ bool DownloadResourceHandler::OnResponseStarted( |
save_info_.prompt_for_save_location && save_info_.file_path.empty(); |
info->referrer_charset = request_->context()->referrer_charset(); |
info->save_info = save_info_; |
- |
+ info->pipe = output_pipe_; |
BrowserThread::PostTask( |
BrowserThread::UI, FROM_HERE, |
base::Bind(&DownloadResourceHandler::StartOnUIThread, this, |
base::Passed(&info), request_handle)); |
- // We can't start saving the data before we create the file on disk and have a |
- // download id. The request will be un-paused in |
- // DownloadFileManager::CreateDownloadFile. |
- ResourceDispatcherHostImpl::Get()->PauseRequest(global_id_.child_id, |
- global_id_.request_id, |
- true); |
- |
return true; |
} |
@@ -229,24 +231,22 @@ bool DownloadResourceHandler::OnReadCompleted(int request_id, int* bytes_read) { |
return true; |
bytes_read_ += *bytes_read; |
DCHECK(read_buffer_); |
- // Swap the data. |
- net::IOBuffer* io_buffer = NULL; |
- read_buffer_.swap(&io_buffer); |
- size_t vector_size = buffer_->AddData(io_buffer, *bytes_read); |
- bool need_update = (vector_size == 1); // Buffer was empty. |
- |
- // We are passing ownership of this buffer to the download file manager. |
- if (need_update) { |
+ |
+ // Take the data from the DataLoanIOBuffer and ship it down the pipe. |
+ // If the pipe is full, pause the request; the pipe callback will resume it. |
+ if (!output_pipe_->AddData(read_buffer_, *bytes_read)) { |
+ // Send a message to ourselves to pause the request. |
+ // Posting to get around fragile/broken rentrancy semantics in |
+ // ResourceDispatcherHost. |
+ // TODO(rdsmith): Remove PostTask when RDH is fixed. |
BrowserThread::PostTask( |
- BrowserThread::FILE, FROM_HERE, |
- base::Bind(&DownloadFileManager::UpdateDownload, |
- download_file_manager_, download_id_, buffer_)); |
+ BrowserThread::IO, FROM_HERE, |
+ base::Bind( |
+ &DownloadResourceHandler::ThrottleRequest, |
+ weak_factory_.GetWeakPtr())); |
} |
- // We schedule a pause outside of the read loop if there is too much file |
- // writing work to do. |
- if (vector_size > kLoadsToWrite) |
- StartPauseTimer(); |
+ read_buffer_ = NULL; // Drop our reference. |
return true; |
} |
@@ -332,14 +332,11 @@ void DownloadResourceHandler::OnResponseCompletedInternal( |
// If the callback was already run on the UI thread, this will be a noop. |
CallStartedCB(download_id_, error_code); |
- // We transfer ownership to |DownloadFileManager| to delete |buffer_|, |
- // so that any functions queued up on the FILE thread are executed |
- // before deletion. |
- BrowserThread::PostTask( |
- BrowserThread::FILE, FROM_HERE, |
- base::Bind(&DownloadFileManager::OnResponseCompleted, |
- download_file_manager_, download_id_, reason, security_info)); |
- buffer_ = NULL; // The buffer is longer needed by |DownloadResourceHandler|. |
+ // Send the info down the pipe. |
+ if (output_pipe_.get()) |
+ output_pipe_->SourceComplete(reason); |
+ |
+ output_pipe_ = NULL; // We no longer need the pipe. |
read_buffer_ = NULL; |
} |
@@ -391,40 +388,42 @@ void DownloadResourceHandler::set_content_disposition( |
content_disposition_ = content_disposition; |
} |
-void DownloadResourceHandler::CheckWriteProgress() { |
- if (!buffer_.get()) |
- return; // The download completed while we were waiting to run. |
- |
- size_t contents_size = buffer_->size(); |
- |
- bool should_pause = contents_size > kLoadsToWrite; |
- |
- // We'll come back later and see if it's okay to unpause the request. |
- if (should_pause) |
- StartPauseTimer(); |
- |
- if (is_paused_ != should_pause) { |
- ResourceDispatcherHostImpl::Get()->PauseRequest(global_id_.child_id, |
- global_id_.request_id, |
- should_pause); |
- is_paused_ = should_pause; |
- } |
-} |
- |
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_id_, net::ERR_ACCESS_DENIED); |
+ |
+ // Remove output pipe callback if a pipe exists. |
+ if (output_pipe_.get()) |
+ output_pipe_->RegisterSourceCallback(scoped_refptr<base::TaskRunner>(), |
+ base::Closure(), 0); |
} |
-void DownloadResourceHandler::StartPauseTimer() { |
- if (!pause_timer_.IsRunning()) |
- pause_timer_.Start(FROM_HERE, |
- base::TimeDelta::FromMilliseconds(kThrottleTimeMs), this, |
- &DownloadResourceHandler::CheckWriteProgress); |
+// Note subtlety here!! We need to post a task to ourselves to throttle the |
+// request because ResourceDispatcherHost isn't reentrant around pausing |
+// requests from ResourceHandlers. But that opens up a window during which |
+// notification of having emptied the pipe might arrive from the destination, |
+// and be dropped because we weren't paused at that point. So whenever we |
+// get a throttle request, we figure out for ourselves if we should be |
+// paused or not, and adjust state appropriately. This will only fail if |
+// we don't get a notification of state transition, and that shouldn't |
+// happen; pipe full events are detected in OnReadCompleted and posted here, |
+// and pipe unfull events are detected on destination read and posted here. |
+void DownloadResourceHandler::ThrottleRequest() { |
+ bool pause_goal = output_pipe_->IsFull(); |
+ |
+ if (pause_goal == is_paused_) |
+ // No need to do anything. |
+ return; |
+ |
+ ResourceDispatcherHostImpl::Get()->PauseRequest(global_id_.child_id, |
+ global_id_.request_id, |
+ pause_goal); |
+ is_paused_ = pause_goal; |
} |
+ |
std::string DownloadResourceHandler::DebugString() const { |
return base::StringPrintf("{" |
" url_ = " "\"%s\"" |