Clean up URLFetcher unit tests, part 7.

net/url_request/url_fetcher_impl_unittest.cc

Index: net/url_request/url_fetcher_impl_unittest.cc
diff --git a/net/url_request/url_fetcher_impl_unittest.cc b/net/url_request/url_fetcher_impl_unittest.cc
index 121251962b7191ecfddcf663d4f79bbf05d3dfc6..4b26756493f4aa15893b8d0dbcd0ed924f6b0c19 100644
--- a/net/url_request/url_fetcher_impl_unittest.cc
+++ b/net/url_request/url_fetcher_impl_unittest.cc
@@ -260,7 +260,9 @@ class URLFetcherTest : public testing::Test,
 
   // testing::Test:
   void SetUp() override {
-    SetUpServer();
+    test_server_.reset(new SpawnedTestServer(SpawnedTestServer::TYPE_HTTP,
+                                             SpawnedTestServer::kLocalhost,
+                                             base::FilePath(kDocRoot)));
     ASSERT_TRUE(test_server_->Start());
 
     // Set up host resolver so requests for |hanging_url_| block on an async DNS
@@ -290,14 +292,6 @@ class URLFetcherTest : public testing::Test,
 #endif
   }
 
-  // Initializes |test_server_| without starting it.  Allows subclasses to use
-  // their own server configuration.
-  virtual void SetUpServer() {
-    test_server_.reset(new SpawnedTestServer(SpawnedTestServer::TYPE_HTTP,
-                                             SpawnedTestServer::kLocalhost,
-                                             base::FilePath(kDocRoot)));
-  }
-
   // URLFetcher is designed to run on the main UI thread, but in our tests
   // we assume that the current thread is the IO thread where the URLFetcher
   // dispatches its requests to.  When we wish to simulate being used from
@@ -406,21 +400,6 @@ class URLFetcherUploadProgressTest : public URLFetcherTest {
   int64 number_of_chunks_added_;
 };
 
-// Version of URLFetcherTest that tests bad HTTPS requests.
-class URLFetcherBadHTTPSTest : public URLFetcherTest {
- public:
-  URLFetcherBadHTTPSTest();
-
-  // URLFetcherTest:
-  void SetUpServer() override;
-
-  // URLFetcherDelegate:
-  void OnURLFetchComplete(const URLFetcher* source) override;
-
- private:
-  base::FilePath cert_dir_;
-};
-
 // Version of URLFetcherTest that tests request cancellation on shutdown.
 class URLFetcherCancelTest : public URLFetcherTest {
  public:
@@ -506,16 +485,6 @@ class CancelTestURLRequestContextGetter
   GURL throttle_for_url_;
 };
 
