Initial implementation of the ByteStream refactor.

content/browser/download/download_resource_handler.cc

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 7dc717becf6676cbd2236e6f1f1f90897bae0079..f8a534b44487898fb1bb7fb43871ca6f6954706a 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,13 @@ 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()));
+
   // Deleted in DownloadManager.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo(
       base::Time::Now(), 0, content_length_, DownloadItem::IN_PROGRESS,
@@ -163,20 +171,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

[ahendrickson, 2012/04/16 15:14:27]

Nice!

[Randy Smith (Not in Mondays), 2012/04/18 19:10:38]

:-).

-  // 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 +230,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;
 }
@@ -331,14 +330,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;
 }
 
@@ -390,40 +386,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());
 }
 
-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\""