Revert 154861 - 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
===================================================================
--- chrome/browser/safe_browsing/safe_browsing_test.cc	(revision 154874)
+++ chrome/browser/safe_browsing/safe_browsing_test.cc	(working copy)
@@ -31,7 +31,6 @@
 #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"
@@ -56,9 +55,10 @@
 
 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 kTestCompletePath[] = "test_complete";
+const char kUrlVerifyPath[] = "/safebrowsing/verify_urls";
+const char kDBVerifyPath[] = "/safebrowsing/verify_database";
+const char kDBResetPath[] = "/reset";
+const char kTestCompletePath[] = "/test_complete";
 
 struct PhishingUrl {
   std::string url;
@@ -108,6 +108,103 @@
 
 }  // 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:
@@ -200,28 +297,13 @@
     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) 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();
-
+  virtual void SetUpCommandLine(CommandLine* command_line) {
     // Makes sure the auto update is not triggered. This test will force the
     // update when needed.
     command_line->AppendSwitch(switches::kSbDisableAutoUpdate);
@@ -241,7 +323,10 @@
         switches::kDisableClientSidePhishingDetection);
 
     // Point to the testing server for all SafeBrowsing requests.
-    std::string url_prefix = test_server_->GetURL("safebrowsing").spec();
+    std::string url_prefix =
+        base::StringPrintf("http://%s:%d/safebrowsing",
+                           SafeBrowsingTestServer::Host(),
+                           SafeBrowsingTestServer::Port());
     command_line->AppendSwitchASCII(switches::kSbURLPrefix, url_prefix);
   }
 
@@ -253,8 +338,6 @@
  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_;
@@ -365,36 +448,52 @@
     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 net::TestServer& test_server,
-      int test_step) {
+  net::URLRequestStatus::Status FetchDBToVerify(const char* host, int port,
+                                                int test_step) {
     // TODO(lzheng): Remove chunk_type=add once it is not needed by the server.
-    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));
+    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);
   }
 
   // Calls test server to fetch URLs for verification.
-  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));
+  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);
   }
 
   // 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 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));
+  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);
   }
 
   // Callback for URLFetcher.
@@ -441,8 +540,10 @@
 
 // See http://crbug.com/96459
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
-                       SafeBrowsingSystemTest) {
+                       DISABLED_SafeBrowsingSystemTest) {
   LOG(INFO) << "Start test";
+  const char* server_host = SafeBrowsingTestServer::Host();
+  int server_port = SafeBrowsingTestServer::Port();
   ASSERT_TRUE(InitSafeBrowsingService());
 
   net::URLRequestContextGetter* request_context =
@@ -450,7 +551,13 @@
   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
   // safe_browsing_helper is called and status from safe_browsing_service_
@@ -486,7 +593,9 @@
 
     // Fetches URLs to verify and waits till server responses with data.
     EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-              safe_browsing_helper->FetchUrlsToVerify(test_server(), step));
+              safe_browsing_helper->FetchUrlsToVerify(server_host,
+                                                      server_port,
+                                                      step));
 
     std::vector<PhishingUrl> phishing_urls;
     EXPECT_TRUE(ParsePhishingUrls(safe_browsing_helper->response_data(),
@@ -515,13 +624,18 @@
     // 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(test_server(), step));
+              safe_browsing_helper->FetchDBToVerify(server_host,
+                                                    server_port,
+                                                    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(test_server(), last_step));
+            safe_browsing_helper->VerifyTestComplete(server_host,
+                                                     server_port,
+                                                     last_step));
   EXPECT_EQ("yes", safe_browsing_helper->response_data());
+  test_server.Stop();
 }