Revert 256688 "Fix various issues in RedirectToFileResourceHandler."

trunk/src/content/browser/loader/redirect_to_file_resource_handler.cc

Index: trunk/src/content/browser/loader/redirect_to_file_resource_handler.cc
===================================================================
--- trunk/src/content/browser/loader/redirect_to_file_resource_handler.cc	(revision 256703)
+++ trunk/src/content/browser/loader/redirect_to_file_resource_handler.cc	(working copy)
@@ -5,12 +5,14 @@
 #include "content/browser/loader/redirect_to_file_resource_handler.h"
 
 #include "base/bind.h"
+#include "base/files/file_util_proxy.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/platform_file.h"
 #include "base/threading/thread_restrictions.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
-#include "content/browser/loader/temporary_file_stream.h"
-#include "content/public/browser/resource_controller.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/common/resource_response.h"
 #include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
@@ -51,118 +53,33 @@
 static const int kInitialReadBufSize = 32768;
 static const int kMaxReadBufSize = 524288;
 
-// A separate IO thread object to manage the lifetime of the net::FileStream and
-// the ShareableFileReference. When the handler is destroyed, it asynchronously
-// closes net::FileStream after all pending writes complete. Only after the
-// stream is closed is the ShareableFileReference released, to ensure the
-// temporary is not deleted before it is closed.
-class RedirectToFileResourceHandler::Writer {
- public:
-  Writer(RedirectToFileResourceHandler* handler,
-         scoped_ptr<net::FileStream> file_stream,
-         ShareableFileReference* deletable_file)
-      : handler_(handler),
-        file_stream_(file_stream.Pass()),
-        is_writing_(false),
-        deletable_file_(deletable_file) {
-    DCHECK(!deletable_file_->path().empty());
-  }
-
-  bool is_writing() const { return is_writing_; }
-  const base::FilePath& path() const { return deletable_file_->path(); }
-
-  int Write(net::IOBuffer* buf, int buf_len) {
-    DCHECK(!is_writing_);
-    DCHECK(handler_);
-    int result = file_stream_->Write(
-        buf, buf_len,
-        base::Bind(&Writer::DidWriteToFile, base::Unretained(this)));
-    if (result == net::ERR_IO_PENDING)
-      is_writing_ = true;
-    return result;
-  }
-
-  void Close() {
-    handler_ = NULL;
-    if (!is_writing_)
-      CloseAndDelete();
-  }
-
- private:
-  // Only DidClose can delete this.
-  ~Writer() {
-  }
-
-  void DidWriteToFile(int result) {
-    DCHECK(is_writing_);
-    is_writing_ = false;
-    if (handler_) {
-      handler_->DidWriteToFile(result);
-    } else {
-      CloseAndDelete();
-    }
-  }
-
-  void CloseAndDelete() {
-    DCHECK(!is_writing_);
-    int result = file_stream_->Close(base::Bind(&Writer::DidClose,
-                                                base::Unretained(this)));
-    if (result != net::ERR_IO_PENDING)
-      DidClose(result);
-  }
-
-  void DidClose(int result) {
-    delete this;
-  }
-
-  RedirectToFileResourceHandler* handler_;
-
-  scoped_ptr<net::FileStream> file_stream_;
-  bool is_writing_;
-
-  // We create a ShareableFileReference that's deletable for the temp file
-  // created as a result of the download.
-  scoped_refptr<webkit_blob::ShareableFileReference> deletable_file_;
-
-  DISALLOW_COPY_AND_ASSIGN(Writer);
-};
-
 RedirectToFileResourceHandler::RedirectToFileResourceHandler(
     scoped_ptr<ResourceHandler> next_handler,
-    net::URLRequest* request)
+    net::URLRequest* request,
+    ResourceDispatcherHostImpl* host)
     : LayeredResourceHandler(request, next_handler.Pass()),
+      weak_factory_(this),
+      host_(host),
       buf_(new net::GrowableIOBuffer()),
       buf_write_pending_(false),
       write_cursor_(0),
-      writer_(NULL),
+      write_callback_pending_(false),
       next_buffer_size_(kInitialReadBufSize),
       did_defer_(false),
