WebSocket test server migration on browser_tests

chrome/browser/ssl/ssl_browser_tests.cc

Index: chrome/browser/ssl/ssl_browser_tests.cc
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 68e005c65632c0585474fae9e576d6adf355f1bd..a982d9cb0ea2c00b46fa9da8df36a643fe18ebe2 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -55,6 +55,7 @@ using content::SSLStatus;
 using content::WebContents;
 
 const FilePath::CharType kDocRoot[] = FILE_PATH_LITERAL("chrome/test/data");
+const FilePath::CharType kWsRoot[] = FILE_PATH_LITERAL("net/data/websocket");

[wtc, 2012/10/23 19:02:55]

(NOTE: my comments in this file also apply to
chrome/browser/net/proxy_browsertest.cc.)

It seems bad to put test data inside src/net. I am worried
that someone who works on src/net may not know there are
unit tests outside src/net that depend on the test data in
net/data/websocket.

Can this be avoided?

[Ryan Sleevi, 2012/10/23 19:04:35]

We do this for the SSL certs already.

We could wrap this up in an API for net:: to expose the path for WS test data,
but I'm not sure that really resolves the issue you highlighted, just shuffles
it (from looking for usage of files to looking for usage of callers)

[Takashi Toyoshima, 2012/10/24 05:29:08]

I have a same idea with Ryan.
But, the directory for SSL certs have a good README document.
So, at least, net/data/websocket should have similar one, I think.

In addition, usually, we don't have information for reverse dependency, right?
I think 'git grep net/data/websocket' works fine to know a reverse dependency.

Anyway, I will file a new bug, and consider what should be done to avoid
mistakes.

 
 namespace {
 
@@ -91,8 +92,6 @@ class ProvisionalLoadWaiter : public content::WebContentsObserver {
 }  // namespace
 
 class SSLUITest : public InProcessBrowserTest {
-  typedef net::TestServer::SSLOptions SSLOptions;
-
  public:
   SSLUITest()
       : https_server_(net::TestServer::TYPE_HTTPS,
@@ -103,7 +102,10 @@ class SSLUITest : public InProcessBrowserTest {
                               FilePath(kDocRoot)),
         https_server_mismatched_(net::TestServer::TYPE_HTTPS,
                                  SSLOptions(SSLOptions::CERT_MISMATCHED_NAME),
-                                 FilePath(kDocRoot)) {}
+                                 FilePath(kDocRoot)),
+        wss_server_expired_(net::TestServer::TYPE_WSS,
+                            SSLOptions(SSLOptions::CERT_EXPIRED),
+                            FilePath(kWsRoot)) {}
 
   virtual void SetUpCommandLine(CommandLine* command_line) {
     // Browser will both run and display insecure content.
@@ -296,6 +298,10 @@ class SSLUITest : public InProcessBrowserTest {
   net::TestServer https_server_;
   net::TestServer https_server_expired_;
   net::TestServer https_server_mismatched_;
+  net::TestServer wss_server_expired_;
+
+ protected:
+  typedef net::TestServer::SSLOptions SSLOptions;

[wtc, 2012/10/23 19:04:32]

It seems that this typedef can be private. Does that work?

[Takashi Toyoshima, 2012/10/24 05:29:08]

Oops.
Thanks. You are right.
I'll fix this.

 
  private:
   DISALLOW_COPY_AND_ASSIGN(SSLUITest);
@@ -550,7 +556,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoForward) {
 // response from UI thread.
 IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndClose) {
   ASSERT_TRUE(test_server()->Start());
-  ASSERT_TRUE(https_server_expired_.Start());
+  ASSERT_TRUE(wss_server_expired_.Start());
 
   // Setup page title observer.
   WebContents* tab = chrome::GetActiveWebContents(browser());
@@ -560,11 +566,11 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndClose) {
   // Create GURLs to test pages.
   std::string masterUrlPath = StringPrintf("%s?%d",
       test_server()->GetURL("files/ssl/wss_close.html").spec().c_str(),
-      https_server_expired_.host_port_pair().port());
+      wss_server_expired_.host_port_pair().port());
   GURL masterUrl(masterUrlPath);
   std::string slaveUrlPath = StringPrintf("%s?%d",
       test_server()->GetURL("files/ssl/wss_close_slave.html").spec().c_str(),
-      https_server_expired_.host_port_pair().port());
+      wss_server_expired_.host_port_pair().port());
   GURL slaveUrl(slaveUrlPath);
 
   // Create tabs and visit pages which keep on creating wss connections.
@@ -592,15 +598,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndClose) {
 // share certificates policy with HTTPS correcly.
 IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndGoForward) {
   ASSERT_TRUE(test_server()->Start());
-  ASSERT_TRUE(https_server_expired_.Start());
-
-  // Start pywebsocket with TLS.
-  content::TestWebSocketServer wss_server;
-  int port = wss_server.UseRandomPort();
-  wss_server.UseTLS();
-  FilePath wss_root_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &wss_root_dir));
-  ASSERT_TRUE(wss_server.Start(wss_root_dir));
+  ASSERT_TRUE(wss_server_expired_.Start());
 
   // Setup page title observer.
   WebContents* tab = chrome::GetActiveWebContents(browser());
