Add URLFetcher::SaveResponseToFileAtPath

content/public/common/url_fetcher.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_PUBLIC_COMMON_URL_FETCHER_H_
 #define CONTENT_PUBLIC_COMMON_URL_FETCHER_H_
 #pragma once
 
 #include <string>
 #include <vector>
@@ skipping 151 matching lines @@
   virtual void SetAutomaticallyRetryOn5xx(bool retry) = 0;
 
   virtual void SetMaxRetries(int max_retries) = 0;
   virtual int GetMaxRetries() const = 0;
 
   // Returns the back-off delay before the request will be retried,
   // when a 5xx response was received.
   virtual base::TimeDelta GetBackoffDelay() const = 0;
 
   // By default, the response is saved in a string. Call this method to save the
+  // response to a file instead. Must be called before Start().
+  // |file_message_loop_proxy| will be used for all file operations.

[Sam Kerner (Chrome), 2012/03/06 00:23:06]

Add something like: 
// To save to a temporary path, use SaveResponseToTemporaryFile().

[hashimoto, 2012/03/06 10:16:01]

Done.

+  virtual void SaveResponseToFile(
+      const FilePath& file_path,
+      scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy) = 0;
+
+  // By default, the response is saved in a string. Call this method to save the
   // response to a temporary file instead. Must be called before Start().
   // |file_message_loop_proxy| will be used for all file operations.
   virtual void SaveResponseToTemporaryFile(
       scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy) = 0;
 
   // Retrieve the response headers from the request.  Must only be called after
   // the OnURLFetchComplete callback has run.
   virtual net::HttpResponseHeaders* GetResponseHeaders() const = 0;
 
   // Retrieve the remote socket address from the request.  Must only
@@ skipping 41 matching lines @@
 
   // Get the response as a string. Return false if the fetcher was not
   // set to store the response as a string.
   virtual bool GetResponseAsString(std::string* out_response_string) const = 0;
 
   // Get the path to the file containing the response body. Returns false
   // if the response body was not saved to a file. If take_ownership is
   // true, caller takes responsibility for the temp file, and it will not
   // be removed once the URLFetcher is destroyed.  User should not take
   // ownership more than once, or call this method after taking ownership.
   virtual bool GetResponseAsFilePath(bool take_ownership,

[Sam Kerner (Chrome), 2012/03/06 00:23:06]

One tricky aspect of fetching a file is deleting it in error cases.  The current
logic is designed for the typical user of a temp file.  We should think about
how it ought to work for writing to a path.


For temp files, the file is owned by the URLFetcher (and will be deleted on
error) until the following method is called:

GetResponseAsFilePath(..., true);

At that point, the URLFetcher does not own the file, and will not write to it or
delete it.

If this method is not called, the temp file gets removed when the URLFetcher is
destructed.


The simplest thing you could do is to use the same semantics for files
downloaded to a path, but you need to point this out because it might be
surprising that the file gets deleted when the URLFetcher is destroyed.  A
comment above the prototype of SaveResponseToFile(), pointing out that  a user
must take ownership to avoid the removal of the file.  A comment on the delegate
base class  URLFetcherDelegate would also be a good thing.

[hashimoto, 2012/03/06 10:16:01]

Good point, added comments for SaveResponseToFileAtPath and
SaveResponseToTemporaryFile.

BTW, it looks like all of GetResponseAsFilePath's users are passing true as
|take_ownership| now.
(http://code.google.com/p/chromium/source/search?q=GetResponseAsFilePath)
Can we remove this argument?
I think it's good to remove this argument and switch to a simpler way where the
file is removed only when a file error occurs.
How about doing this in a following patch?

[Sam Kerner (Chrome), 2012/03/06 19:44:31]

Sounds good to me.  When I created his method, the extensions updater code
needed to call it without taking ownership.  I see the code has been cleaned up
and no longer does this.

                                      FilePath* out_response_path) const = 0;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_COMMON_URL_FETCHER_H_