Schetch changes required to support URLFetcher saving response to a file

chrome/common/net/url_fetcher.cc

Index: chrome/common/net/url_fetcher.cc
diff --git a/chrome/common/net/url_fetcher.cc b/chrome/common/net/url_fetcher.cc
index 29695646bdf8e3059204a797f052930d9cdde859..3c4b519f0f109928919a8dd89bbe4124e89da648 100644
--- a/chrome/common/net/url_fetcher.cc
+++ b/chrome/common/net/url_fetcher.cc
@@ -26,6 +26,55 @@
 
 static const int kBufferSize = 4096;
 
+// Hold the body of a response to a URL fetch in a std::string.
+class StringResponseBodyContainer
+    : public URLFetcher::ResponseBodyContainerInterface {
+ public:
+  StringResponseBodyContainer() {}
+
+  virtual void Append(char* data, int data_length) {

[darin (slow to review), 2011/04/28 03:25:17]

if the goal is to have an alternative implementation of this that writes to a
file, then this API is not good.  it implies that files writes will happen
synchronously on this thread.  instead, because Append is called on the IO
thread, you need to use a FileStream object to asynchronously write to the file
so that you do not block the IO thread.

however, given this interface, you may end up generating "writes" faster than
you can process them, and a large queue of pending writes might build up, which
could cause serious problems.

this API would need to have a way to signal to the caller to back off... and
then another way to signal to the caller when it is time to resume.  that sounds
complicated.

for the above reason, and because i don't think we will need any other
implementations of ResponseBodyContainerInterface, I think it would be better to
just teach URLFetcher how to optionally save data to a file.

I recommend moving the 'string' based result to a getter function on URLFetcher,
and I would add a similar getter for accessing the temp file produced when
saving to a file.  There would be a configuration option to select between these
two saving modes.  I'd also spec that the temp file gets deleted when the
URLFetcher is destroyed.  (Be sure to delete the tep file on the FILE thread
using FileUtilProxy.)

Note: URLFetcher is responsible for calling URLRequest::Read, so it can easily
implement the back off and resume logic I was talking about.

[Sam Kerner (Chrome), 2011/04/28 04:24:58]

My plan was to have each call to Append() send a message to the file thread to
write the data.  But you anticipated the problem I hit: 
That makes sense.  Thanks for reviewing this.

+    data_.append(data, data_length);
+  }
+
+  virtual bool GetAsString(std::string* out_body) const {
+    *out_body = data_;
+    return true;
+  }
+
+ private:
+  std::string data_;
+};
+
+// Default implementation of OnURLFetchCompleteNew() takes the response
+// body, makes it into a string, and calls the old callback.
+void URLFetcher::Delegate::OnURLFetchCompleteNew(
+    const URLFetcher* source,
+    const GURL& url,
+    const net::URLRequestStatus& status,
+    int response_code,
+    const ResponseCookies& cookies,
+    const ResponseBodyContainerInterface& response_body) {
+
+  std::string response_data;
+
+  // If the response container doesn't support getting the response as
+  // a string, then it is not the default string container. The delegate that
+  // passed in its wn container should have overriden this method and
+  // dealt with the container itself.
+  CHECK(response_body.GetAsString(&response_data));
+
+  // To avoid changing existing code, call the old callback with a string
+  // holding the response.
+  OnURLFetchComplete(
+      source, url,  status, response_code, cookies, response_data);
+}
+
+URLFetcher::ResponseBodyContainerInterface*
+URLFetcher::Delegate::CreateResponseBodyContainer() {
+  // If subclass does not override, use a string.
+  return new StringResponseBodyContainer;
+}
+
 class URLFetcher::Core
     : public base::RefCountedThreadSafe<URLFetcher::Core>,
       public net::URLRequest::Delegate {
@@ -124,7 +173,8 @@ class URLFetcher::Core
   scoped_ptr<net::URLRequest> request_;   // The actual request this wraps
   int load_flags_;                   // Flags for the load operation
   int response_code_;                // HTTP status code for the request
-  std::string data_;                 // Results of the request
+  scoped_ptr<ResponseBodyContainerInterface> response_body_;
+                                     // Results of the request
   scoped_refptr<net::IOBuffer> buffer_;
                                      // Read buffer
   scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
@@ -324,7 +374,7 @@ void URLFetcher::Core::OnReadCompleted(net::URLRequest* request,
   do {
     if (!request_->status().is_success() || bytes_read <= 0)
       break;
-    data_.append(buffer_->data(), bytes_read);
+    response_body_->Append(buffer_->data(), bytes_read);
   } while (request_->Read(buffer_, kBufferSize, &bytes_read));
 
   if (request_->status().is_success())
@@ -399,7 +449,7 @@ void URLFetcher::Core::StartURLRequest() {
     request_->SetExtraRequestHeaders(extra_request_headers_);
 
   // There might be data left over from a previous request attempt.
-  data_.clear();
+  response_body_.reset(delegate_->CreateResponseBodyContainer());
 
   request_->Start();
 }
@@ -465,15 +515,15 @@ void URLFetcher::Core::OnCompletedURLRequest(
             FROM_HERE,
             NewRunnableMethod(this, &Core::StartURLRequestWhenAppropriate));
       } else {
-        delegate_->OnURLFetchComplete(fetcher_, url_, status, response_code_,
-                                      cookies_, data_);
+        delegate_->OnURLFetchCompleteNew(fetcher_, url_, status, response_code_,
+                                         cookies_, *response_body_);
       }
     }
   } else {
     if (delegate_) {
       fetcher_->backoff_delay_ = base::TimeDelta();
-      delegate_->OnURLFetchComplete(fetcher_, url_, status, response_code_,
-                                    cookies_, data_);
+      delegate_->OnURLFetchCompleteNew(fetcher_, url_, status, response_code_,
+                                       cookies_, *response_body_);
     }
   }
 }