Run safebrowsing_service_test through the net testserver code.

chrome/browser/safe_browsing/safe_browsing_test.cc

Index: chrome/browser/safe_browsing/safe_browsing_test.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_test.cc b/chrome/browser/safe_browsing/safe_browsing_test.cc
index ccec75c38f06a36148e760a5c6bd26a508b810dc..4d4d3f1ddc3776192ac12430fcefc63d07d264f0 100644
--- a/chrome/browser/safe_browsing/safe_browsing_test.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_test.cc
@@ -31,6 +31,7 @@
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/safe_browsing/local_safebrowsing_test_server.h"
 #include "chrome/browser/safe_browsing/protocol_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/chrome_notification_types.h"
@@ -55,10 +56,9 @@ namespace {
 
 const FilePath::CharType kDataFile[] =
     FILE_PATH_LITERAL("testing_input_nomac.dat");
-const char kUrlVerifyPath[] = "/safebrowsing/verify_urls";
-const char kDBVerifyPath[] = "/safebrowsing/verify_database";
-const char kDBResetPath[] = "/reset";
-const char kTestCompletePath[] = "/test_complete";
+const char kUrlVerifyPath[] = "safebrowsing/verify_urls";
+const char kDBVerifyPath[] = "safebrowsing/verify_database";
+const char kTestCompletePath[] = "test_complete";
 
 struct PhishingUrl {
   std::string url;
@@ -108,103 +108,6 @@ bool ParsePhishingUrls(const std::string& data,
 
 }  // namespace
 
-class SafeBrowsingTestServer {
- public:
-  explicit SafeBrowsingTestServer(const FilePath& datafile)
-      : datafile_(datafile),
-        server_handle_(base::kNullProcessHandle) {
-  }
-
-  ~SafeBrowsingTestServer() {
-    EXPECT_EQ(base::kNullProcessHandle, server_handle_);
-  }
-
-  // Start the python server test suite.
-  bool Start() {
-    // Get path to python server script
-    FilePath testserver_path;
-    if (!PathService::Get(base::DIR_SOURCE_ROOT, &testserver_path)) {
-      LOG(ERROR) << "Failed to get DIR_SOURCE_ROOT";
-      return false;
-    }
-    testserver_path = testserver_path
-        .Append(FILE_PATH_LITERAL("third_party"))
-        .Append(FILE_PATH_LITERAL("safe_browsing"))
-        .Append(FILE_PATH_LITERAL("testing"));
-    AppendToPythonPath(testserver_path);
-    FilePath testserver = testserver_path.Append(
-        FILE_PATH_LITERAL("safebrowsing_test_server.py"));
-
-    FilePath pyproto_code_dir;
-    if (!GetPyProtoPath(&pyproto_code_dir)) {
-      LOG(ERROR) << "Failed to get generated python protobuf dir";
-      return false;
-    }
-    AppendToPythonPath(pyproto_code_dir);
-    pyproto_code_dir = pyproto_code_dir.Append(FILE_PATH_LITERAL("google"));
-    AppendToPythonPath(pyproto_code_dir);
-
-    FilePath python_runtime;
-    EXPECT_TRUE(GetPythonRunTime(&python_runtime));
-    CommandLine cmd_line(python_runtime);
-    // Make python stdout and stderr unbuffered, to prevent incomplete stderr on
-    // win bots, and also fix mixed up ordering of stdout and stderr.
-    cmd_line.AppendSwitch("-u");
-    FilePath datafile = testserver_path.Append(datafile_);
-    cmd_line.AppendArgPath(testserver);
-    cmd_line.AppendArg(base::StringPrintf("--port=%d", kPort_));
-    cmd_line.AppendArgNative(FILE_PATH_LITERAL("--datafile=") +
-                             datafile.value());
-
-    base::LaunchOptions options;
-#if defined(OS_WIN)
-    options.start_hidden = true;
-#endif
-    if (!base::LaunchProcess(cmd_line, options, &server_handle_)) {
-      LOG(ERROR) << "Failed to launch server: "
-                 << cmd_line.GetCommandLineString();
-      return false;
-    }
-    return true;
-  }
-
-  // Stop the python server test suite.
-  bool Stop() {
-    if (server_handle_ == base::kNullProcessHandle)
-      return true;
-
-    // First check if the process has already terminated.
-    if (!base::WaitForSingleProcess(server_handle_, base::TimeDelta()) &&
-        !base::KillProcess(server_handle_, 1, true)) {
-      VLOG(1) << "Kill failed?";
-      return false;
-    }
-
-    base::CloseProcessHandle(server_handle_);
-    server_handle_ = base::kNullProcessHandle;
-    VLOG(1) << "Stopped.";
-    return true;
-  }
-
-  static const char* Host() {
-    return kHost_;
-  }
-
-  static int Port() {
-    return kPort_;
-  }
-
- private:
-  static const char kHost_[];
-  static const int kPort_;
-  FilePath datafile_;
-  base::ProcessHandle server_handle_;
-  DISALLOW_COPY_AND_ASSIGN(SafeBrowsingTestServer);
-};
-
-const char SafeBrowsingTestServer::kHost_[] = "localhost";
-const int SafeBrowsingTestServer::kPort_ = 40102;
-
 // This starts the browser and keeps status of states related to SafeBrowsing.
 class SafeBrowsingServiceTest : public InProcessBrowserTest {
  public:
@@ -297,13 +200,28 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
     return safe_browsing_service_->safe_browsing_thread_->message_loop();
   }
 
+  const net::TestServer& test_server() const {
+    return *test_server_;
+  }
+
  protected:
   bool InitSafeBrowsingService() {
     safe_browsing_service_ = g_browser_process->safe_browsing_service();
     return safe_browsing_service_ != NULL;
   }
 
-  virtual void SetUpCommandLine(CommandLine* command_line) {
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    FilePath datafile_path;
+    ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &datafile_path));
+
+    datafile_path = datafile_path.Append(FILE_PATH_LITERAL("third_party"))
+          .Append(FILE_PATH_LITERAL("safe_browsing"))
+          .Append(FILE_PATH_LITERAL("testing"))
+          .Append(kDataFile);
+    test_server_.reset(new LocalSafeBrowsingTestServer(datafile_path));
+    ASSERT_TRUE(test_server_->Start());
+    LOG(INFO) << "server is " << test_server_->host_port_pair().ToString();
+
     // Makes sure the auto update is not triggered. This test will force the
     // update when needed.
     command_line->AppendSwitch(switches::kSbDisableAutoUpdate);
@@ -323,10 +241,7 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
         switches::kDisableClientSidePhishingDetection);
 
     // Point to the testing server for all SafeBrowsing requests.