-      completed_during_write_(false),
-      weak_factory_(this) {
+      completed_during_write_(false) {
 }
 
 RedirectToFileResourceHandler::~RedirectToFileResourceHandler() {
-  // Orphan the writer to asynchronously close and release the temporary file.
-  if (writer_) {
-    writer_->Close();
-    writer_ = NULL;
-  }
 }
 
-void RedirectToFileResourceHandler::
-    SetCreateTemporaryFileStreamFunctionForTesting(
-        const CreateTemporaryFileStreamFunction& create_temporary_file_stream) {
-  create_temporary_file_stream_ = create_temporary_file_stream;
-}
-
 bool RedirectToFileResourceHandler::OnResponseStarted(
     int request_id,
     ResourceResponse* response,
     bool* defer) {
   if (response->head.error_code == net::OK ||
       response->head.error_code == net::ERR_IO_PENDING) {
-    DCHECK(writer_);
-    response->head.download_file_path = writer_->path();
+    DCHECK(deletable_file_.get() && !deletable_file_->path().empty());
+    response->head.download_file_path = deletable_file_->path();
   }
   return next_handler_->OnResponseStarted(request_id, response, defer);
 }
@@ -170,23 +87,19 @@
 bool RedirectToFileResourceHandler::OnWillStart(int request_id,
                                                 const GURL& url,
                                                 bool* defer) {
-  DCHECK(!writer_);
-
-  // Defer starting the request until we have created the temporary file.
-  // TODO(darin): This is sub-optimal.  We should not delay starting the
-  // network request like this.
-  will_start_url_ = url;
-  did_defer_ = *defer = true;
-  if (create_temporary_file_stream_.is_null()) {
-    CreateTemporaryFileStream(
+  if (!deletable_file_.get()) {
+    // Defer starting the request until we have created the temporary file.
+    // TODO(darin): This is sub-optimal.  We should not delay starting the
+    // network request like this.
+    did_defer_ = *defer = true;
+    base::FileUtilProxy::CreateTemporary(
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE).get(),
+        base::PLATFORM_FILE_ASYNC,
         base::Bind(&RedirectToFileResourceHandler::DidCreateTemporaryFile,
                    weak_factory_.GetWeakPtr()));
-  } else {
-    create_temporary_file_stream_.Run(
-        base::Bind(&RedirectToFileResourceHandler::DidCreateTemporaryFile,
-                   weak_factory_.GetWeakPtr()));
+    return true;
   }
-  return true;
+  return next_handler_->OnWillStart(request_id, url, defer);
 }
 
 bool RedirectToFileResourceHandler::OnWillRead(
@@ -238,7 +151,7 @@
     const net::URLRequestStatus& status,
     const std::string& security_info,
     bool* defer) {
-  if (writer_ && writer_->is_writing()) {
+  if (write_callback_pending_) {
     completed_during_write_ = true;
     completed_status_ = status;
     completed_security_info_ = security_info;
@@ -250,31 +163,25 @@
 }
 
 void RedirectToFileResourceHandler::DidCreateTemporaryFile(
-    base::File::Error error_code,
-    scoped_ptr<net::FileStream> file_stream,
-    ShareableFileReference* deletable_file) {
-  DCHECK(!writer_);
-  if (error_code != base::File::FILE_OK) {
-    controller()->CancelWithError(net::FileErrorToNetError(error_code));
-    return;
-  }
-
-  writer_ = new Writer(this, file_stream.Pass(), deletable_file);
-
-  // Resume the request.
-  DCHECK(did_defer_);
-  bool defer = false;
-  if (!next_handler_->OnWillStart(GetRequestID(), will_start_url_, &defer)) {
-    controller()->Cancel();
-  } else if (!defer) {
-    ResumeIfDeferred();
-  } else {
-    did_defer_ = false;
-  }
-  will_start_url_ = GURL();
+    base::File::Error /*error_code*/,
+    base::PassPlatformFile file_handle,
+    const base::FilePath& file_path) {
+  deletable_file_ = ShareableFileReference::GetOrCreate(
+      file_path,
+      ShareableFileReference::DELETE_ON_FINAL_RELEASE,
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE).get());
+  file_stream_.reset(
+      new net::FileStream(file_handle.ReleaseValue(),
+                          base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC,
+                          NULL));
+  const ResourceRequestInfo* info = GetRequestInfo();
+  host_->RegisterDownloadedTempFile(
+      info->GetChildID(), info->GetRequestID(), deletable_file_.get());
+  ResumeIfDeferred();
 }
 
 void RedirectToFileResourceHandler::DidWriteToFile(int result) {
+  write_callback_pending_ = false;
   int request_id = GetRequestID();
 
   bool failed = false;
@@ -287,38 +194,20 @@
   }
 
   if (failed) {
-    DCHECK(!writer_->is_writing());
-    // TODO(davidben): Recover the error code from WriteMore or |result|, as
-    // appropriate.
-    if (completed_during_write_ && completed_status_.is_success()) {
-      // If the request successfully completed mid-write, but the write failed,
-      // convert the status to a failure for downstream.
-      completed_status_.set_status(net::URLRequestStatus::CANCELED);
-      completed_status_.set_error(net::ERR_FAILED);
-    }
-    if (!completed_during_write_)
-      controller()->CancelWithError(net::ERR_FAILED);
-  }
-
-  if (completed_during_write_ && !writer_->is_writing()) {
-    // Resume shutdown now that all data has been written to disk. Note that
-    // this should run even in the |failed| case above, otherwise a failed write
-    // leaves the handler stuck.
+    ResumeIfDeferred();
+  } else if (completed_during_write_) {
     bool defer = false;
     next_handler_->OnResponseCompleted(request_id,
                                        completed_status_,
                                        completed_security_info_,
                                        &defer);
-    if (!defer) {
+    if (!defer)
       ResumeIfDeferred();
-    } else {
-      did_defer_ = false;
-    }
   }
 }
 
 bool RedirectToFileResourceHandler::WriteMore() {
-  DCHECK(writer_);
+  DCHECK(file_stream_.get());
   for (;;) {
     if (write_cursor_ == buf_->offset()) {
       // We've caught up to the network load, but it may be in the process of
@@ -331,7 +220,7 @@
       }
       return true;
     }
-    if (writer_->is_writing())
+    if (write_callback_pending_)
       return true;
     DCHECK(write_cursor_ < buf_->offset());
 
@@ -353,9 +242,15 @@
         buf_.get(), buf_->StartOfBuffer() + write_cursor_);
     int write_len = buf_->offset() - write_cursor_;
 
-    int rv = writer_->Write(wrapped.get(), write_len);
-    if (rv == net::ERR_IO_PENDING)
+    int rv = file_stream_->Write(
+        wrapped.get(),
+        write_len,
+        base::Bind(&RedirectToFileResourceHandler::DidWriteToFile,
+                   base::Unretained(this)));
+    if (rv == net::ERR_IO_PENDING) {
+      write_callback_pending_ = true;
       return true;
+    }
     if (rv <= 0)
       return false;
     next_handler_->OnDataDownloaded(GetRequestID(), rv);