net: Replace URLFetcherFileWriter's implementation with FileStream

BUG=126753
TEST=net_unittests


Review URL: https://chromiumcodereview.appspot.com/12334118

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192322 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 88 insertions, 143 deletions

diff --git a/net/url_request/url_fetcher_impl_unittest.cc b/net/url_request/url_fetcher_impl_unittest.cc
index e860feb..40c60cb5c 100644
--- a/net/url_request/url_fetcher_impl_unittest.cc
+++ b/net/url_request/url_fetcher_impl_unittest.cc
@@ -1379,10 +1379,10 @@ TEST_F(URLFetcherFileTest, OverwriteExistingFile) {
 
   // Create a file before trying to fetch.
   static const char kFileToFetch[] = "simple.html";
-  static const char kData[] = "abcdefghijklmnopqrstuvwxyz";
+  std::string data(10000, '?');  // Meant to be larger than simple.html.
   file_path_ = temp_dir.path().AppendASCII(kFileToFetch);
-  const int data_size = arraysize(kData);
-  ASSERT_EQ(file_util::WriteFile(file_path_, kData, data_size), data_size);
+  ASSERT_EQ(static_cast<int>(data.size()),
+            file_util::WriteFile(file_path_, data.data(), data.size()));
   ASSERT_TRUE(file_util::PathExists(file_path_));
   expected_file_ = test_server.GetDocumentRoot().AppendASCII(kFileToFetch);
   ASSERT_FALSE(file_util::ContentsEqual(file_path_, expected_file_));
diff --git a/net/url_request/url_fetcher_response_writer.cc b/net/url_request/url_fetcher_response_writer.cc
index c4e75c3..863939e 100644
--- a/net/url_request/url_fetcher_response_writer.cc
+++ b/net/url_request/url_fetcher_response_writer.cc
@@ -4,8 +4,11 @@
 
 #include "net/url_request/url_fetcher_response_writer.h"
 
-#include "base/files/file_util_proxy.h"
-#include "base/single_thread_task_runner.h"
+#include "base/file_util.h"
+#include "base/location.h"
+#include "base/task_runner.h"
+#include "base/task_runner_util.h"
+#include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 
@@ -41,7 +44,6 @@ URLFetcherFileWriter::URLFetcherFileWriter(
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       file_task_runner_(file_task_runner),
       owns_file_(false),
-      file_handle_(base::kInvalidPlatformFileValue),
       total_bytes_written_(0) {
   DCHECK(file_task_runner_.get());
 }
@@ -51,86 +53,76 @@ URLFetcherFileWriter::~URLFetcherFileWriter() {
 }
 
 int URLFetcherFileWriter::Initialize(const CompletionCallback& callback) {
-  DCHECK_EQ(base::kInvalidPlatformFileValue, file_handle_);
+  DCHECK(!file_stream_);
   DCHECK(!owns_file_);
 
+  file_stream_.reset(new FileStream(NULL));
+
+  int result = ERR_IO_PENDING;
   if (file_path_.empty()) {
-    base::FileUtilProxy::CreateTemporary(
+    base::FilePath* temp_file_path = new base::FilePath;
+    base::PostTaskAndReplyWithResult(
         file_task_runner_,
-        0,  // No additional file flags.
+        FROM_HERE,
+        base::Bind(&file_util::CreateTemporaryFile,
+                   temp_file_path),
         base::Bind(&URLFetcherFileWriter::DidCreateTempFile,
                    weak_factory_.GetWeakPtr(),
-                   callback));
+                   callback,
+                   base::Owned(temp_file_path)));
   } else {
-    base::FileUtilProxy::CreateOrOpen(
-        file_task_runner_,
+    result = file_stream_->Open(
         file_path_,
-        base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_WRITE,
-        base::Bind(&URLFetcherFileWriter::DidCreateFile,
+        base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC |
+        base::PLATFORM_FILE_CREATE_ALWAYS,
+        base::Bind(&URLFetcherFileWriter::DidOpenFile,
                    weak_factory_.GetWeakPtr(),
-                   callback,
-                   file_path_));
+                   callback));
+    DCHECK_NE(OK, result);
+    if (result != ERR_IO_PENDING)
+      error_code_ = result;
   }
-  return ERR_IO_PENDING;
+  return result;
 }
 
 int URLFetcherFileWriter::Write(IOBuffer* buffer,
                                 int num_bytes,
                                 const CompletionCallback& callback) {
-  DCHECK_NE(base::kInvalidPlatformFileValue, file_handle_);
+  DCHECK(file_stream_);
   DCHECK(owns_file_);
 
-  ContinueWrite(new DrainableIOBuffer(buffer, num_bytes), callback,
-                base::PLATFORM_FILE_OK, 0);
+  ContinueWrite(new DrainableIOBuffer(buffer, num_bytes), callback, OK);
   return ERR_IO_PENDING;
 }
 
 int URLFetcherFileWriter::Finish(const CompletionCallback& callback) {
-  if (file_handle_ != base::kInvalidPlatformFileValue) {
-    base::FileUtilProxy::Close(
-        file_task_runner_, file_handle_,
-        base::Bind(&URLFetcherFileWriter::DidCloseFile,
-                   weak_factory_.GetWeakPtr(),
-                   callback));
-    file_handle_ = base::kInvalidPlatformFileValue;
-    return ERR_IO_PENDING;
-  }
+  file_stream_.reset();
   return OK;
 }
 
 void URLFetcherFileWriter::ContinueWrite(
     scoped_refptr<DrainableIOBuffer> buffer,
     const CompletionCallback& callback,
-    base::PlatformFileError error_code,
-    int bytes_written) {
-  if (file_handle_ == base::kInvalidPlatformFileValue) {
-    // While a write was being done on the file thread, a request
-    // to close or disown the file occured on the IO thread.  At
-    // this point a request to close the file is pending on the
-    // file thread.
-    return;
-  }
+    int result) {
+  // |file_stream_| should be alive when write is in progress.
+  DCHECK(file_stream_);
 
-  const int net_error = PlatformFileErrorToNetError(error_code);
-  if (net_error != OK) {
-    error_code_ = net_error;
+  if (result < 0) {
+    error_code_ = result;
     CloseAndDeleteFile();
-    callback.Run(net_error);
+    callback.Run(result);
     return;
   }
 
-  total_bytes_written_ += bytes_written;
-  buffer->DidConsume(bytes_written);
+  total_bytes_written_ += result;
+  buffer->DidConsume(result);
 
   if (buffer->BytesRemaining() > 0) {
-    base::FileUtilProxy::Write(
-        file_task_runner_, file_handle_,
-        total_bytes_written_,  // Append to the end
-        buffer->data(), buffer->BytesRemaining(),
-        base::Bind(&URLFetcherFileWriter::ContinueWrite,
-                   weak_factory_.GetWeakPtr(),
-                   buffer,
-                   callback));
+    file_stream_->Write(buffer, buffer->BytesRemaining(),
+                        base::Bind(&URLFetcherFileWriter::ContinueWrite,
+                                   weak_factory_.GetWeakPtr(),
+                                   buffer,
+                                   callback));
   } else {
     // Finished writing buffer to the file.
     callback.Run(buffer->size());
@@ -140,7 +132,7 @@ void URLFetcherFileWriter::ContinueWrite(
 void URLFetcherFileWriter::DisownFile() {
   // Disowning is done by the delegate's OnURLFetchComplete method.
   // The file should be closed by the time that method is called.
-  DCHECK_EQ(base::kInvalidPlatformFileValue, file_handle_);
+  DCHECK(!file_stream_);
 
   owns_file_ = false;
 }
@@ -149,71 +141,45 @@ void URLFetcherFileWriter::CloseAndDeleteFile() {
   if (!owns_file_)
     return;
 
-  if (file_handle_ == base::kInvalidPlatformFileValue) {
-    DeleteFile(base::PLATFORM_FILE_OK);
+  file_stream_.reset();
+  DisownFile();
+  file_task_runner_->PostTask(FROM_HERE,
+                              base::Bind(base::IgnoreResult(&file_util::Delete),
+                                         file_path_,
+                                         false /* recursive */));
+}
+
+void URLFetcherFileWriter::DidCreateTempFile(const CompletionCallback& callback,
+                                             base::FilePath* temp_file_path,
+                                             bool success) {
+  if (!success) {
+    error_code_ = ERR_FILE_NOT_FOUND;
+    callback.Run(error_code_);
     return;
   }
-  // Close the file if it is open.
-  base::FileUtilProxy::Close(
-      file_task_runner_, file_handle_,
-      base::Bind(&URLFetcherFileWriter::DeleteFile,
-                 weak_factory_.GetWeakPtr()));
-  file_handle_ = base::kInvalidPlatformFileValue;
-}
-
-void URLFetcherFileWriter::DeleteFile(base::PlatformFileError error_code) {
-  if (file_path_.empty())
-    return;
-
-  base::FileUtilProxy::Delete(
-      file_task_runner_, file_path_,
-      false,  // No need to recurse, as the path is to a file.
-      base::FileUtilProxy::StatusCallback());
-  DisownFile();
-}
-
-void URLFetcherFileWriter::DidCreateFile(
-    const CompletionCallback& callback,
-    const base::FilePath& file_path,
-    base::PlatformFileError error_code,
-    base::PassPlatformFile file_handle,
-    bool created) {
-  DidCreateFileInternal(callback, file_path, error_code, file_handle);
-}
-
-void URLFetcherFileWriter::DidCreateTempFile(
-    const CompletionCallback& callback,
-    base::PlatformFileError error_code,
-    base::PassPlatformFile file_handle,
-    const base::FilePath& file_path) {
-  DidCreateFileInternal(callback, file_path, error_code, file_handle);
-}
-
-void URLFetcherFileWriter::DidCreateFileInternal(
-    const CompletionCallback& callback,
-    const base::FilePath& file_path,
-    base::PlatformFileError error_code,
-    base::PassPlatformFile file_handle) {
-  const int net_error = PlatformFileErrorToNetError(error_code);
-  if (net_error == OK) {
-    file_path_ = file_path;
-    file_handle_ = file_handle.ReleaseValue();
+  file_path_ = *temp_file_path;
+  owns_file_ = true;
+  const int result = file_stream_->Open(
+      file_path_,
+      base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC |
+      base::PLATFORM_FILE_OPEN,
+      base::Bind(&URLFetcherFileWriter::DidOpenFile,
+                 weak_factory_.GetWeakPtr(),
+                 callback));
+  if (result != ERR_IO_PENDING)
+    DidOpenFile(callback, result);
+}
+
+void URLFetcherFileWriter::DidOpenFile(const CompletionCallback& callback,
+                                       int result) {
+  if (result == OK) {
     total_bytes_written_ = 0;
     owns_file_ = true;
   } else {
-    error_code_ = net_error;
-  }
-  callback.Run(net_error);
-}
-
-void URLFetcherFileWriter::DidCloseFile(const CompletionCallback& callback,
-                                        base::PlatformFileError error_code) {
-  const int net_error = PlatformFileErrorToNetError(error_code);
-  if (net_error != OK) {
-    error_code_ = net_error;
+    error_code_ = result;
     CloseAndDeleteFile();
   }
-  callback.Run(net_error);
+  callback.Run(result);
 }
 
 }  // namespace net
diff --git a/net/url_request/url_fetcher_response_writer.h b/net/url_request/url_fetcher_response_writer.h
index d340806..85fabc5 100644
--- a/net/url_request/url_fetcher_response_writer.h
+++ b/net/url_request/url_fetcher_response_writer.h
@@ -6,19 +6,20 @@
 #define NET_URL_REQUEST_URL_FETCHER_RESPONSE_WRITER_H_
 
 #include "base/basictypes.h"
+#include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/platform_file.h"
 #include "net/base/completion_callback.h"
 
 namespace base {
-class FilePath;
 class TaskRunner;
 }  // namespace base
 
 namespace net {
 
 class DrainableIOBuffer;
+class FileStream;
 class IOBuffer;
 
 // This class encapsulates all state involved in writing URLFetcher response
@@ -87,8 +88,7 @@ class URLFetcherFileWriter : public URLFetcherResponseWriter {
   // bytes to write.  Otherwise, runs |callback|.
   void ContinueWrite(scoped_refptr<DrainableIOBuffer> buffer,
                      const CompletionCallback& callback,
-                     base::PlatformFileError error_code,
-                     int bytes_written);
+                     int result);
 
   // Drops ownership of the file at |file_path_|.
   // This class will not delete it or write to it again.
@@ -106,34 +106,14 @@ class URLFetcherFileWriter : public URLFetcherResponseWriter {
   int error_code() const { return error_code_; }
 
  private:
-  // Callback which gets the result of a permanent file creation.
-  void DidCreateFile(const CompletionCallback& callback,
-                     const base::FilePath& file_path,
-                     base::PlatformFileError error_code,
-                     base::PassPlatformFile file_handle,
-                     bool created);
-
   // Callback which gets the result of a temporary file creation.
   void DidCreateTempFile(const CompletionCallback& callback,
-                         base::PlatformFileError error_code,
-                         base::PassPlatformFile file_handle,
-                         const base::FilePath& file_path);
-
-  // This method is used to implement DidCreateFile and DidCreateTempFile.
-  // |callback| is run with the result.
-  void DidCreateFileInternal(const CompletionCallback& callback,
-                             const base::FilePath& file_path,
-                             base::PlatformFileError error_code,
-                             base::PassPlatformFile file_handle);
-
-  // Callback which gets the result of closing the file.
-  // |callback| is run with the result.
-  void DidCloseFile(const CompletionCallback& callback,
-                    base::PlatformFileError error);
-
-  // Callback which gets the result of closing the file. Deletes the file if
-  // it has been created.
-  void DeleteFile(base::PlatformFileError error_code);
+                         base::FilePath* temp_file_path,
+                         bool success);
+
+  // Callback which gets the result of FileStream::Open.
+  void DidOpenFile(const CompletionCallback& callback,
+                   int result);
 
   // The last error encountered on a file operation.  OK if no error occurred.
   int error_code_;
@@ -151,8 +131,7 @@ class URLFetcherFileWriter : public URLFetcherResponseWriter {
   // True when this instance is responsible to delete the file at |file_path_|.
   bool owns_file_;
 
-  // Handle to the file.
-  base::PlatformFile file_handle_;
+  scoped_ptr<FileStream> file_stream_;
 
   // We always append to the file.  Track the total number of bytes
   // written, so that writes know the offset to give.