-    std::string url_prefix =
-        base::StringPrintf("http://%s:%d/safebrowsing",
-                           SafeBrowsingTestServer::Host(),
-                           SafeBrowsingTestServer::Port());
+    std::string url_prefix = test_server_->GetURL("safebrowsing").spec();
     command_line->AppendSwitchASCII(switches::kSbURLPrefix, url_prefix);
   }
 
@@ -338,6 +253,8 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
  private:
   SafeBrowsingService* safe_browsing_service_;
 
+  scoped_ptr<net::TestServer> test_server_;
+
   // Protects all variables below since they are read on UI thread
   // but updated on IO thread or safebrowsing thread.
   base::Lock update_status_mutex_;
@@ -448,52 +365,36 @@ class SafeBrowsingServiceTestHelper
     content::RunMessageLoop();
   }
 
-  void WaitTillServerReady(const char* host, int port) {
-    response_status_ = net::URLRequestStatus::FAILED;
-    GURL url(base::StringPrintf("http://%s:%d%s?test_step=0",
-                                host, port, kDBResetPath));
-    // TODO(lzheng): We should have a way to reliably tell when a server is
-    // ready so we could get rid of the Sleep and retry loop.
-    while (true) {
-      if (FetchUrl(url) == net::URLRequestStatus::SUCCESS)
-        break;
-      // Wait and try again if last fetch was failed. The loop will hit the
-      // timeout in OutOfProcTestRunner if the fetch can not get success
-      // response.
-      base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
-    }
-  }
-
   // Calls test server to fetch database for verification.
