Make test URLFetcher implementations not derive from the URLFetcher implementation, since we want...

content/test/test_url_fetcher_factory.h

Index: content/test/test_url_fetcher_factory.h
===================================================================
--- content/test/test_url_fetcher_factory.h	(revision 107268)
+++ content/test/test_url_fetcher_factory.h	(working copy)
@@ -12,15 +12,16 @@
 #include <utility>
 
 #include "base/threading/non_thread_safe.h"
-#include "content/common/net/url_fetcher.h"
+#include "content/public/common/url_fetcher_factory.h"
 #include "googleurl/src/gurl.h"
+#include "net/http/http_request_headers.h"
 #include "net/url_request/url_request_status.h"
 
 // Changes URLFetcher's Factory for the lifetime of the object.
 // Note that this scoper cannot be nested (to make it even harder to misuse).
 class ScopedURLFetcherFactory : public base::NonThreadSafe {
  public:
-  explicit ScopedURLFetcherFactory(URLFetcher::Factory* factory);
+  explicit ScopedURLFetcherFactory(content::URLFetcherFactory* factory);
   virtual ~ScopedURLFetcherFactory();
 
  private:
@@ -52,7 +53,7 @@
 // might want to use the FakeUrlFetcher and FakeUrlFetcherFactory classes

[wtc, 2011/10/27 21:29:16]

Typo: FakeUrlFetcherFactory => FakeURLFetcherFactory

FakeUrlFetcher may also be a typo.  I found a FakeURLFetcher
class defined in content/test/test_url_fetcher_factory.cc.

[jam, 2011/10/27 22:34:41]

Done.

 // below.
 
-class TestURLFetcher : public URLFetcher {
+class TestURLFetcher : public content::URLFetcher {
  public:
   TestURLFetcher(int id,
                  const GURL& url,
@@ -60,50 +61,75 @@
                  content::URLFetcherDelegate* d);
   virtual ~TestURLFetcher();
 
-  // Overriden to do nothing. It is assumed the caller will notify the delegate.
-  virtual void Start() {}
-
+  // content::URLFetcher implementation
+  virtual void SetUploadData(const std::string& upload_content_type,
+                             const std::string& upload_content) OVERRIDE;
+  virtual void SetChunkedUpload(
+      const std::string& upload_content_type) OVERRIDE;
   // Overriden to cache the chunks uploaded. Caller can read back the uploaded
-  // chunks with the upload_data() accessor.
-  virtual void AppendChunkToUpload(const std::string& data, bool is_last_chunk)
-      OVERRIDE;
+  // chunks with the upload_chunks() accessor.
+  virtual void AppendChunkToUpload(const std::string& data,
+                                   bool is_last_chunk) OVERRIDE;
+  virtual void SetLoadFlags(int load_flags) OVERRIDE;
+  virtual int GetLoadFlags() const OVERRIDE;
+  virtual void SetReferrer(const std::string& referrer) OVERRIDE;
+  virtual void SetExtraRequestHeaders(
+      const std::string& extra_request_headers) OVERRIDE;
+  virtual void GetExtraRequestHeaders(
+      net::HttpRequestHeaders* headers) OVERRIDE;
+  virtual void SetRequestContext(
+      net::URLRequestContextGetter* request_context_getter) OVERRIDE;
+  virtual void SetAutomaticallyRetryOn5xx(bool retry) OVERRIDE;
+  virtual void SetMaxRetries(int max_retries) OVERRIDE;
+  virtual int GetMaxRetries() const OVERRIDE;
+  virtual base::TimeDelta GetBackoffDelay() const OVERRIDE;
+  virtual void SaveResponseToTemporaryFile(
+      scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy) OVERRIDE;
+  virtual net::HttpResponseHeaders* GetResponseHeaders() const OVERRIDE;
+  virtual net::HostPortPair GetSocketAddress() const OVERRIDE;
+  virtual bool WasFetchedViaProxy() const OVERRIDE;
+  virtual void Start() OVERRIDE;
+  virtual void StartWithRequestContextGetter(
+      net::URLRequestContextGetter* request_context_getter) OVERRIDE;
 
-  // Unique ID in our factory.
-  int id() const { return id_; }
-
   // URL we were created with. Because of how we're using URLFetcher GetUrl()
   // always returns an empty URL. Chances are you'll want to use
   // GetOriginalUrl() in your tests.
   virtual const GURL& GetOriginalUrl() const OVERRIDE;
+  virtual const GURL& GetUrl() const OVERRIDE;
+  virtual const net::URLRequestStatus& GetStatus() const OVERRIDE;
+  virtual int GetResponseCode() const OVERRIDE;
+  virtual const net::ResponseCookies& GetCookies() const OVERRIDE;
+  virtual bool FileErrorOccurred(
+      base::PlatformFileError* out_error_code) const OVERRIDE;
+  virtual void ReceivedContentWasMalformed() OVERRIDE;
+  // Override response access functions to return fake data.
+  virtual bool GetResponseAsString(
+      std::string* out_response_string) const OVERRIDE;
+  virtual bool GetResponseAsFilePath(
+      bool take_ownership, FilePath* out_response_path) const OVERRIDE;
 
+  // Unique ID in our factory.
+  int id() const { return id_; }
+
   // Returns the data uploaded on this URLFetcher.
-  const std::string& upload_data() const { return URLFetcher::upload_data(); }
+  const std::string& upload_data() const { return upload_data_; }
 
   // Returns the chunks of data uploaded on this URLFetcher.
   const std::list<std::string>& upload_chunks() const { return chunks_; }
 
   // Returns the delegate installed on the URLFetcher.
-  content::URLFetcherDelegate* delegate() const {
-    return URLFetcher::delegate();
-  }
+  content::URLFetcherDelegate* delegate() const { return delegate_; }
 
   void set_url(const GURL& url) { fake_url_ = url; }
-  virtual const GURL& GetUrl() const OVERRIDE;
-
   void set_status(const net::URLRequestStatus& status);
-  virtual const net::URLRequestStatus& GetStatus() const OVERRIDE;
-
   void set_response_code(int response_code) {
     fake_response_code_ = response_code;
   }
-  virtual int GetResponseCode() const OVERRIDE;
-
   void set_cookies(const net::ResponseCookies& c) { fake_cookies_ = c; }
-  virtual const net::ResponseCookies& GetCookies() const OVERRIDE;
-
   void set_was_fetched_via_proxy(bool flag);
-
   void set_response_headers(scoped_refptr<net::HttpResponseHeaders> headers);
+  void set_backoff_delay(base::TimeDelta backoff_delay);
 
   // Set string data.
   void SetResponseString(const std::string& response);
@@ -111,16 +137,16 @@
   // Set File data.
   void SetResponseFilePath(const FilePath& path);
 
-  // Override response access functions to return fake data.
-  virtual bool GetResponseAsString(std::string* out_response_string) const
-      OVERRIDE;
-  virtual bool GetResponseAsFilePath(bool take_ownership,
-                                     FilePath* out_response_path) const
-      OVERRIDE;
+ private:
+  enum ResponseDestinationType {
+    STRING,  // Default: In a std::string
+    TEMP_FILE  // Write to a temp file
+  };
 
- private:
   const int id_;
   const GURL original_url_;
+  content::URLFetcherDelegate* delegate_;
+  std::string upload_data_;
   std::list<std::string> chunks_;
   bool did_receive_last_chunk_;
 
@@ -128,28 +154,36 @@
   // Setting the real values is not possible, because the real class
   // has no setters. The data is a private member of a class defined
   // in a .cc file, so we can't get at it with friendship.
+  int fake_load_flags_;
   GURL fake_url_;
   net::URLRequestStatus fake_status_;
   int fake_response_code_;
   net::ResponseCookies fake_cookies_;
+  ResponseDestinationType fake_response_destination_;
   std::string fake_response_string_;
   FilePath fake_response_file_path_;
+  bool fake_was_fetched_via_proxy_;
+  scoped_refptr<net::HttpResponseHeaders> fake_response_headers_;
+  net::HttpRequestHeaders fake_extra_request_headers_;
+  int fake_max_retries_;
+  base::TimeDelta fake_backoff_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(TestURLFetcher);
 };
 
 // Simple URLFetcher::Factory method that creates TestURLFetchers. All fetchers
 // are registered in a map by the id passed to the create method.
-class TestURLFetcherFactory : public URLFetcher::Factory,
+class TestURLFetcherFactory : public content::URLFetcherFactory,
                               public ScopedURLFetcherFactory {
  public:
   TestURLFetcherFactory();
   virtual ~TestURLFetcherFactory();
 
-  virtual URLFetcher* CreateURLFetcher(int id,
-                                       const GURL& url,
-                                       URLFetcher::RequestType request_type,
-                                       content::URLFetcherDelegate* d) OVERRIDE;
+  virtual content::URLFetcher* CreateURLFetcher(
+      int id,
+      const GURL& url,
+      content::URLFetcher::RequestType request_type,
+      content::URLFetcherDelegate* d) OVERRIDE;
   TestURLFetcher* GetFetcherByID(int id) const;
   void RemoveFetcherFromMap(int id);
 
@@ -193,13 +227,13 @@
 //  SomeService service;
 //  service.Run();  // Will eventually request these two URLs.
 
-class FakeURLFetcherFactory : public URLFetcher::Factory,
+class FakeURLFetcherFactory : public content::URLFetcherFactory,
                               public ScopedURLFetcherFactory {
  public:
   FakeURLFetcherFactory();
   // FakeURLFetcherFactory that will delegate creating URLFetcher for unknown
   // url to the given factory.
-  explicit FakeURLFetcherFactory(URLFetcher::Factory* default_factory);
+  explicit FakeURLFetcherFactory(content::URLFetcherFactory* default_factory);
   virtual ~FakeURLFetcherFactory();
 
   // If no fake response is set for the given URL this method will delegate the
@@ -207,10 +241,11 @@
   // NULL.
   // Otherwise, it will return a URLFetcher object which will respond with the
   // pre-baked response that the client has set by calling SetFakeResponse().
-  virtual URLFetcher* CreateURLFetcher(int id,
-                                       const GURL& url,
-                                       URLFetcher::RequestType request_type,
-                                       content::URLFetcherDelegate* d) OVERRIDE;
+  virtual content::URLFetcher* CreateURLFetcher(
+      int id,
+      const GURL& url,
+      content::URLFetcher::RequestType request_type,
+      content::URLFetcherDelegate* d) OVERRIDE;
 
   // Sets the fake response for a given URL.  If success is true we will serve
   // an HTTP/200 and an HTTP/500 otherwise.  The |response_data| may be empty.
@@ -225,7 +260,7 @@
  private:
   typedef std::map<GURL, std::pair<std::string, bool> > FakeResponseMap;
   FakeResponseMap fake_responses_;
-  URLFetcher::Factory* default_factory_;
+  content::URLFetcherFactory* default_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeURLFetcherFactory);
 };
@@ -234,16 +269,17 @@
 // URLFetcher. It can be use in conjunction with a FakeURLFetcherFactory in
 // integration tests to control the behavior of some requests but execute
 // all the other ones.
-class URLFetcherFactory : public URLFetcher::Factory {
+class URLFetcherFactory : public content::URLFetcherFactory {

[wtc, 2011/10/27 21:29:16]

Although the content namespace disambiguates the two
URLFetcherFactory classes, this seems like a bad idea.
Can we avoid using the same name?

One way to solve this problem is to turn content::URLFetcherFactory
back into a nested type content::URLFetcher::Factory.

[jam, 2011/10/27 22:34:41]

sure, good point. i've renamed it to URLFetcherImplFactory to make what it does
obvious 
the new convention, afaik, is to avoid nested classes, that's why i split it
off.

  public:
   URLFetcherFactory();
   virtual ~URLFetcherFactory();
 
   // This method will create a real URLFetcher.
-  virtual URLFetcher* CreateURLFetcher(int id,
-                                       const GURL& url,
-                                       URLFetcher::RequestType request_type,
-                                       content::URLFetcherDelegate* d) OVERRIDE;
+  virtual content::URLFetcher* CreateURLFetcher(
+      int id,
+      const GURL& url,
+      content::URLFetcher::RequestType request_type,
+      content::URLFetcherDelegate* d) OVERRIDE;
 
 };