@@ -608,11 +606,15 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndGoForward) {
   watcher.AlsoWaitForTitle(ASCIIToUTF16("FAIL"));
 
   // Visit bad HTTPS page.
-  std::string urlPath =
-      StringPrintf("%s%d%s", "https://localhost:", port, "/ws.html");
-  ui_test_utils::NavigateToURL(browser(), GURL(urlPath));
-  CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID,
-                                 false, true);  // Interstitial showing
+  std::string scheme("https");
+  GURL::Replacements replacements;
+  replacements.SetSchemeStr(scheme);

[wtc, 2012/10/23 19:02:55]

Just curious: why does this unit test need to replace the
scheme? What is the original scheme in the URL for
"connect_check.html"?

[Takashi Toyoshima, 2012/10/24 05:29:08]

|wss_server_expired_| is launched with TYPE_WSS.
So, its GetURL() returns wss scheme for 'connect_check.html'.
This is because TYPE_WS serves http and ws, and TYPE_WSS does https and wss.

[wtc, 2012/10/25 00:18:09]

I see. Does this mean ui_test_utils::NavigateToURL() cannot
handle wss and ws URLs, so you have to change the scheme
in the URL to https or http?

+  ui_test_utils::NavigateToURL(
+      browser(),
+      wss_server_expired_.GetURL(
+          "connect_check.html").ReplaceComponents(replacements));
+  CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
+                                 true);  // Interstitial showing
 
   // Proceed anyway.
   ProceedThroughInterstitial(tab);
@@ -1577,15 +1579,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITestBlock, TestBlockRunningInsecureContent) {
 // interstitial page showing.
 IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreCertErrors, TestWSS) {
   ASSERT_TRUE(test_server()->Start());
-  ASSERT_TRUE(https_server_expired_.Start());
-
-  // Start pywebsocket with TLS.
-  content::TestWebSocketServer wss_server;
-  int port = wss_server.UseRandomPort();
-  wss_server.UseTLS();
-  FilePath wss_root_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &wss_root_dir));
-  ASSERT_TRUE(wss_server.Start(wss_root_dir));
+  ASSERT_TRUE(wss_server_expired_.Start());
 
   // Setup page title observer.
   WebContents* tab = chrome::GetActiveWebContents(browser());
@@ -1593,9 +1587,13 @@ IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreCertErrors, TestWSS) {
   watcher.AlsoWaitForTitle(ASCIIToUTF16("FAIL"));
 
   // Visit bad HTTPS page.
-  std::string url_path =
-      StringPrintf("%s%d%s", "https://localhost:", port, "/ws.html");
-  ui_test_utils::NavigateToURL(browser(), GURL(url_path));
+  std::string scheme("https");
+  GURL::Replacements replacements;
+  replacements.SetSchemeStr(scheme);
+  ui_test_utils::NavigateToURL(
+      browser(),
+      wss_server_expired_.GetURL(
+          "connect_check.html").ReplaceComponents(replacements));
 
   // We shouldn't have an interstitial page showing here.