Resolve issue from feedback for 1417363004

chrome/browser/local_discovery/privet_http_unittest.cc

 // Copyright 2013 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.
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "chrome/browser/local_discovery/privet_http_impl.h"
+#include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
-#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_test_util.h"
 #include "printing/pwg_raster_settings.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(ENABLE_PRINT_PREVIEW)
 #include "chrome/browser/local_discovery/pwg_raster_converter.h"
 #endif  // ENABLE_PRINT_PREVIEW
 
 namespace local_discovery {
 
 namespace {
 
 using testing::StrictMock;
 using testing::NiceMock;
 
 using content::BrowserThread;
-using net::SpawnedTestServer;
+using net::EmbeddedTestServer;
 
 const char kSampleInfoResponse[] = "{"
     "       \"version\": \"1.0\","
     "       \"name\": \"Common printer\","
     "       \"description\": \"Printer connected through Chrome connector\","
     "       \"url\": \"https://www.google.com/cloudprint\","
     "       \"type\": ["
     "               \"printer\""
     "       ],"
     "       \"id\": \"\","
@@ skipping 1029 matching lines @@
       kSampleErrorResponsePrinterBusy));
 
   RunFor(base::TimeDelta::FromSeconds(3));
 
   EXPECT_TRUE(SuccessfulResponseToURL(
       GURL("http://10.0.0.8:6006/privet/printer/createjob"),
       kSampleCreatejobResponse));
 }
 #endif  // ENABLE_PRINT_PREVIEW
 
-const base::FilePath::CharType kDocRoot[] =
-    FILE_PATH_LITERAL("chrome/test/data");
-
 class PrivetHttpWithServerTest : public ::testing::Test,
                                  public PrivetURLFetcher::Delegate {
  protected:
-  PrivetHttpWithServerTest()
-      : thread_bundle_(content::TestBrowserThreadBundle::REAL_IO_THREAD) {}
+  PrivetHttpWithServerTest() : network_thread_("network thread") {}
+
+  void SetUp() override {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kEnablePrivetV3);
+
+    base::Thread::Options network_thread_options;
+    network_thread_options.message_loop_type = base::MessageLoop::TYPE_IO;
+    ASSERT_TRUE(network_thread_.StartWithOptions(network_thread_options));

[Ryan Sleevi, 2015/10/30 23:17:27]

BUG: Don't create threads like this. Use TestBrowserThread /
TestBrowserThreadBundle if you're trying to create threads for testing in
//content or //chrome that reflect 'real' things.

[Vitaly Buka (NO REVIEWS), 2015/10/31 03:55:35]

Done.

+
+    context_getter_ =
+        new net::TestURLRequestContextGetter(network_thread_.task_runner());
+  }
+
   void OnError(PrivetURLFetcher* fetcher,
                PrivetURLFetcher::ErrorType error) override {
     done_ = true;
     success_ = false;
     error_ = error;
 
     base::MessageLoop::current()->PostTask(FROM_HERE, quit_);

[Ryan Sleevi, 2015/10/30 23:17:27]

At some point, preferably in a separate CL, you should scrub these
base::MessageLoop::current() calls in //chrome/browser/local_discovery with
base::ThreadTaskRunnerHandle::Get()

[Vitaly Buka (NO REVIEWS), 2015/10/31 03:55:35]

Acknowledged.

   }
 
   void OnParsedJson(PrivetURLFetcher* fetcher,
                     const base::DictionaryValue& value,
                     bool has_error) override {
     NOTREACHED();
     base::MessageLoop::current()->PostTask(FROM_HERE, quit_);
   }
 
   bool OnRawData(PrivetURLFetcher* fetcher,
                  bool response_is_file,
                  const std::string& data_string,
                  const base::FilePath& data_file) override {
     done_ = true;
     success_ = true;
 
     base::MessageLoop::current()->PostTask(FROM_HERE, quit_);
     return true;
   }
 
