Add URLFetcher::SaveResponseToFileAtPath

content/common/net/url_fetcher_impl.cc

Index: content/common/net/url_fetcher_impl.cc
diff --git a/content/common/net/url_fetcher_impl.cc b/content/common/net/url_fetcher_impl.cc
index be442898bdd0e5ee55900bcb1948c63207ad150f..7c3c43e2063a9395475d6cedc687ef9b736319f4 100644
--- a/content/common/net/url_fetcher_impl.cc
+++ b/content/common/net/url_fetcher_impl.cc
@@ -100,7 +100,8 @@ class URLFetcherImpl::Core
   };
 
   // Class FileWriter encapsulates all state involved in writing response bytes
-  // to a file. It is only used if |Core::response_destination_| == FILE.
+  // to a file. It is only used if |Core::response_destination_| == TEMP_FILE ||
+  // |Core::response_destination_| == PERMANENT_FILE.
   // Each instance of FileWriter is owned by a URLFetcher::Core, which manages
   // its lifetime and never transfers ownership.  While writing to
   // a file, all function calls happen on the IO thread.
@@ -110,6 +111,11 @@ class URLFetcherImpl::Core
                scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy);
 
     ~FileWriter();
+    void CreateFileAtPath(const FilePath& file_path);
+    void DidCreateFile(const FilePath& file_path,
+                       base::PlatformFileError error_code,
+                       base::PassPlatformFile file_handle,
+                       bool created);
     void CreateTempFile();
     void DidCreateTempFile(base::PlatformFileError error_code,
                            base::PassPlatformFile file_handle,
@@ -282,6 +288,9 @@ class URLFetcherImpl::Core
   // Where should responses be saved?
   ResponseDestinationType response_destination_;
 
+  // Path to the file where the response is written.
+  FilePath response_destination_file_path_;
+
   // If |automatically_retry_on_5xx_| is false, 5xx responses will be
   // propagated to the observer, if it is true URLFetcher will automatically
   // re-execute the request, after the back-off delay has expired.
@@ -339,20 +348,24 @@ URLFetcherImpl::Core::FileWriter::~FileWriter() {
   RemoveFile();
 }
 
-void URLFetcherImpl::Core::FileWriter::CreateTempFile() {
+void URLFetcherImpl::Core::FileWriter::CreateFileAtPath(
+    const FilePath& file_path) {
   DCHECK(core_->io_message_loop_proxy_->BelongsToCurrentThread());
   DCHECK(file_message_loop_proxy_.get());
-  base::FileUtilProxy::CreateTemporary(
+  base::FileUtilProxy::CreateOrOpen(
       file_message_loop_proxy_,
-      0,  // No additional file flags.
-      base::Bind(&URLFetcherImpl::Core::FileWriter::DidCreateTempFile,
-                 weak_factory_.GetWeakPtr()));
+      file_path,
+      base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_WRITE,
+      base::Bind(&URLFetcherImpl::Core::FileWriter::DidCreateFile,
+                 weak_factory_.GetWeakPtr(),
+                 file_path));
 }
 
-void URLFetcherImpl::Core::FileWriter::DidCreateTempFile(
+void URLFetcherImpl::Core::FileWriter::DidCreateFile(
+    const FilePath& file_path,
     base::PlatformFileError error_code,
     base::PassPlatformFile file_handle,
-    const FilePath& file_path) {
+    bool created) {

[willchan no longer on Chromium, 2012/03/08 16:38:39]

Is this just ignored? It's sorta weird that DidCreateTempFile() is calling
DidCreateFile() with kCreated = true, when created is just ignored.

[hashimoto, 2012/03/09 03:22:52]

Added DidCreateFileInternal

   DCHECK(core_->io_message_loop_proxy_->BelongsToCurrentThread());
 
   if (base::PLATFORM_FILE_OK != error_code) {
@@ -371,6 +384,24 @@ void URLFetcherImpl::Core::FileWriter::DidCreateTempFile(
       FROM_HERE, base::Bind(&Core::StartURLRequestWhenAppropriate, core_));
 }
 
+void URLFetcherImpl::Core::FileWriter::CreateTempFile() {
+  DCHECK(core_->io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(file_message_loop_proxy_.get());
+  base::FileUtilProxy::CreateTemporary(
+      file_message_loop_proxy_,
+      0,  // No additional file flags.
+      base::Bind(&URLFetcherImpl::Core::FileWriter::DidCreateTempFile,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void URLFetcherImpl::Core::FileWriter::DidCreateTempFile(
+    base::PlatformFileError error_code,
+    base::PassPlatformFile file_handle,
+    const FilePath& file_path) {
+  const bool kCreated = true;
+  DidCreateFile(file_path, error_code, file_handle, kCreated);
+}
+
 void URLFetcherImpl::Core::FileWriter::WriteBuffer(int num_bytes) {
   DCHECK(core_->io_message_loop_proxy_->BelongsToCurrentThread());
 
@@ -583,16 +614,25 @@ void URLFetcherImpl::Core::StartOnIOThread() {
       StartURLRequestWhenAppropriate();
       break;
 
-    case FILE:
+    case PERMANENT_FILE:
+    case TEMP_FILE:
       DCHECK(file_message_loop_proxy_.get())
           << "Need to set the file message loop proxy.";
 
-      file_writer_.reset(
-          new FileWriter(this, file_message_loop_proxy_));
+      file_writer_.reset(new FileWriter(this, file_message_loop_proxy_));
 
-      // If the temp file is successfully created,
+      // If the file is successfully created,
       // Core::StartURLRequestWhenAppropriate() will be called.
-      file_writer_->CreateTempFile();
+      switch (response_destination_) {
+        case PERMANENT_FILE:
+          file_writer_->CreateFileAtPath(response_destination_file_path_);
+          break;
+        case TEMP_FILE:
+          file_writer_->CreateTempFile();
+          break;
+        default:
+          NOTREACHED();
+      }
       break;
 
     default:
@@ -669,7 +709,8 @@ bool URLFetcherImpl::Core::WriteBuffer(int num_bytes) {
       write_complete = true;
       break;
 
-    case FILE:
+    case PERMANENT_FILE:
+    case TEMP_FILE:
       file_writer_->WriteBuffer(num_bytes);
       // WriteBuffer() sends a request the file thread.
       // The write is not done yet.
@@ -1032,10 +1073,18 @@ base::TimeDelta URLFetcherImpl::GetBackoffDelay() const {
   return core_->backoff_delay_;
 }
 
+void URLFetcherImpl::SaveResponseToFileAtPath(
+    const FilePath& file_path,
+    scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy) {
+  core_->file_message_loop_proxy_ = file_message_loop_proxy;

[willchan no longer on Chromium, 2012/03/08 16:38:39]

Please DCHECK that this executes on the delegate message loop. Same with the
SaveResponseToTemporaryFile(). It'd be great to DCHECK() that Start() hasn't
been called yet too, but I don't see a good way to do that.

[hashimoto, 2012/03/09 03:22:52]

Done.
Can we do this in another change?
Maybe we should have a bool member in |core_| to remember if Start() was already
called?

+  core_->response_destination_ = PERMANENT_FILE;
+  core_->response_destination_file_path_ = file_path;
+}
+
 void URLFetcherImpl::SaveResponseToTemporaryFile(
     scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy) {
   core_->file_message_loop_proxy_ = file_message_loop_proxy;
-  core_->response_destination_ = FILE;
+  core_->response_destination_ = TEMP_FILE;
 }
 
 net::HttpResponseHeaders* URLFetcherImpl::GetResponseHeaders() const {
@@ -1126,7 +1175,9 @@ bool URLFetcherImpl::GetResponseAsString(
 bool URLFetcherImpl::GetResponseAsFilePath(bool take_ownership,
                                            FilePath* out_response_path) const {
   DCHECK(core_->delegate_loop_proxy_->BelongsToCurrentThread());
-  if (core_->response_destination_ != FILE || !core_->file_writer_.get())
+  const bool destination_is_file = core_->response_destination_ == TEMP_FILE ||
+      core_->response_destination_ == PERMANENT_FILE;
+  if (!destination_is_file || !core_->file_writer_.get())
     return false;
 
   *out_response_path = core_->file_writer_->file_path();