GTTF: test server cleanup:

- simplify the public interface
- remove unneeded methods
- make it easier to understand

TEST=none
BUG=49680

Review URL: http://codereview.chromium.org/2881028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@53509 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 124 insertions, 297 deletions

diff --git a/net/base/net_test_constants.h b/net/base/net_test_constants.h
deleted file mode 100644
index 83eb43f..0000000
--- a/net/base/net_test_constants.h
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright (c) 2009 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.
-
-#ifndef NET_BASE_NET_TEST_CONSTANTS_H_
-#define NET_BASE_NET_TEST_CONSTANTS_H_
-
-namespace net {
-
-// Number of connection attempts for tests.
-const int kDefaultTestConnectionAttempts = 10;
-
-// Connection timeout in milliseconds for tests.
-const int kDefaultTestConnectionTimeout = 1000;
-
-}  // namespace net
-
-#endif  // NET_BASE_NET_TEST_CONSTANTS_H_
diff --git a/net/net.gyp b/net/net.gyp
index c89d8ed..cde670a 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -679,7 +679,6 @@
         'base/mime_sniffer_unittest.cc',
         'base/mime_util_unittest.cc',
         'base/net_log_unittest.h',
-        'base/net_test_constants.h',
         'base/net_test_suite.h',
         'base/net_util_unittest.cc',
         'base/pem_tokenizer_unittest.cc',
diff --git a/net/proxy/proxy_resolver_perftest.cc b/net/proxy/proxy_resolver_perftest.cc
index 0c57955..c7f9c7a 100644
--- a/net/proxy/proxy_resolver_perftest.cc
+++ b/net/proxy/proxy_resolver_perftest.cc
@@ -145,7 +145,7 @@ class PacPerfSuiteRunner {
     DCHECK(!resolver_->expects_pac_bytes());
     if (!server_) {
       server_ = HTTPTestServer::CreateServer(
-          L"net/data/proxy_resolver_perftest", NULL);
+          L"net/data/proxy_resolver_perftest");
     }
     ASSERT_TRUE(server_.get() != NULL);
   }