+  void CreateServer(EmbeddedTestServer::Type type) {
+    server_.reset(new EmbeddedTestServer(type));
+    ASSERT_TRUE(server_->InitializeAndWaitUntilReady());
+
+    base::FilePath test_data_dir;
+    ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &test_data_dir));
+    server_->ServeFilesFromDirectory(
+        test_data_dir.Append(FILE_PATH_LITERAL("chrome/test/data")));
+  }
+
   void CreateClient() {
-    client_.reset(new PrivetHTTPClientImpl(
-        "test", https_server_->host_port_pair(),
-        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
+    client_.reset(new PrivetHTTPClientImpl("test", server_->host_port_pair(),
+                                           context_getter_));
   }
 
   void Run() {
     success_ = false;
     done_ = false;
 
     base::RunLoop run_loop;
     quit_ = run_loop.QuitClosure();
 
-    auto fetcher = client_->CreateURLFetcher(
-        https_server_->GetURL("files/simple.html"), net::URLFetcher::GET, this);
+    auto fetcher = client_->CreateURLFetcher(server_->GetURL("/simple.html"),

[Ryan Sleevi, 2015/10/30 23:17:27]

STYLE: This is not an appropriate use of auto

See the discussion on the dangers of this usage at
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/5-Bt3BJzAo0/dis...
(linked from chromium-cpp.appspot.com)

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/5-Bt3BJzAo0/ZW8Lm...
for some specific context

[Vitaly Buka (NO REVIEWS), 2015/10/31 03:55:36]

Done.

+                                             net::URLFetcher::GET, this);
 
     fetcher->SetMaxRetries(1);
     fetcher->V3Mode();
     fetcher->Start();
 
     run_loop.Run();
 
     EXPECT_TRUE(done_);
   }
 
   bool success_ = false;
   bool done_ = false;
   PrivetURLFetcher::ErrorType error_ = PrivetURLFetcher::ErrorType();
-  content::TestBrowserThreadBundle thread_bundle_;
-
-  scoped_ptr<SpawnedTestServer> https_server_;
+  base::Thread network_thread_;
+  base::MessageLoop message_loop_;
+  scoped_refptr<net::TestURLRequestContextGetter> context_getter_;
+  scoped_ptr<EmbeddedTestServer> server_;
   scoped_ptr<PrivetHTTPClientImpl> client_;
 
   base::Closure quit_;
 };
 
-TEST_F(PrivetHttpWithServerTest, Http) {
-  https_server_.reset(new SpawnedTestServer(net::BaseTestServer::TYPE_HTTP,
-                                            SpawnedTestServer::kLocalhost,
-                                            base::FilePath(kDocRoot)));
-  ASSERT_TRUE(https_server_->Start());
+TEST_F(PrivetHttpWithServerTest, HttpServer) {
+  CreateServer(EmbeddedTestServer::TYPE_HTTP);
 
   CreateClient();
   Run();
   EXPECT_TRUE(success_);
 
   CreateClient();
   net::SHA256HashValue fingerprint = {};
-  client_->SwitchToHttps(https_server_->host_port_pair().port(), fingerprint);
-
+  client_->SwitchToHttps(server_->host_port_pair().port(), fingerprint);
   Run();
   EXPECT_FALSE(success_);
   EXPECT_EQ(PrivetURLFetcher::UNKNOWN_ERROR, error_);
 }
 
-TEST_F(PrivetHttpWithServerTest, Https) {
-  https_server_.reset(new SpawnedTestServer(
-      SpawnedTestServer::TYPE_HTTPS,
-      SpawnedTestServer::SSLOptions(SpawnedTestServer::SSLOptions::CERT_OK),
-      base::FilePath(kDocRoot)));
-  ASSERT_TRUE(https_server_->Start());
+TEST_F(PrivetHttpWithServerTest, HttpsServer) {
+  CreateServer(EmbeddedTestServer::TYPE_HTTPS);
 
   CreateClient();
   Run();
-#ifdef OS_MACOSX
-  // Seems like an issue of the test server, it's in HTTPS mode and still
-  // accepts
-  // HTTP requests.
-  // TODO(vitalybuka): Remove #ifdef when test server is fixed.
-  EXPECT_TRUE(success_);
-#else
   EXPECT_FALSE(success_);
   EXPECT_EQ(PrivetURLFetcher::UNKNOWN_ERROR, error_);
-#endif  // OS_MACOSX
 
   CreateClient();
   net::SHA256HashValue fingerprint =
       net::X509Certificate::CalculateFingerprint256(
-          https_server_->GetCertificate()->os_cert_handle());
-  client_->SwitchToHttps(https_server_->host_port_pair().port(), fingerprint);
-
+          server_->GetCertificate()->os_cert_handle());
+  client_->SwitchToHttps(server_->host_port_pair().port(), fingerprint);
   Run();
   EXPECT_TRUE(success_);
 
   CreateClient();
   fingerprint = {};
-  client_->SwitchToHttps(https_server_->host_port_pair().port(), fingerprint);
-
+  client_->SwitchToHttps(server_->host_port_pair().port(), fingerprint);
   Run();
   EXPECT_FALSE(success_);
   EXPECT_EQ(PrivetURLFetcher::REQUEST_CANCELED, error_);
 }
 
 }  // namespace
 
 }  // namespace local_discovery