Allow URLFetcher to save results in a file. Have CRX updates use this capability.

chrome/common/net/url_fetcher.h

Index: chrome/common/net/url_fetcher.h
diff --git a/chrome/common/net/url_fetcher.h b/chrome/common/net/url_fetcher.h
index f4fcf77ee5696ae4ae6c5ea7b06038cf2368d557..6a485be200f9b0cb9b5779b488a3eab97d447382 100644
--- a/chrome/common/net/url_fetcher.h
+++ b/chrome/common/net/url_fetcher.h
@@ -20,10 +20,16 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
+#include "base/platform_file.h"
 #include "base/time.h"
 
+class FilePath;
 class GURL;
 
+namespace base {
+class MessageLoopProxy;
+}  // namespace base
+
 namespace net {
 class HostPortPair;
 class HttpResponseHeaders;
@@ -44,14 +50,15 @@ typedef std::vector<std::string> ResponseCookies;
 // Finally, start the request:
 //   fetcher->Start();
 //
+//
 // The object you supply as a delegate must inherit from URLFetcher::Delegate;
-// when the fetch is completed, OnURLFetchComplete() will be called with the
-// resulting status and (if applicable) HTTP response code.  From that point
-// until the original URLFetcher instance is destroyed, you may examine the
-// provided status and data for the URL.  (You should copy these objects if you
-// need them to live longer than the URLFetcher instance.)  If the URLFetcher
-// instance is destroyed before the callback happens, the fetch will be
-// canceled and no callback will occur.
+// when the fetch is completed, OnURLFetchComplete() will be called with a
+// pointer to the URLFetcher.  From that point until the original URLFetcher
+// instance is destroyed, you may use accessor methods to see the result of
+// the fetch. You should copy these objects if you need them to live longer
+// than the URLFetcher instance. If the URLFetcher instance is destroyed
+// before the callback happens, the fetch will be canceled and no callback
+// will occur.
 //
 // You may create the URLFetcher instance on any thread; OnURLFetchComplete()
 // will be called back on the same thread you use to create the instance.
@@ -68,18 +75,25 @@ class URLFetcher {
     HEAD,
   };
 
+  // Imposible http response code. Used to signal that no http response code
+  // was received.
+  static const int kInvalidHttpResponseCode;
+
   class Delegate {
    public:
-    // This will be called when the URL has been fetched, successfully or not.
-    // |response_code| is the HTTP response code (200, 404, etc.) if
-    // applicable.  |url|, |status| and |data| are all valid until the
-    // URLFetcher instance is destroyed.
+    // TODO(skerner): This will be removed in favor of the |source|-only
+    // version below. Leaving this for now to make the initial code review
+    // easy to read.
     virtual void OnURLFetchComplete(const URLFetcher* source,
                                     const GURL& url,
                                     const net::URLRequestStatus& status,
                                     int response_code,
                                     const net::ResponseCookies& cookies,
-                                    const std::string& data) = 0;
+                                    const std::string& data);
+
+    // This will be called when the URL has been fetched, successfully or not.
+    // Use accessor methods on |source| to get the results.
+    virtual void OnURLFetchComplete(const URLFetcher* source);
 
    protected:
     virtual ~Delegate() {}
@@ -183,6 +197,17 @@ class URLFetcher {
   }
 #endif  // defined(UNIT_TEST)
 
+  // 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().
+  // User must also call set_file_message_loop_proxy() to give the thread on
+  // which file operations may be done.
+  void SaveResponseToTemporaryFile();

[asargent_no_longer_on_chrome, 2011/05/18 19:00:49]

Here's a thought for future cleanup along the lines of Darin's concern about it
being confusing what methods must be called before Start. Would it make sense to
move to a pattern where there is a URLFetcherOptions struct that you pass to the
Start method? Then instead of:

my_fetcher_ = new URLFetcher();
my_fetcher_->SaveResponseToTemporaryFile();
my_fetcher_->Start();

it would be:

URLFetcherOptions options;
options.save_response_to_tempfile = true;
my_fetcher_ = new URLFetcher();
my_fetcher_->Start(&options);

[Sam Kerner (Chrome), 2011/05/18 23:45:57]

A good idea, but more than I can chew in this CL.

+
+  // Set the message loop proxy for file operations. Used to write the

[asargent_no_longer_on_chrome, 2011/05/18 19:00:49]

Are there other file operations that this would apply to apart from
SaveResponseToTemporaryFile? If not, why not just have the MessageLoopProxy be a
required parameter to SaveResponseToTemporaryFile()?

[Sam Kerner (Chrome), 2011/05/18 23:45:57]

Done.

+  // response to a file.
+  void set_file_message_loop_proxy(
+      scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy);
+
   // Retrieve the response headers from the request.  Must only be called after
   // the OnURLFetchComplete callback has run.
   virtual net::HttpResponseHeaders* response_headers() const;
@@ -202,11 +227,38 @@ class URLFetcher {
   virtual void Start();
 
   // Return the URL that this fetcher is processing.
-  const GURL& url() const;
+  virtual const GURL& url() const;
+
+  // The status of the URL fetch.
+  virtual const net::URLRequestStatus& status() const;
+
+  // The http response code received. Will return
+  // URLFetcher::kInvalidHttpResponseCode if an error prevented any response
+  // from being received.
+  virtual int response_code() const;
+
+  // Cookies recieved.
+  virtual const net::ResponseCookies& cookies() const;
+
+  // Return true if any file system operation failed.  If so, set |error_code|
+  // to the error code. File system errors are only possible if user called
+  // SaveResponseToTemporaryFile().
+  virtual bool FileErrorOccurred(base::PlatformFileError* out_error_code) const;
 
   // Reports that the received content was malformed.
   void ReceivedContentWasMalformed();
 
+  // 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;
+
+  // 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.
+  virtual bool GetResponseAsFilePath(bool take_ownership,
+                                     FilePath* out_response_path) const;
+
   // Cancels all existing URLFetchers.  Will notify the URLFetcher::Delegates.
   // Note that any new URLFetchers created while this is running will not be
   // cancelled.  Typically, one would call this in the CleanUp() method of an IO
@@ -224,13 +276,19 @@ class URLFetcher {
 
  private:
   friend class URLFetcherTest;
+  friend class TestURLFetcher;
+
+  // How should the response be stored?
+  enum ResponseDestinationType {
+    STRING,  // Default: In a std::string
+    TEMP_FILE  // Write to a temp file
+  };
 
   // Only used by URLFetcherTest, returns the number of URLFetcher::Core objects
   // actively running.
   static int GetNumFetcherCores();
 
   class Core;
-
   scoped_refptr<Core> core_;
 
   static Factory* factory_;
@@ -245,6 +303,9 @@ class URLFetcher {
   // Maximum retries allowed.
   int max_retries_;
 
+  // Where should responses be saved?
+  ResponseDestinationType response_destination_;
+
   static bool g_interception_enabled;
 
   DISALLOW_COPY_AND_ASSIGN(URLFetcher);