diff --git a/net/proxy/proxy_script_fetcher_unittest.cc b/net/proxy/proxy_script_fetcher_unittest.cc
index e0e64c9..d96da03 100644
--- a/net/proxy/proxy_script_fetcher_unittest.cc
+++ b/net/proxy/proxy_script_fetcher_unittest.cc
@@ -93,8 +93,7 @@ TEST_F(ProxyScriptFetcherTest, FileUrl) {
 // Note that all mime types are allowed for PAC file, to be consistent
 // with other browsers.
 TEST_F(ProxyScriptFetcherTest, HttpMimeType) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
@@ -130,8 +129,7 @@ TEST_F(ProxyScriptFetcherTest, HttpMimeType) {
 }
 
 TEST_F(ProxyScriptFetcherTest, HttpStatusCode) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
@@ -158,8 +156,7 @@ TEST_F(ProxyScriptFetcherTest, HttpStatusCode) {
 }
 
 TEST_F(ProxyScriptFetcherTest, ContentDisposition) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
@@ -177,8 +174,7 @@ TEST_F(ProxyScriptFetcherTest, ContentDisposition) {
 }
 
 TEST_F(ProxyScriptFetcherTest, NoCache) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
@@ -212,8 +208,7 @@ TEST_F(ProxyScriptFetcherTest, NoCache) {
 }
 
 TEST_F(ProxyScriptFetcherTest, TooLarge) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
@@ -255,8 +250,7 @@ TEST_F(ProxyScriptFetcherTest, TooLarge) {
 }
 
 TEST_F(ProxyScriptFetcherTest, Hang) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
@@ -295,8 +289,7 @@ TEST_F(ProxyScriptFetcherTest, Hang) {
 // (like gzip, bzip, etc.), and apply any charset conversions to yield
 // UTF8.
 TEST_F(ProxyScriptFetcherTest, Encodings) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(kDocRoot));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new RequestContext;
   scoped_ptr<ProxyScriptFetcher> pac_fetcher(
diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index 4abed54..ff4d4dc 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -24,7 +24,6 @@
 #include "base/utf_string_conversions.h"
 #include "net/base/cert_test_util.h"
 #include "net/base/host_resolver.h"
-#include "net/base/net_test_constants.h"
 #include "net/base/test_completion_callback.h"
 #include "net/socket/tcp_client_socket.h"
 #include "net/socket/tcp_pinger.h"
@@ -34,6 +33,16 @@
 #pragma comment(lib, "crypt32.lib")
 #endif
 
+namespace {
+
+// Number of connection attempts for tests.
+const int kServerConnectionAttempts = 10;
+
+// Connection timeout in milliseconds for tests.
+const int kServerConnectionTimeoutMs = 1000;
+
+}  // namespace
+
 namespace net {
 
 #if defined(OS_MACOSX)
@@ -49,22 +58,8 @@ const int TestServerLauncher::kBadHTTPSPort = 9666;
 // The issuer name of the cert that should be trusted for the test to work.
 const wchar_t TestServerLauncher::kCertIssuerName[] = L"Test CA";
 
-TestServerLauncher::TestServerLauncher() : process_handle_(
-    base::kNullProcessHandle),
-    forking_(false),
-    connection_attempts_(kDefaultTestConnectionAttempts),
-    connection_timeout_(kDefaultTestConnectionTimeout)
-{
-  InitCertPath();
-}
-
-TestServerLauncher::TestServerLauncher(int connection_attempts,
-                                       int connection_timeout)
-                        : process_handle_(base::kNullProcessHandle),
-                          forking_(false),
-                          connection_attempts_(connection_attempts),
-                          connection_timeout_(connection_timeout)
-{
+TestServerLauncher::TestServerLauncher()
+    : process_handle_(base::kNullProcessHandle) {
   InitCertPath();
 }
 
@@ -157,15 +152,16 @@ bool TestServerLauncher::Start(Protocol protocol,
 
 #if defined(OS_WIN)
   // Get path to python interpreter
-  if (!PathService::Get(base::DIR_SOURCE_ROOT, &python_runtime_))
+  FilePath python_exe;
+  if (!PathService::Get(base::DIR_SOURCE_ROOT, &python_exe))
     return false;
-  python_runtime_ = python_runtime_
+  python_exe = python_exe
       .Append(FILE_PATH_LITERAL("third_party"))
       .Append(FILE_PATH_LITERAL("python_24"))
       .Append(FILE_PATH_LITERAL("python.exe"));
 
   std::wstring command_line =
-      L"\"" + python_runtime_.ToWStringHack() + L"\" " +
+      L"\"" + python_exe.ToWStringHack() + L"\" " +
       L"\"" + testserver_path.ToWStringHack() +
       L"\" --port=" + UTF8ToWide(port_str) +
       L" --data-dir=\"" + document_root_dir_.ToWStringHack() + L"\"";
@@ -181,8 +177,6 @@ bool TestServerLauncher::Start(Protocol protocol,
     command_line.append(file_root_url);
     command_line.append(L"\"");
   }
-  // Deliberately do not pass the --forking flag. It breaks the tests
-  // on Windows.
 
   if (!LaunchTestServerAsJob(command_line,
                              true,
@@ -201,8 +195,6 @@ bool TestServerLauncher::Start(Protocol protocol,
     command_line.push_back("-f");
   if (!cert_path.value().empty())
     command_line.push_back("--https=" + cert_path.value());
-  if (forking_)
-    command_line.push_back("--forking");
 
   base::file_handle_mapping_vector no_mappings;
   LOG(INFO) << "Trying to launch " << command_line[0] << " ...";
@@ -237,8 +229,9 @@ bool TestServerLauncher::WaitToStart(const std::string& host_name, int port) {
     return false;
 
   net::TCPPinger pinger(addr);
-  rv = pinger.Ping(base::TimeDelta::FromMilliseconds(connection_timeout_),
-                   connection_attempts_);
+  rv = pinger.Ping(
+      base::TimeDelta::FromMilliseconds(kServerConnectionTimeoutMs),
+      kServerConnectionAttempts);
   return rv == net::OK;
 }
 
diff --git a/net/test/test_server.h b/net/test/test_server.h
index 25f2c8b..39cae12 100644
--- a/net/test/test_server.h
+++ b/net/test/test_server.h
@@ -9,6 +9,7 @@
 
 #include <string>
 
+#include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/process_util.h"
 
@@ -32,8 +33,6 @@ namespace net {
 class TestServerLauncher {
  public:
   TestServerLauncher();
-  TestServerLauncher(int connection_attempts, int connection_timeout);
-
   virtual ~TestServerLauncher();
 
   enum Protocol {
@@ -41,11 +40,7 @@ class TestServerLauncher {
   };
 
   // Load the test root cert, if it hasn't been loaded yet.
-  bool LoadTestRootCert();
-
-  // Tells the server to enable/disable servicing each request
-  // in a separate process. Takes effect only if called before Start.
-  void set_forking(bool forking) { forking_ = forking; }
+  bool LoadTestRootCert() WARN_UNUSED_RESULT;
 
   // Start src/net/tools/testserver/testserver.py and
   // ask it to serve the given protocol.
@@ -58,7 +53,7 @@ class TestServerLauncher {
              const std::string& host_name, int port,
              const FilePath& document_root,
              const FilePath& cert_path,
-             const std::wstring& file_root_url);
+             const std::wstring& file_root_url) WARN_UNUSED_RESULT;
 
   // Stop the server started by Start().
   bool Stop();
@@ -67,7 +62,7 @@ class TestServerLauncher {
   // without a call to Stop().
   // WaitToFinish is handy in that case.
   // It returns true if the server exited cleanly.
-  bool WaitToFinish(int milliseconds);
+  bool WaitToFinish(int milliseconds) WARN_UNUSED_RESULT;
 
   // Paths to a good, an expired, and an invalid server certificate
   // (use as arguments to Start()).
@@ -94,7 +89,7 @@ class TestServerLauncher {
  private:
   // Wait a while for the server to start, return whether
   // we were able to make a connection to it.
-  bool WaitToStart(const std::string& host_name, int port);
+  bool WaitToStart(const std::string& host_name, int port) WARN_UNUSED_RESULT;
 
   // Append to PYTHONPATH so Python can find pyftpdlib and tlslite.
   void SetPythonPath();
@@ -103,7 +98,7 @@ class TestServerLauncher {
   FilePath GetRootCertPath();
 
   // Returns false if our test root certificate is not trusted.
-  bool CheckCATrusted();
+  bool CheckCATrusted() WARN_UNUSED_RESULT;
 
   // Initilize the certificate path.
   void InitCertPath();
@@ -112,8 +107,6 @@ class TestServerLauncher {
 
   FilePath cert_dir_;
 
-  FilePath python_runtime_;
-
   base::ProcessHandle process_handle_;
 
 #if defined(OS_WIN)
@@ -121,13 +114,6 @@ class TestServerLauncher {
   ScopedHandle job_handle_;
 #endif
 
-  // True if the server should handle each request in a separate process.
-  bool forking_;
-
-  // Number of tries and timeout for each try used for WaitToStart.
-  int connection_attempts_;
-  int connection_timeout_;
-
 #if defined(USE_NSS)
   scoped_refptr<X509Certificate> cert_;
 #endif
diff --git a/net/tools/testserver/run_testserver.cc b/net/tools/testserver/run_testserver.cc
index 716e320..dc16a16 100644
--- a/net/tools/testserver/run_testserver.cc
+++ b/net/tools/testserver/run_testserver.cc
@@ -55,7 +55,7 @@ int main(int argc, const char* argv[]) {
   } else if (protocol == "ftp") {
     test_server = FTPTestServer::CreateServer(doc_root);
   } else if (protocol == "http") {
-    test_server = HTTPTestServer::CreateServer(doc_root, NULL);
+    test_server = HTTPTestServer::CreateServer(doc_root);
   } else {
     NOTREACHED();
   }
diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py
index 8e3df5e..999cde7 100644
--- a/net/tools/testserver/testserver.py
+++ b/net/tools/testserver/testserver.py
@@ -82,16 +82,6 @@ class HTTPSServer(tlslite.api.TLSSocketServerMixIn, StoppableHTTPServer):
       print "Handshake failure:", str(error)
       return False
 
-class ForkingHTTPServer(SocketServer.ForkingMixIn, StoppableHTTPServer):
-  """This is a specialization of of StoppableHTTPServer which serves each
-  request in a separate process"""
-  pass
-
-class ForkingHTTPSServer(SocketServer.ForkingMixIn, HTTPSServer):
-  """This is a specialization of of HTTPSServer which serves each
-  request in a separate process"""
-  pass
-
 class TestPageHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
   def __init__(self, request, client_address, socket_server):
@@ -1200,18 +1190,10 @@ def main(options, args):
       if not os.path.isfile(options.cert):
         print 'specified cert file not found: ' + options.cert + ' exiting...'
         return
-      if options.forking:
-        server_class = ForkingHTTPSServer
-      else:
-        server_class = HTTPSServer
-      server = server_class(('127.0.0.1', port), TestPageHandler, options.cert)
+      server = HTTPSServer(('127.0.0.1', port), TestPageHandler, options.cert)
       print 'HTTPS server started on port %d...' % port
     else:
-      if options.forking:
-        server_class = ForkingHTTPServer
-      else:
-        server_class = StoppableHTTPServer
-      server = server_class(('127.0.0.1', port), TestPageHandler)
+      server = StoppableHTTPServer(('127.0.0.1', port), TestPageHandler)
       print 'HTTP server started on port %d...' % port
 
     server.data_dir = MakeDataDir()
@@ -1263,9 +1245,6 @@ if __name__ == '__main__':
                            const=SERVER_FTP, default=SERVER_HTTP,
                            dest='server_type',
                            help='FTP or HTTP server: default is HTTP.')
-  option_parser.add_option('--forking', action='store_true', default=False,
-                           dest='forking',
-                           help='Serve each request in a separate process.')
   option_parser.add_option('', '--port', default='8888', type='int',
                            help='Port used by the server.')
   option_parser.add_option('', '--data-dir', dest='data_dir',
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index bd4e56a..8c842fe 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -96,8 +96,7 @@ class URLRequestTest : public PlatformTest {
 class URLRequestTestHTTP : public URLRequestTest {
  protected:
   static void SetUpTestCase() {
-    server_ = HTTPTestServer::CreateForkingServer(
-        L"net/data/url_request_unittest/");
+    server_ = HTTPTestServer::CreateServer(L"net/data/url_request_unittest/");
   }
 
   static void TearDownTestCase() {
@@ -262,13 +261,81 @@ TEST_F(URLRequestTestHTTP, HTTPSToHTTPRedirectNoRefererTest) {
   EXPECT_EQ(std::string(), req.referrer());
 }
 
+namespace {
+
+// Used by MakeGETRequest to implement sync load behavior.
+class SyncTestDelegate : public TestDelegate {
+ public:
+  SyncTestDelegate() : event_(false, false), success_(false) {
+  }
+  virtual void OnResponseCompleted(URLRequest* request) {
+    MessageLoop::current()->DeleteSoon(FROM_HERE, request);
+    success_ = request->status().is_success();
+    event_.Signal();
+  }
+  bool Wait(int64 secs) {
+    return event_.TimedWait(TimeDelta::FromSeconds(secs));
+  }
+  bool did_succeed() const { return success_; }
+ private:
+  base::WaitableEvent event_;
+  bool success_;
+  DISALLOW_COPY_AND_ASSIGN(SyncTestDelegate);
+};
+
+void StartGETRequest(const GURL& url, URLRequest::Delegate* delegate) {
+  URLRequest* request = new URLRequest(url, delegate);
+  request->set_context(new TestURLRequestContext());
+  request->set_method("GET");
+  request->Start();
+  EXPECT_TRUE(request->is_pending());
+}
+
+bool MakeGETRequest(const GURL& url) {
+  // Spin up a background thread for this request so that we have access to
+  // an IO message loop, and in cases where this thread already has an IO
+  // message loop, we also want to avoid spinning a nested message loop.
+  SyncTestDelegate d;
+  {
+    base::Thread io_thread("MakeGETRequest");
+    base::Thread::Options options;
+    options.message_loop_type = MessageLoop::TYPE_IO;
+    io_thread.StartWithOptions(options);
+    io_thread.message_loop()->PostTask(FROM_HERE, NewRunnableFunction(
+        &StartGETRequest, url, &d));
+
+    const int kWaitSeconds = 30;
+    if (!d.Wait(kWaitSeconds))
+      return false;
+  }
+  return d.did_succeed();
+}
+
+}  // namespace
+
+// Some tests use browser javascript to fetch a 'kill' url that causes
+// the server to exit by itself (rather than letting TestServerLauncher's
+// destructor kill it). We now unit test this mechanism.
 TEST_F(URLRequestTest, QuitTest) {
-  // Don't use shared server here because we order it to quit.
-  // It would impact other tests.
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
-  server->SendQuit();
+  // Append the time to avoid problems where the kill page
+  // is being cached rather than being executed on the server
+  std::string page_name = StringPrintf("kill?%u",
+      static_cast<int>(base::Time::Now().ToInternalValue()));
+  int retry_count = 5;
+  while (retry_count > 0) {
+    bool r = MakeGETRequest(server->TestServerPage(page_name));
+    // BUG #1048625 causes the kill GET to fail.  For now we just retry.
+    // Once the bug is fixed, we should remove the while loop and put back
+    // the following DCHECK.
+    // DCHECK(r);
+    if (r)
+      break;
+    retry_count--;
+  }
+  // Make sure we were successful in stopping the testserver.
+  EXPECT_LT(0, retry_count);
   EXPECT_TRUE(server->WaitToFinish(20000));
 }
 
@@ -1189,8 +1256,7 @@ TEST_F(URLRequestTestHTTP, BasicAuthWithCookies) {
 }
 
 TEST_F(URLRequestTest, DoNotSendCookies) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new TestURLRequestContext();
 
@@ -1238,8 +1304,7 @@ TEST_F(URLRequestTest, DoNotSendCookies) {
 }
 
 TEST_F(URLRequestTest, DoNotSaveCookies) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<URLRequestContext> context = new TestURLRequestContext();
 
@@ -1294,8 +1359,7 @@ TEST_F(URLRequestTest, DoNotSaveCookies) {
 }
 
 TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1348,8 +1412,7 @@ TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy) {
 }
 
 TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1405,8 +1468,7 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy) {
 }
 
 TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy_Async) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1460,8 +1522,7 @@ TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy_Async) {
 }
 
 TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1517,8 +1578,7 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) {
 }
 
 TEST_F(URLRequestTest, CancelTest_During_CookiePolicy) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1548,8 +1608,7 @@ TEST_F(URLRequestTest, CancelTest_During_CookiePolicy) {
 }
 
 TEST_F(URLRequestTest, CancelTest_During_OnGetCookies) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1577,8 +1636,7 @@ TEST_F(URLRequestTest, CancelTest_During_OnGetCookies) {
 }
 
 TEST_F(URLRequestTest, CancelTest_During_OnSetCookie) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
@@ -1611,8 +1669,7 @@ TEST_F(URLRequestTest, CancelTest_During_OnSetCookie) {
 }
 
 TEST_F(URLRequestTest, CookiePolicy_ForceSession) {
-  scoped_refptr<HTTPTestServer> server =
-      HTTPTestServer::CreateServer(L"", NULL);
+  scoped_refptr<HTTPTestServer> server(HTTPTestServer::CreateServer(L""));
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<TestURLRequestContext> context = new TestURLRequestContext();
 
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index 8f090ef..969e034 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -26,7 +26,6 @@
 #include "net/base/host_resolver.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
-#include "net/base/net_test_constants.h"
 #include "net/base/ssl_config_service_defaults.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/ftp/ftp_network_layer.h"
@@ -402,15 +401,8 @@ class TestDelegate : public URLRequest::Delegate {
 class BaseTestServer : public base::RefCounted<BaseTestServer> {
  protected:
   BaseTestServer() {}
-  BaseTestServer(int connection_attempts, int connection_timeout)
-      : launcher_(connection_attempts, connection_timeout) {}
 
  public:
-  void set_forking(bool forking) {
-    launcher_.set_forking(forking);
-  }
-
-  // Used with e.g. HTTPTestServer::SendQuit()
   bool WaitToFinish(int milliseconds) {
     return launcher_.WaitToFinish(milliseconds);
   }
@@ -442,8 +434,6 @@ class BaseTestServer : public base::RefCounted<BaseTestServer> {
                 "@" + host_name_ + ":" + port_str_ + "/" + path);
   }
 
-  virtual bool MakeGETRequest(const std::string& page_name) = 0;
-
   FilePath GetDataDirectory() {
     return launcher_.GetDocumentRootPath();
   }
@@ -473,29 +463,6 @@ class BaseTestServer : public base::RefCounted<BaseTestServer> {
     return true;
   }
 
-  // Used by MakeGETRequest to implement sync load behavior.
-  class SyncTestDelegate : public TestDelegate {
-   public:
-    SyncTestDelegate() : event_(false, false), success_(false) {
-    }
-    virtual void OnResponseCompleted(URLRequest* request) {
-      MessageLoop::current()->DeleteSoon(FROM_HERE, request);
-      success_ = request->status().is_success();
-      event_.Signal();
-    }
-    bool Wait(int64 secs) {
-      TimeDelta td = TimeDelta::FromSeconds(secs);
-      if (event_.TimedWait(td))
-        return true;
-      return false;
-    }
-    bool did_succeed() const { return success_; }
-   private:
-    base::WaitableEvent event_;
-    bool success_;
-    DISALLOW_COPY_AND_ASSIGN(SyncTestDelegate);
-  };
-
   net::TestServerLauncher launcher_;
   std::string scheme_;
   std::string host_name_;
@@ -507,65 +474,20 @@ class BaseTestServer : public base::RefCounted<BaseTestServer> {
 // HTTP
 class HTTPTestServer : public BaseTestServer {
  protected:
-  explicit HTTPTestServer() : loop_(NULL) {
-  }
-
-  explicit HTTPTestServer(int connection_attempts, int connection_timeout)
-      : BaseTestServer(connection_attempts, connection_timeout), loop_(NULL) {
-  }
-
-  virtual ~HTTPTestServer() {}
+  HTTPTestServer() {}
 
  public:
   // Creates and returns a new HTTPTestServer. If |loop| is non-null, requests
   // are serviced on it, otherwise a new thread and message loop are created.
   static scoped_refptr<HTTPTestServer> CreateServer(
-      const std::wstring& document_root,
-      MessageLoop* loop) {
-    return CreateServerWithFileRootURL(document_root, std::wstring(), loop);
-  }
-
-  static scoped_refptr<HTTPTestServer> CreateServer(
-      const std::wstring& document_root,
-      MessageLoop* loop,
-      int connection_attempts,
-      int connection_timeout) {
-    return CreateServerWithFileRootURL(document_root, std::wstring(), loop,
-                                       connection_attempts,
-                                       connection_timeout);
-  }
-
-  static scoped_refptr<HTTPTestServer> CreateServerWithFileRootURL(
-      const std::wstring& document_root,
-      const std::wstring& file_root_url,
-      MessageLoop* loop) {
-    return CreateServerWithFileRootURL(document_root, file_root_url, loop,
-                                       net::kDefaultTestConnectionAttempts,
-                                       net::kDefaultTestConnectionTimeout);
-  }
-
-  static scoped_refptr<HTTPTestServer> CreateForkingServer(
       const std::wstring& document_root) {
-    scoped_refptr<HTTPTestServer> test_server =
-        new HTTPTestServer(net::kDefaultTestConnectionAttempts,
-                           net::kDefaultTestConnectionTimeout);
-    test_server->set_forking(true);
-    FilePath no_cert;
-    FilePath docroot = FilePath::FromWStringHack(document_root);
-    if (!StartTestServer(test_server.get(), docroot, no_cert, std::wstring()))
-      return NULL;
-    return test_server;
+    return CreateServerWithFileRootURL(document_root, std::wstring());
   }
 
   static scoped_refptr<HTTPTestServer> CreateServerWithFileRootURL(
       const std::wstring& document_root,
-      const std::wstring& file_root_url,
-      MessageLoop* loop,
-      int connection_attempts,
-      int connection_timeout) {
-    scoped_refptr<HTTPTestServer> test_server =
-        new HTTPTestServer(connection_attempts, connection_timeout);
-    test_server->loop_ = loop;
+      const std::wstring& file_root_url) {
+    scoped_refptr<HTTPTestServer> test_server(new HTTPTestServer());
     FilePath no_cert;
     FilePath docroot = FilePath::FromWStringHack(document_root);
     if (!StartTestServer(test_server.get(), docroot, no_cert, file_root_url))
@@ -582,75 +504,7 @@ class HTTPTestServer : public BaseTestServer {
                          file_root_url);
   }
 
-  // A subclass may wish to send the request in a different manner
-  virtual bool MakeGETRequest(const std::string& page_name) {
-    const GURL& url = TestServerPage(page_name);
-
-    // Spin up a background thread for this request so that we have access to
-    // an IO message loop, and in cases where this thread already has an IO
-    // message loop, we also want to avoid spinning a nested message loop.
-    SyncTestDelegate d;
-    {
-      MessageLoop* loop = loop_;
-      scoped_ptr<base::Thread> io_thread;
-
-      if (!loop) {
-        io_thread.reset(new base::Thread("MakeGETRequest"));
-        base::Thread::Options options;
-        options.message_loop_type = MessageLoop::TYPE_IO;
-        io_thread->StartWithOptions(options);
-        loop = io_thread->message_loop();
-      }
-      loop->PostTask(FROM_HERE, NewRunnableFunction(
-            &HTTPTestServer::StartGETRequest, url, &d));
-
-      // Build bot wait for only 300 seconds we should ensure wait do not take
-      // more than 300 seconds
-      if (!d.Wait(250))
-        return false;
-    }
-    return d.did_succeed();
-  }
-
-  static void StartGETRequest(const GURL& url, URLRequest::Delegate* delegate) {
-    URLRequest* request = new URLRequest(url, delegate);
-    request->set_context(new TestURLRequestContext());
-    request->set_method("GET");
-    request->Start();
-    EXPECT_TRUE(request->is_pending());
-  }
-
-  // Some tests use browser javascript to fetch a 'kill' url that causes
-  // the server to exit by itself (rather than letting TestServerLauncher's
-  // destructor kill it).
-  // This method does the same thing so we can unit test that mechanism.
-  // You can then use WaitToFinish() to sleep until the server terminates.
-  void SendQuit() {
-    // Append the time to avoid problems where the kill page
-    // is being cached rather than being executed on the server
-    std::string page_name = StringPrintf("kill?%u",
-        static_cast<int>(base::Time::Now().ToInternalValue()));
-    int retry_count = 5;
-    while (retry_count > 0) {
-      bool r = MakeGETRequest(page_name);
-      // BUG #1048625 causes the kill GET to fail.  For now we just retry.
-      // Once the bug is fixed, we should remove the while loop and put back
-      // the following DCHECK.
-      // DCHECK(r);
-      if (r)
-        break;
-      retry_count--;
-    }
-    // Make sure we were successful in stopping the testserver.
-    DCHECK_GT(retry_count, 0);
-  }
-
   virtual std::string scheme() { return "http"; }
-
- private:
-  // If non-null a background thread isn't created and instead this message loop
-  // is used.
-  MessageLoop* loop_;
 };
 
 //-----------------------------------------------------------------------------
@@ -750,22 +604,6 @@ class FTPTestServer : public BaseTestServer {
     return test_server;
   }
 
-  virtual bool MakeGETRequest(const std::string& page_name) {
-    const GURL& url = TestServerPage(page_name);
-    TestDelegate d;
-    URLRequest request(url, &d);
-    request.set_context(new TestURLRequestContext());
-    request.set_method("GET");
-    request.Start();
-    EXPECT_TRUE(request.is_pending());
-
-    MessageLoop::current()->Run();
-    if (request.is_pending())
-      return false;
-
-    return true;
-  }
-
  private:
   ~FTPTestServer() {}
 };