-  net::URLRequestStatus::Status FetchDBToVerify(const char* host, int port,
-                                                int test_step) {
+  net::URLRequestStatus::Status FetchDBToVerify(
+      const net::TestServer& test_server,
+      int test_step) {
     // TODO(lzheng): Remove chunk_type=add once it is not needed by the server.
-    GURL url(base::StringPrintf(
-        "http://%s:%d%s?"
-        "client=chromium&appver=1.0&pver=2.2&test_step=%d&"
-        "chunk_type=add",
-        host, port, kDBVerifyPath, test_step));
-    return FetchUrl(url);
+    std::string path = base::StringPrintf(
+        "%s?client=chromium&appver=1.0&pver=2.2&test_step=%d&chunk_type=add",
+        kDBVerifyPath, test_step);
+    return FetchUrl(test_server.GetURL(path));
   }
 
   // Calls test server to fetch URLs for verification.
-  net::URLRequestStatus::Status FetchUrlsToVerify(const char* host, int port,
-                                                  int test_step) {
-    GURL url(base::StringPrintf(
-        "http://%s:%d%s?"
-        "client=chromium&appver=1.0&pver=2.2&test_step=%d",
-        host, port, kUrlVerifyPath, test_step));
-    return FetchUrl(url);
+  net::URLRequestStatus::Status FetchUrlsToVerify(
+      const net::TestServer& test_server,
+      int test_step) {
+    std::string path = base::StringPrintf(
+        "%s?client=chromium&appver=1.0&pver=2.2&test_step=%d",
+        kUrlVerifyPath, test_step);
+    return FetchUrl(test_server.GetURL(path));
   }
 
   // Calls test server to check if test data is done. E.g.: if there is a
   // bad URL that server expects test to fetch full hash but the test didn't,
   // this verification will fail.
-  net::URLRequestStatus::Status VerifyTestComplete(const char* host, int port,
-                                                   int test_step) {
-    GURL url(StringPrintf("http://%s:%d%s?test_step=%d",
-                          host, port, kTestCompletePath, test_step));
-    return FetchUrl(url);
+  net::URLRequestStatus::Status VerifyTestComplete(
+      const net::TestServer& test_server,
+      int test_step) {
+    std::string path = base::StringPrintf(
+        "%s?test_step=%d", kTestCompletePath, test_step);
+    return FetchUrl(test_server.GetURL(path));
   }
 
   // Callback for URLFetcher.
@@ -540,10 +441,8 @@ class SafeBrowsingServiceTestHelper
 
 // See http://crbug.com/96459
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
-                       DISABLED_SafeBrowsingSystemTest) {
+                       SafeBrowsingSystemTest) {
   LOG(INFO) << "Start test";
-  const char* server_host = SafeBrowsingTestServer::Host();
-  int server_port = SafeBrowsingTestServer::Port();
   ASSERT_TRUE(InitSafeBrowsingService());
 
   net::URLRequestContextGetter* request_context =
@@ -551,12 +450,6 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
   scoped_refptr<SafeBrowsingServiceTestHelper> safe_browsing_helper(
       new SafeBrowsingServiceTestHelper(this, request_context));
   int last_step = 0;
-  FilePath datafile_path = FilePath(kDataFile);
-  SafeBrowsingTestServer test_server(datafile_path);
-  ASSERT_TRUE(test_server.Start());
-
-  // Make sure the server is running.
-  safe_browsing_helper->WaitTillServerReady(server_host, server_port);
 
   // Waits and makes sure safebrowsing update is not happening.
   // The wait will stop once OnWaitForStatusUpdateDone in
@@ -593,9 +486,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
 
     // Fetches URLs to verify and waits till server responses with data.
     EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-              safe_browsing_helper->FetchUrlsToVerify(server_host,
-                                                      server_port,
-                                                      step));
+              safe_browsing_helper->FetchUrlsToVerify(test_server(), step));
 
     std::vector<PhishingUrl> phishing_urls;
     EXPECT_TRUE(ParsePhishingUrls(safe_browsing_helper->response_data(),
@@ -624,18 +515,13 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
     // TODO(lzheng): We should verify the fetched database with local
     // database to make sure they match.
     EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-              safe_browsing_helper->FetchDBToVerify(server_host,
-                                                    server_port,
-                                                    step));
+              safe_browsing_helper->FetchDBToVerify(test_server(), step));
     EXPECT_GT(safe_browsing_helper->response_data().size(), 0U);
     last_step = step;
   }
 
   // Verifies with server if test is done and waits till server responses.
   EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-            safe_browsing_helper->VerifyTestComplete(server_host,
-                                                     server_port,
-                                                     last_step));
+            safe_browsing_helper->VerifyTestComplete(test_server(), last_step));
   EXPECT_EQ("yes", safe_browsing_helper->response_data());
-  test_server.Stop();
 }