Add WebSocket cookie tests.

net/websockets/websocket_stream_test_util.h

+// Copyright 2015 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_WEBSOCKETS_WEBSOCKET_STREAM_TEST_UTIL_H_
+#define NET_WEBSOCKETS_WEBSOCKET_STREAM_TEST_UTIL_H_

[Ryan Sleevi, 2015/02/07 02:32:46]

This file needs to be renamed to websocket_stream_test_base.h

[yhirano, 2015/02/09 10:50:15]

websocket_stream_create_test_base.h

+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "net/base/net_export.h"
+#include "net/http/http_request_headers.h"

[Ryan Sleevi, 2015/02/02 20:14:42]

Forward declare

[yhirano, 2015/02/09 10:50:15]

Done.

+#include "net/http/http_response_headers.h"

[Ryan Sleevi, 2015/02/02 20:14:42]

Forward declare

[yhirano, 2015/02/09 10:50:16]

Done.

+#include "net/socket/socket_test_util.h"

[Ryan Sleevi, 2015/02/02 20:14:43]

unnecessary?

[yhirano, 2015/02/09 10:50:15]

For ScopedWebSocketEndpointZeroUnlockDelay

+#include "net/websockets/websocket_handshake_request_info.h"

[Ryan Sleevi, 2015/02/02 20:14:42]

Forward declare

[yhirano, 2015/02/09 10:50:15]

Done.

+#include "net/websockets/websocket_handshake_response_info.h"

[Ryan Sleevi, 2015/02/02 20:14:42]

Forward declare

[yhirano, 2015/02/09 10:50:15]

Done.

+#include "net/websockets/websocket_test_util.h"
+#include "url/gurl.h"

[Ryan Sleevi, 2015/02/02 20:14:42]

Forward declare

[yhirano, 2015/02/09 10:50:15]

removed

+
+namespace net {
+
+NET_EXPORT_PRIVATE class WebSocketStreamCreateTestBase {

[Ryan Sleevi, 2015/02/02 20:14:42]

class NET_EXPORT_PRIVATE WebSocketStreamCreateTestBase

[yhirano, 2015/02/09 10:50:15]

Done.

+  class TestConnectDelegate;

[Ryan Sleevi, 2015/02/02 20:14:42]

STYLE: Inappropriate forward declaration. See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Class_Format

private goes last / end of file.

[yhirano, 2015/02/09 10:50:15]

Done.

+
+ public:
+  using HeaderKeyValuePair = std::pair<std::string, std::string>;
+
+  WebSocketStreamCreateTestBase();
+  virtual ~WebSocketStreamCreateTestBase();
+
+  // A wrapper for CreateAndConnectStreamForTesting that knows about our default
+  // parameters.
+  void CreateAndConnectStream(const std::string& socket_url,
+                              const std::vector<std::string>& sub_protocols,
+                              const std::string& origin,
+                              scoped_ptr<base::Timer> timer);

[Ryan Sleevi, 2015/02/02 20:14:43]

IWYU: Include timer header

[yhirano, 2015/02/09 10:50:15]

Done.
You suggested forward-declaring WebSocketHandshakeRequest used in
scoped_ptr<WebSocketHandshakeRequestInfo> request_info_; and suggested including
timer.h for scoped_ptr<base::Timer>. Can you tell me the difference between
them?

[Ryan Sleevi, 2015/02/10 01:26:48]

Forward declaring should also work here. I was noting that there was no
definition of base::Timer, either in forward declare or explicit headers. Sorry
for the ambiguity.

+
+  static std::vector<HeaderKeyValuePair> ToVector(
+      const HttpRequestHeaders& headers);
+  static std::vector<HeaderKeyValuePair> ToVector(
+      const HttpResponseHeaders& headers);
+
+  const std::string& failure_message() const { return failure_message_; }
+  bool has_failed() const { return has_failed_; }
+
+  static void RunUntilIdle() { base::RunLoop().RunUntilIdle(); }

[Ryan Sleevi, 2015/02/02 20:14:42]

Don't inline

[yhirano, 2015/02/09 10:50:15]

Done.

+
+  // A simple function to make the tests more readable. Creates an empty vector.

[Ryan Sleevi, 2015/02/02 20:14:42]

Drop second sentance

[yhirano, 2015/02/09 10:50:15]

Done.

+  static std::vector<std::string> NoSubProtocols() {

[Ryan Sleevi, 2015/02/02 20:14:42]

Is static really necessary? Ditto line 49.

[yhirano, 2015/02/03 01:26:32]

I'd like to leave it be because we can see more easily that no instance
information is used in the implementation.

[Ryan Sleevi, 2015/02/03 02:16:27]

Why is this desirable? It's a unit test. A new instance of this class is created
for every single test. Rather than providing clarity that this can be called
without accessing instance data, it instead suggests that this can be publicly
called by any test.

Further in that regard, why aren't these test methods all protected as well?
This is the base class for unit tests, so everything will derive from it.

[Adam Rice, 2015/02/03 05:34:35]

I agree with yhirano that this method should be static. Methods which read/write
instance data must be clearly distinguished from those which don't.

I don't think it matters whether it is public or protected. It would be
pointless to use it in a test which doesn't inherit from this class, but it
would not break anything.

[yhirano, 2015/02/09 10:50:15]

Done (static).

Regarding public / protected, some existing tests are assuming that some methods
are public. I don't want to fix them in this CL.

+    return std::vector<std::string>();

[Ryan Sleevi, 2015/02/02 20:14:42]

Don't inline

[yhirano, 2015/02/09 10:50:15]

Done.

+  }
+
+ protected:
+  WebSocketTestURLRequestContextHost url_request_context_host_;
+  scoped_ptr<WebSocketStreamRequest> stream_request_;
+  // Only set if the connection succeeded.
+  scoped_ptr<WebSocketStream> stream_;
+  // Only set if the connection failed.
+  std::string failure_message_;
+  bool has_failed_;
+  scoped_ptr<WebSocketHandshakeRequestInfo> request_info_;
+  scoped_ptr<WebSocketHandshakeResponseInfo> response_info_;
+  scoped_ptr<WebSocketEventInterface::SSLErrorCallbacks> ssl_error_callbacks_;
+  SSLInfo ssl_info_;

[Ryan Sleevi, 2015/02/02 20:14:43]

IWYU: Include SSLInfo header

[yhirano, 2015/02/09 10:50:15]

Done.

+  bool ssl_fatal_;
+  ScopedVector<SSLSocketDataProvider> ssl_data_;

[Ryan Sleevi, 2015/02/02 20:14:42]

IWYU: Include base/memory/scoped_vector

[yhirano, 2015/02/09 10:50:15]

Done.

+  ScopedWebSocketEndpointZeroUnlockDelay zero_unlock_delay_;
+};

[Ryan Sleevi, 2015/02/02 20:14:42]

private:
 DISALLOW_COPY_AND_ASSIGN

[yhirano, 2015/02/09 10:50:16]

Done.

+
+}  // namespace net
+
+#endif  // NET_WEBSOCKETS_WEBSOCKET_STREAM_TEST_UTIL_H_