-// Version of URLFetcherTest that tests retying the same request twice.
-class URLFetcherMultipleAttemptTest : public URLFetcherTest {
- public:
-  // URLFetcherDelegate:
-  void OnURLFetchComplete(const URLFetcher* source) override;
-
- private:
-  std::string data_;
-};
-
 void URLFetcherDownloadProgressTest::CreateFetcher(const GURL& url) {
   fetcher_ = new URLFetcherImpl(url, URLFetcher::GET, this);
   fetcher_->SetRequestContext(new ThrottlingTestURLRequestContextGetter(
@@ -587,39 +556,6 @@ void URLFetcherUploadProgressTest::OnURLFetchUploadProgress(
   }
 }
 
-URLFetcherBadHTTPSTest::URLFetcherBadHTTPSTest() {
-  PathService::Get(base::DIR_SOURCE_ROOT, &cert_dir_);
-  cert_dir_ = cert_dir_.AppendASCII("chrome");
-  cert_dir_ = cert_dir_.AppendASCII("test");
-  cert_dir_ = cert_dir_.AppendASCII("data");
-  cert_dir_ = cert_dir_.AppendASCII("ssl");
-  cert_dir_ = cert_dir_.AppendASCII("certificates");

[mmenke, 2015/04/16 19:11:40]

Didn't notice this wasn't even being used in my earlier CL that touched this.

-}
-
-void URLFetcherBadHTTPSTest::SetUpServer() {
-  SpawnedTestServer::SSLOptions ssl_options(
-      SpawnedTestServer::SSLOptions::CERT_EXPIRED);
-  test_server_.reset(new SpawnedTestServer(
-      SpawnedTestServer::TYPE_HTTPS, ssl_options, base::FilePath(kDocRoot)));
-}
-
-// The "server certificate expired" error should result in automatic
-// cancellation of the request by
-// URLRequest::Delegate::OnSSLCertificateError.
-void URLFetcherBadHTTPSTest::OnURLFetchComplete(
-    const URLFetcher* source) {
-  // This part is different from URLFetcherTest::OnURLFetchComplete
-  // because this test expects the request to be cancelled.
-  EXPECT_EQ(URLRequestStatus::CANCELED, source->GetStatus().status());
-  EXPECT_EQ(ERR_ABORTED, source->GetStatus().error());
-  EXPECT_EQ(-1, source->GetResponseCode());
-  EXPECT_TRUE(source->GetCookies().empty());
-  std::string data;
-  EXPECT_TRUE(source->GetResponseAsString(&data));
-  EXPECT_TRUE(data.empty());
-  CleanupAfterFetchComplete();
-}
-
 void URLFetcherCancelTest::CreateFetcher(const GURL& url) {
   fetcher_ = new URLFetcherImpl(url, URLFetcher::GET, this);
   CancelTestURLRequestContextGetter* context_getter =
@@ -647,24 +583,6 @@ void URLFetcherCancelTest::CancelRequest() {
   // did not work.
 }
 
-void URLFetcherMultipleAttemptTest::OnURLFetchComplete(
-    const URLFetcher* source) {
-  EXPECT_TRUE(source->GetStatus().is_success());
-  EXPECT_EQ(200, source->GetResponseCode());  // HTTP OK
-  std::string data;
-  EXPECT_TRUE(source->GetResponseAsString(&data));
-  EXPECT_FALSE(data.empty());
-  if (!data.empty() && data_.empty()) {
-    data_ = data;
-    fetcher_->SetRequestContext(new ThrottlingTestURLRequestContextGetter(
-        io_message_loop_proxy().get(), request_context()));
-    fetcher_->Start();
-  } else {
-    EXPECT_EQ(data, data_);
-    CleanupAfterFetchComplete();
-  }
-}
-
 // Create the fetcher on the main thread.  Since network IO will happen on the
 // main thread, this will test URLFetcher's ability to do everything on one
 // thread.
@@ -1127,9 +1045,27 @@ TEST_F(URLFetcherTest, ProtectTestPassedThrough) {
   EXPECT_TRUE(Time::Now() - start_time < TimeDelta::FromMinutes(1));
 }
 
-TEST_F(URLFetcherBadHTTPSTest, BadHTTPSTest) {
-  CreateFetcher(test_server_->GetURL(kDefaultResponsePath));
-  base::MessageLoop::current()->Run();
+// Test the case of requesting a file from an SSL server with an expired cert.
+TEST_F(URLFetcherTest, BadHTTPS) {
+  SpawnedTestServer::SSLOptions ssl_options(
+      SpawnedTestServer::SSLOptions::CERT_EXPIRED);
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS, ssl_options,
+                                base::FilePath(kDocRoot));
+  ASSERT_TRUE(test_server.Start());
+
+  WaitingURLFetcherDelegate delegate;
+  delegate.CreateFetcherWithContext(test_server.GetURL(kDefaultResponsePath),
+                                    URLFetcher::GET, request_context());
+  delegate.StartFetcherAndWait();
+
+  EXPECT_EQ(URLRequestStatus::CANCELED,
+            delegate.fetcher()->GetStatus().status());
+  EXPECT_EQ(ERR_ABORTED, delegate.fetcher()->GetStatus().error());
+  EXPECT_EQ(-1, delegate.fetcher()->GetResponseCode());
+  EXPECT_TRUE(delegate.fetcher()->GetCookies().empty());
+  std::string data;
+  EXPECT_TRUE(delegate.fetcher()->GetResponseAsString(&data));
+  EXPECT_TRUE(data.empty());
 }
 
 TEST_F(URLFetcherCancelTest, ReleasesContext) {
@@ -1183,13 +1119,61 @@ TEST_F(URLFetcherCancelTest, CancelWhileDelayedStartTaskPending) {
   base::MessageLoop::current()->Run();
 }
 
-TEST_F(URLFetcherMultipleAttemptTest, SameData) {
-  // Create the fetcher on the main thread.  Since IO will happen on the main
-  // thread, this will test URLFetcher's ability to do everything on one
-  // thread.
-  CreateFetcher(test_server_->GetURL(kDefaultResponsePath));
+// A URLFetcherDelegate that expects to receive a response body of "request1"
+// and then reuses the fetcher for the same URL, setting the "test" request
+// header to "request2".
+class ReuseFetcherDelegate : public WaitingURLFetcherDelegate {
+ public:
+  // |second_request_context_getter| is the context getter used for the second
+  // request. Can't reuse the old one because fetchers release it on completion.
+  ReuseFetcherDelegate(
+      scoped_refptr<URLRequestContextGetter> second_request_context_getter)
+      : first_request_complete_(false),
+        second_request_context_getter_(second_request_context_getter) {}
 
-  base::MessageLoop::current()->Run();
+  ~ReuseFetcherDelegate() override {}
+
+  void OnURLFetchComplete(const URLFetcher* source) override {
+    EXPECT_EQ(fetcher(), source);
+    if (!first_request_complete_) {
+      first_request_complete_ = true;
+      EXPECT_TRUE(fetcher()->GetStatus().is_success());
+      EXPECT_EQ(200, fetcher()->GetResponseCode());
+      std::string data;
+      ASSERT_TRUE(fetcher()->GetResponseAsString(&data));
+      EXPECT_EQ("request1", data);
+
+      fetcher()->SetRequestContext(second_request_context_getter_.get());
+      fetcher()->SetExtraRequestHeaders("test: request2");
+      fetcher()->Start();
+      return;
+    }
+    WaitingURLFetcherDelegate::OnURLFetchComplete(source);
+  }
+
+ private:
+  bool first_request_complete_;
+  scoped_refptr<URLRequestContextGetter> second_request_context_getter_;
+
+  DISALLOW_COPY_AND_ASSIGN(ReuseFetcherDelegate);
+};
+
+TEST_F(URLFetcherTest, ReuseFetcherForSameURL) {
+  // TODO(mmenke):  It's really weird that this is supported, particularly
+  // some fields can be modified between requests, but some (Like upload body)
+  // cannot be. Can we get rid of support for this?
+  ReuseFetcherDelegate delegate(new TrivialURLRequestContextGetter(
+      request_context(), base::MessageLoopProxy::current()));
+  delegate.CreateFetcherWithContext(test_server_->GetURL("echoheader?test"),
+                                    URLFetcher::GET, request_context());
+  delegate.fetcher()->SetExtraRequestHeaders("test: request1");
+  delegate.StartFetcherAndWait();
+
+  EXPECT_TRUE(delegate.fetcher()->GetStatus().is_success());
+  EXPECT_EQ(200, delegate.fetcher()->GetResponseCode());
+  std::string data;
+  ASSERT_TRUE(delegate.fetcher()->GetResponseAsString(&data));
+  EXPECT_EQ("request2", data);
 }
 
 // Get a small file.