Refactoring for merging WebSocket test server to net::TestServer

net/test/base_test_server.h

 // Copyright (c) 2012 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_TEST_BASE_TEST_SERVER_H_
 #define NET_TEST_BASE_TEST_SERVER_H_
 
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 17 matching lines @@
 // The base class of Test server implementation.
 class BaseTestServer {
  public:
   typedef std::pair<std::string, std::string> StringPair;
 
   enum Type {
     TYPE_FTP,
     TYPE_GDATA,
     TYPE_HTTP,
     TYPE_HTTPS,
+    TYPE_WS,

[Paweł Hajdan Jr., 2012/08/30 08:56:34]

This is not obvious (at least to me). Could you make that TYPE_WEB_SOCKET, or
add a comment like this:

TYPE_WS,  // Web Socket.
(and same below)

[Ryan Sleevi, 2012/08/30 14:25:46]

Given that this enumeration is listing protocol schemes (ftp, gdata, HTTP, WS),
this does seem like overkill. Its not abbreviating - it is the registered uri
scheme.

[Paweł Hajdan Jr., 2012/08/31 07:51:29]

Please add the comment then. "ws" is not that easy to search on Google...

[Takashi Toyoshima, 2012/09/01 09:07:30]

WebSocket or ws.
I'm often worried about which is better symbol there.
We never say HyperTextTransferProtocol, but just HTTP.
On the other hand, WS is too short to identify, and is not well-known for now.

How about adding a common comment like
// Following types represent protocol schemes. See also
http://www.iana.org/assignments/uri-schemes.html
or something just before the line 33.

I'll apply this to http://codereview.chromium.org/10879029/

[Paweł Hajdan Jr., 2012/09/04 09:30:11]

+1
Generally sounds good, although I'm not sure if gdata or sync are IANA
schemes... It may be better to just a comment for ws and wss that they are about
websockets.

+    TYPE_WSS,
     TYPE_SYNC,
     TYPE_TCP_ECHO,
     TYPE_UDP_ECHO,
   };
 
-  // Container for various options to control how the HTTPS server is
+  // Container for various options to control how the HTTPS or WSS server is
   // initialized.
-  struct HTTPSOptions {
+  struct SSLOptions {
     enum ServerCertificate {
       CERT_OK,
 
       // CERT_AUTO causes the testserver to generate a test certificate issued
       // by "Testing CA" (see net/data/ssl/certificates/ocsp-test-root.pem).
       CERT_AUTO,
 
       CERT_MISMATCHED_NAME,
       CERT_EXPIRED,
       // Cross-signed certificate to test PKIX path building. Contains an
@@ skipping 32 matching lines @@
 
     // NOTE: the values of these enumerators are passed to the the Python test
     // server. Do not change them.
     enum TLSIntolerantLevel {
       TLS_INTOLERANT_NONE = 0,
       TLS_INTOLERANT_ALL = 1,  // Intolerant of all TLS versions.
       TLS_INTOLERANT_TLS1_1 = 2,  // Intolerant of TLS 1.1 or higher.
       TLS_INTOLERANT_TLS1_2 = 3,  // Intolerant of TLS 1.2 or higher.
     };
 
-    // Initialize a new HTTPSOptions using CERT_OK as the certificate.
-    HTTPSOptions();
+    // Initialize a new SSLOptions using CERT_OK as the certificate.
+    SSLOptions();
 
-    // Initialize a new HTTPSOptions that will use the specified certificate.
-    explicit HTTPSOptions(ServerCertificate cert);
-    ~HTTPSOptions();
+    // Initialize a new SSLOptions that will use the specified certificate.
+    explicit SSLOptions(ServerCertificate cert);
+    ~SSLOptions();
 
     // Returns the relative filename of the file that contains the
     // |server_certificate|.
     FilePath GetCertificateFile() const;
 
     // GetOCSPArgument returns the value of any OCSP argument to testserver or
     // the empty string if there is none.
     std::string GetOCSPArgument() const;
 
     // The certificate to use when serving requests.
@@ skipping 30 matching lines @@
 
   // Pass as the 'host' parameter during construction to server on 127.0.0.1
   static const char kLocalhost[];
 
   // The auth token to be used for TYPE_GDATA server.
   static const char kGDataAuthToken[];
 
   // Initialize a TestServer listening on a specific host (IP or hostname).
   BaseTestServer(Type type,  const std::string& host);
 
-  // Initialize a HTTPS TestServer with a specific set of HTTPSOptions.
-  explicit BaseTestServer(const HTTPSOptions& https_options);
+  // Initialize a TestServer with a specific set of SSLOptions for HTTPS or WSS.
+  explicit BaseTestServer(Type type, const SSLOptions& ssl_options);
 
   // Returns the host port pair used by current Python based test server only
   // if the server is started.
   const HostPortPair& host_port_pair() const;
 
   const FilePath& document_root() const { return document_root_; }
   const base::DictionaryValue& server_data() const;
   std::string GetScheme() const;
   bool GetAddressList(AddressList* address_list) const WARN_UNUSED_RESULT;
 
@@ skipping 52 matching lines @@
 
   // Directory that contains the SSL certificates.
   FilePath certificates_dir_;
 
   // Address the test server listens on.
   HostPortPair host_port_pair_;
 
   // Holds the data sent from the server (e.g., port number).
   scoped_ptr<base::DictionaryValue> server_data_;
 
-  // If |type_| is TYPE_HTTPS, the TLS settings to use for the test server.
-  HTTPSOptions https_options_;
+  // If |type_| is TYPE_HTTPS or TYPE_WSS, the TLS settings to use for the test
+  // server.
+  SSLOptions ssl_options_;
 
   Type type_;
 
   // Has the server been started?
   bool started_;
 
   // Enables logging of the server to the console.
   bool log_to_console_;
 
   scoped_ptr<ScopedPortException> allowed_port_;
 
   DISALLOW_COPY_AND_ASSIGN(BaseTestServer);
 };
 
 }  // namespace net
 
 #endif  // NET_TEST_BASE_TEST_SERVER_H_