Convert URLFetcher::Delegates to use an interface in content/public/common. Also remove the old U...

content/common/net/url_fetcher.h

 // Copyright (c) 2011 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.
 
 // This file contains URLFetcher, a wrapper around net::URLRequest that handles
 // low-level details like thread safety, ref counting, and incremental buffer
 // reading.  This is useful for callers who simply want to get the data from a
 // URL and don't care about all the nitty-gritty details.
 //
 // NOTE(willchan): Only one "IO" thread is supported for URLFetcher.  This is a
@@ skipping 13 matching lines @@
 #include "base/time.h"
 #include "content/common/content_export.h"
 
 class FilePath;
 class GURL;
 
 namespace base {
 class MessageLoopProxy;
 }  // namespace base
 
+namespace content {
+class URLFetcherDelegate;
+}
+
 namespace net {
 class HostPortPair;
 class HttpResponseHeaders;
 class HttpRequestHeaders;
 class URLRequestContextGetter;
 class URLRequestStatus;
 typedef std::vector<std::string> ResponseCookies;
 }  // namespace net
 
 // To use this class, create an instance with the desired URL and a pointer to
 // the object to be notified when the URL has been loaded:
 //   URLFetcher* fetcher = new URLFetcher("http://www.google.com",
 //                                        URLFetcher::GET, this);
 //
 // Then, optionally set properties on this object, like the request context or
 // extra headers:
 //   fetcher->set_extra_request_headers("X-Foo: bar");
 //
 // 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 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.
+// The object you supply as a delegate must inherit from
+// content::URLFetcherDelegate; 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.
 //
 //
 // NOTE: By default URLFetcher requests are NOT intercepted, except when
 // interception is explicitly enabled in tests.
 
 class CONTENT_EXPORT URLFetcher {
  public:
   enum RequestType {
     GET,
     POST,
     HEAD,
   };
 
   // Imposible http response code. Used to signal that no http response code
   // was received.
   static const int kInvalidHttpResponseCode;
 
-  class CONTENT_EXPORT Delegate {
-   public:
-    // 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);
-
-    // 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() {}
-  };
-
   // URLFetcher::Create uses the currently registered Factory to create the
   // URLFetcher. Factory is intended for testing.
   class Factory {
    public:
     virtual URLFetcher* CreateURLFetcher(int id,
                                          const GURL& url,
                                          RequestType request_type,
-                                         Delegate* d) = 0;
+                                         content::URLFetcherDelegate* d) = 0;
 
    protected:
     virtual ~Factory() {}
   };
 
   // |url| is the URL to send the request to.
   // |request_type| is the type of request to make.
   // |d| the object that will receive the callback on fetch completion.
-  URLFetcher(const GURL& url, RequestType request_type, Delegate* d);
+  URLFetcher(const GURL& url,
+             RequestType request_type,
+             content::URLFetcherDelegate* d);
   virtual ~URLFetcher();
 
   // Normally interception is disabled for URLFetcher, but you can use this
   // to enable it for tests. Also see ScopedURLFetcherFactory for another way
   // of testing code that uses an URLFetcher.
   static void enable_interception_for_tests(bool enabled) {
     g_interception_enabled = enabled;
   }
 
   // Creates a URLFetcher, ownership returns to the caller. If there is no
   // Factory (the default) this creates and returns a new URLFetcher. See the
   // constructor for a description of the args. |id| may be used during testing
   // to identify who is creating the URLFetcher.
   static URLFetcher* Create(int id, const GURL& url, RequestType request_type,
-                            Delegate* d);
+                            content::URLFetcherDelegate* d);
 
   // Sets data only needed by POSTs.  All callers making POST requests should
   // call this before the request is started.  |upload_content_type| is the MIME
   // type of the content, while |upload_content| is the data to be sent (the
   // Content-Length header value will be set to the length of this data).
   void set_upload_data(const std::string& upload_content_type,
                        const std::string& upload_content);
 
   // Indicates that the POST data is sent via chunked transfer encoding.
   // This may only be called before calling Start().
@@ skipping 93 matching lines @@
   // 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;
 
-  // Return a const reference to the string data fetched.  Response type
-  // must be STRING, or this will CHECK.
-  virtual const std::string& GetResponseStringRef() const;

[Sam Kerner (Chrome), 2011/10/24 18:36:25]

Does GetResponseAsString() cause a copy of the string to be made?  It depends on
how std::string is implemented.  If 

std::string a = "long long long long string";
std::string b = a;

Is the string in memory twice?  I ask because some of the items fetched by
URLFetcher are large, and making an extra copy might be an issue.  Passing the
reference avoids this.

[willchan no longer on Chromium, 2011/10/24 18:42:12]

I don't think using URLFetcher for fetching large items is particularly
advisable. It's been done for simplicity's sake, but it's a bit disturbing to me
that we're requiring MB-sized contiguous memory buffers.

That said, I only looked at the url_fetcher changes and neglected to look at the
consumers to see how much of a regression this change would entail. It might
indeed be a blocker for this change.

-
   // 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,
                                      FilePath* out_response_path) const;
 
-  // Cancels all existing URLFetchers.  Will notify the URLFetcher::Delegates.
+  // Cancels all existing URLFetchers.  Will notify the URLFetcherDelegates.
   // 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
   // thread, so that no new URLRequests would be able to start on the IO thread
   // anyway.  This doesn't prevent new URLFetchers from trying to post to the IO
   // thread though, even though the task won't ever run.
   static void CancelAll();
 
  protected:
   // How should the response be stored?
   enum ResponseDestinationType {
     STRING,  // Default: In a std::string
     TEMP_FILE  // Write to a temp file
   };
 
   // Returns the delegate.
-  Delegate* delegate() const;
+  content::URLFetcherDelegate* delegate() const;
 
   // Used by tests.
   const std::string& upload_data() const;
 
   // Used by tests.
   void set_was_fetched_via_proxy(bool flag);
 
   // Used by tests.
   void set_response_headers(scoped_refptr<net::HttpResponseHeaders> headers);
 
@@ skipping 24 matching lines @@
   scoped_refptr<Core> core_;
 
   static Factory* factory_;
 
   static bool g_interception_enabled;
 
   DISALLOW_COPY_AND_ASSIGN(URLFetcher);
 };
 
 #endif  // CONTENT_COMMON_NET_URL_FETCHER_H_