Add WebSocket cookie tests.

net/websockets/websocket_stream_test.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 "net/websockets/websocket_stream.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 106 matching lines @@
       const std::string& extra_request_headers,
       const std::string& response_body,
       scoped_ptr<base::Timer> timer = scoped_ptr<base::Timer>()) {
     url_request_context_host_.SetExpectations(
         WebSocketStandardRequest(socket_path, socket_host, origin,
                                  extra_request_headers),
         response_body);
     CreateAndConnectStream(socket_url, sub_protocols, origin, timer.Pass());
   }
 
+  void CreateAndConnectCustomResponseWithCookies(
+      const std::string& socket_url,
+      const std::string& socket_host,
+      const std::string& socket_path,

[Ryan Sleevi, 2015/01/24 04:13:07]

Why is this not simply a single GURL?

[edit: I guess this is consistent with the existing tests, which I consider
fairly unreadable and unmaintainable. Not that the rest of //net is much better,
but I wish we could do better and we need to]

[Adam Rice, 2015/01/26 06:38:59]

Because then both the production code and the test would have a common point of
failure in the GURL code. Plus it is tempting to use GURL::host to get the value
for |socket_host|, but that isn't actually correct when port numbers are
involved. It's a specific case of the general maxim that "putting logic in tests
in bad".
It grew organically to the current state. Certainly it could use some
re-factoring.

[Ryan Sleevi, 2015/01/27 00:47:22]

I'm sorry, I don't understand this?

It's not your responsibility to test GURL. GURL should test GURL. You should
presume that GURL does the right thing (or conforms to your interface), and then
test to make sure that *your* code does the right thing and conforms to *your*
interface.
I'm sorry, but I think it's the opposite. You should strive to make sure your
tests conform to reasonable interfaces and meet the external behaviours,
regardless of the internal implementation.

Your argument about using GURL::host to get the value to |socket_host| doesn't
make sense. I could use GURL::querystring to get the value to |socket_host|, and
it'd be just as well a bug.

Using host() and port() explicitly from GURL seems better, not worse here.

[Adam Rice, 2015/01/27 06:32:46]

In principle, yes. However, if the implementation and the tests both consist of
the same code, then the tests are no longer testing anything.
The tests don't have to conform to anything. They need to be concise and
readable, so that

a) A maintainer unfamiliar with the code can look at a test failure and quickly
see what it means
b) Someone with some familiarity with the code or subject matter can browse
through the tests and efficiently determine what is and is not covered.
I doubt that anyone in the history of HTTP has ever put the query string in the
Host: header. Whereas leaving off the port, or specifying it wrongly, is
something that happens in HTTP implementations all the time.
We'd need some code like the following

  std::string socket_host = socket_url.host();
  if (socket_url.has_port())
    socket_host += ":" + socket_url.port();

That's logic. It needs tests. Fortunately, we can create
websocket_test_util_test.cc and that's probably what we'll do. Like I said, the
tests need refactoring, although why this is blocking fixing a crash bug I don't
quite understand.

[Ryan Sleevi, 2015/01/27 22:28:10]

Because quality matters. Because I've had to deal with a lot of teams acting
reactively towards bugs, rather than proactively towards debt. Because I think
it's important that when code is in front of you, it's important to call out
patterns and designs that don't make sense or add unnecessary complexity.

Yes, this is unfamiliar code, and I'm viewing in a limited context of a diff,
but the important feedback is and remains that "This is confusing, complex code,
and I have trouble reasoning about the correctness of _this fix_ precisely
because of it". That's the last place you want ot be when reactively fixing
something.

Please, ensure test cleanup is a high priority, and please, feel free to get
more //net owners involved in reviewing, so that people unfamiliar with the code
can share their impressions and confusion, since that overall helps raise the
quality and readability.

+      const std::vector<std::string>& sub_protocols,
+      const std::string& origin,
+      const std::string& cookies,
+      const std::string& extra_request_headers,
+      const std::string& response_body,
+      scoped_ptr<base::Timer> timer = scoped_ptr<base::Timer>()) {
+    url_request_context_host_.SetExpectations(
+        WebSocketStandardRequestWithCookies(socket_path, socket_host, origin,
+                                            cookies, extra_request_headers),
+        response_body);
+    CreateAndConnectStream(socket_url, sub_protocols, origin, timer.Pass());
+  }
+
   // |extra_request_headers| and |extra_response_headers| must end in "\r\n" or
   // errors like "Unable to perform synchronous IO while stopped" will occur.
   void CreateAndConnectStandard(
       const std::string& socket_url,
       const std::string& socket_host,
       const std::string& socket_path,
       const std::vector<std::string>& sub_protocols,
       const std::string& origin,
       const std::string& extra_request_headers,
       const std::string& extra_response_headers,
       scoped_ptr<base::Timer> timer = scoped_ptr<base::Timer>()) {
     CreateAndConnectCustomResponse(
         socket_url, socket_host, socket_path, sub_protocols, origin,
         extra_request_headers,
         WebSocketStandardResponse(extra_response_headers), timer.Pass());
   }
 
+  // |cookies| must be empty or end with "\r\n".
+  // |extra_request_headers| and |extra_response_headers| must end in "\r\n" or
+  // errors like "Unable to perform synchronous IO while stopped" will occur.

[Ryan Sleevi, 2015/01/24 04:13:08]

Both of these seem undesirable. Can this be expressed via compile-time
capabilities / assertions, rather than the comment that will inevitably be
ignored (because this file is huge)

[Adam Rice, 2015/01/26 06:38:59]

How? We can't make compile-time assertions about the contents of a std::string.

[Ryan Sleevi, 2015/01/27 00:47:22]

It seems entirely pointless to require "" or "something\r\n". If
!string.empty(), then this method could append "\r\n", so then the test author
needs only write "" or "something".

That is, requiring everyone to ensure "something\r\n" seems pointless,
especially since you never test that "something" fails (and it would be
unrelated to the test anyways)

[Adam Rice, 2015/01/27 06:32:46]

That's logic. Logic requires tests. At some point you have to stop putting logic
in your tests, or you'll have an infinite regression. Different people may draw
the line in different places. Personally I have no problem with a few "\r\n"s
here and there.

[Ryan Sleevi, 2015/01/27 22:28:10]

I'm sorry, I really have to disagree with you on your testing philosophy. I want
tests that are easy to read, easy to write, and easy to understand what's going
on.

Every single test has logic. Logic makes tests. The important thing is to not
_duplicate_ logic, and to make sure you're testing the _right_ interface.

I think it's absolutely unnecessary to require that the test author know to put
\r\n at the end of every string, lest their test mysteriously fail. That's an
onerous requirement on test authors, and a burden for test readers, for what is
ultimately something so superfluous that it's something a test harness _should_
be helping with.

Logic *BELONGS* in tests. That's how tests test. It's about finding a balance.

+  void CreateAndConnectStandardWithCookies(
+      const std::string& socket_url,
+      const std::string& socket_host,
+      const std::string& socket_path,
+      const std::vector<std::string>& sub_protocols,
+      const std::string& origin,
+      const std::string& cookies,
+      const std::string& extra_request_headers,
+      const std::string& extra_response_headers,
+      scoped_ptr<base::Timer> timer = scoped_ptr<base::Timer>()) {
+    CreateAndConnectCustomResponseWithCookies(
+        socket_url, socket_host, socket_path, sub_protocols, origin, cookies,
+        extra_request_headers,
+        WebSocketStandardResponse(extra_response_headers), timer.Pass());
+  }
+
   void CreateAndConnectRawExpectations(
       const std::string& socket_url,
       const std::vector<std::string>& sub_protocols,
       const std::string& origin,
       scoped_ptr<DeterministicSocketData> socket_data,
       scoped_ptr<base::Timer> timer = scoped_ptr<base::Timer>()) {
     AddRawExpectations(socket_data.Pass());
     CreateAndConnectStream(socket_url, sub_protocols, origin, timer.Pass());
   }
 
@@ skipping 255 matching lines @@
   };
 
   scoped_ptr<base::HistogramSamples> GetSamples(const std::string& name) {
     base::HistogramBase* histogram =
         base::StatisticsRecorder::FindHistogram(name);
     return histogram ? histogram->SnapshotSamples()
                      : scoped_ptr<base::HistogramSamples>();
   }
 };
 
+struct WebSocketStreamUseCookieTestParameter {

[Ryan Sleevi, 2015/01/24 04:13:08]

Document & Pluralize (Parameters)

+  WebSocketStreamUseCookieTestParameter(const GURL& url,

[Ryan Sleevi, 2015/01/24 04:13:07]

Document

+                                        const GURL& cookie_url,
+                                        const std::string& origin,
+                                        const std::string& cookie_line,
+                                        const std::string& cookies)
+      : url(url),
+        cookie_url(cookie_url),
+        origin(origin),
+        cookie_line(cookie_line),
+        cookies(cookies) {}
+
+  const GURL url;
+  const GURL cookie_url;
+  const std::string origin;
+  const std::string cookie_line;
+  const std::string cookies;

[Ryan Sleevi, 2015/01/24 04:13:07]

Making these const prevents operator=, which would be quite useful. I'm not sure
it's worth the const-ness to do this.

+};
+
+class WebSocketStreamUseCookieTest
+    : public WebSocketStreamCreateTest,
+      public ::testing::WithParamInterface<
+          WebSocketStreamUseCookieTestParameter> {
+ public:
+  void SetCookie(CookieStore* store,
+                 const GURL& url,
+                 const std::string& cookie_line) {
+    class RunLoop {

[Ryan Sleevi, 2015/01/24 04:13:07]

This is deeply confusing, because it shadows a name in another namespace *that
it's directly interacting with*.

+     public:
+      void Run() { run_loop_.Run(); }
+      void Quit(bool) { run_loop_.Quit(); }
+
+     private:
+      base::RunLoop run_loop_;
+    };
+
+    RunLoop run_loop;
+    store->SetCookieWithOptionsAsync(
+        url, cookie_line, CookieOptions(),
+        base::Bind(&RunLoop::Quit, base::Unretained(&run_loop)));

[Ryan Sleevi, 2015/01/24 04:13:07]

You should be using
https://code.google.com/p/chromium/codesearch#chromium/src/base/run_loop.h&l=71
instead of creating a custom object.

[Adam Rice, 2015/01/26 06:38:59]

We need to swallow the bool parameter that SetCookieWithOptionsAsync is passing
to the callback somehow.

[Ryan Sleevi, 2015/01/27 00:47:22]

Then just use a bound free function

static void IgnoreBoolArg(const base::Closure& closure, bool unused) {
  return closure.Run();
}

Base::Bind(&IgnoreBoolArg, run_loop_->QuitClosure());

+    run_loop.Run();
+  }
+};
+
+class WebSocketStreamSetCookieTest
+    : public WebSocketStreamCreateTest,
+      public ::testing::WithParamInterface<
+          WebSocketStreamUseCookieTestParameter> {
+ public:
+  std::string GetCookies(CookieStore* store, const GURL& url) {
+    struct GetCookiesHelper {
+      void Run() { run_loop_.Run(); }
+      void Quit(const std::string& cookies) {
+        cookies_ = cookies;
+        run_loop_.Quit();
+      }
+
+      const std::string& cookies() const { return cookies_; }
+
+     private:
+      base::RunLoop run_loop_;
+      std::string cookies_;
+    };
+    GetCookiesHelper helper;
+
+    store->GetCookiesWithOptionsAsync(
+        url, CookieOptions(),
+        base::Bind(&GetCookiesHelper::Quit, base::Unretained(&helper)));

[Ryan Sleevi, 2015/01/24 04:13:08]

static void StashCookiesHelper(const base::Closure& task, const
base::WeakPtr<std::string>& weak_cookies, const std::string& incoming_cookies) {
  if (weak_cookies.get())
    *weak_cookies = incoming_cookies;
  PostTask(task);
}

std::string cookies;
base::WeakPtrFactory<std::string> weak_cookies(&cookies);
base::Bind(&StashCookiesHelper, run_loop.QuitClosure(),
weak_cookies.GetWeakPtr())


Benefits: If the message loop gets quit because of some other reason, things
don't blow up if the task is still pending. And you avoid the class helper and
base::Unretained(), which are non-idiomatic Chromium threading.

+    helper.Run();
+    return helper.cookies();
+  }
+};
+
 // Confirm that the basic case works as expected.
 TEST_F(WebSocketStreamCreateTest, SimpleSuccess) {
   CreateAndConnectStandard("ws://localhost/", "localhost", "/",
                            NoSubProtocols(), "http://localhost", "", "");
   EXPECT_FALSE(request_info_);
   EXPECT_FALSE(response_info_);
   RunUntilIdle();
   EXPECT_FALSE(has_failed());
   EXPECT_TRUE(stream_);
   EXPECT_TRUE(request_info_);
@@ skipping 945 matching lines @@
   scoped_ptr<base::HistogramSamples> samples(GetSamples(name));
   ASSERT_TRUE(samples);
   if (original) {
     samples->Subtract(*original);  // Cancel the original values.
   }
   EXPECT_EQ(1, samples->GetCount(INCOMPLETE));
   EXPECT_EQ(0, samples->GetCount(CONNECTED));
   EXPECT_EQ(0, samples->GetCount(FAILED));
 }
 
+TEST_P(WebSocketStreamUseCookieTest, UseCookie) {
+  // In case we use a TLS connection.

[Ryan Sleevi, 2015/01/24 04:13:07]

This comment doesn't make sense. It sounds like you're uncertain whether it's
necessary, but it is (judging by your test below)

+  ssl_data_.push_back(new SSLSocketDataProvider(ASYNC, OK));
+
+  CookieStore* store =
+      url_request_context_host_.GetURLRequestContext()->cookie_store();
+
+  const GURL& url = GetParam().url;
+  const GURL& cookie_url = GetParam().cookie_url;
+  const std::string& origin = GetParam().origin;
+  const std::string& cookie_line = GetParam().cookie_line;
+  const std::string& cookies = GetParam().cookies;
+
+  SetCookie(store, cookie_url, cookie_line);
+
+  CreateAndConnectStandardWithCookies(url.spec(), url.host(), url.path(),
+                                      NoSubProtocols(), origin, cookies, "",
+                                      "");

[Ryan Sleevi, 2015/01/24 04:13:08]

s/""/std::string()/

+
+  RunUntilIdle();
+  EXPECT_FALSE(has_failed());
+}
+
+INSTANTIATE_TEST_CASE_P(
+    WebSocketStreamUseCookieTest,
+    WebSocketStreamUseCookieTest,
+    ::testing::Values(
+        // Non-secure cookies for ws
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),

[Ryan Sleevi, 2015/01/24 04:13:07]

If you deal with the constness I mentioned above, you should be able to use the
simple { } struct initialization sequence.

::testing::Values<MyType>(
  { GURL("ws://..."),
    GURL("http://..."),
    ...
  },
  { GURL("ws://...") }
)

etc

[yhirano, 2015/01/26 07:01:40]

IIUC having constant members is not related to initialization. As Adam pointed,
we can use {} initialization if we make the parameter POD class (in C++03). On
the other hand, in C++11, the uniform initialization syntax is introduced and
it's working on my local environment, though it is currently banned (See
http://chromium-cpp.appspot.com/). I would rather keep this until the uniform
initialization syntax is allowed in the project.
I'm not a C++ guru, so please correct me if I'm wrong.

[Ryan Sleevi, 2015/01/27 00:47:22]

Using { } to initialize POD structs is not a C++11 construct. The C++11
construct is using { } to initialize complex types (e.g. a vector). Here, what
we're initializing is a constant array, which uses { } syntax, with a structure,
which uses { }. The { } applies regardless of POD-ness, provided that each
member not elaborated in the list follows a default-initializer rule (e.g. even
std::string() can be default-initialized by calling the no-arg constructor)

The issue with const-ness is now ::testing::Values() can behave with that array,
which is why it was easier to not explicitly specify it, but you may be able to
get away with it.

[Adam Rice, 2015/01/27 06:32:46]

Indeed. Brace-initialisation of function arguments is not valid before C++11,
and so is forbidden.

If the initialisation is taken out of the function then it would be a global
array, which would have to be of POD type to be permitted by the style guide.

[Ryan Sleevi, 2015/01/27 22:28:10]

This hasn't applied to unit tests historically, but can be trivially dealt with
by making the class POD, which introduces *no* functional/logic change (using
your criteria)

struct WebSocketStreamUseCookieTestParameters {
  const char* url;
  const char* cookie_url;
  const char* origin;
  const char* cookie_line;
  const char* cookies;
};

Then just:

const GURL url(GetParam().url);
const GURL cookie_url(GetParam().cookie_url);
std::string cookie_line(GetParam().cookie_line);
if (!cookie_line.empty())
  cookie_line.append("\r\n");

SetCookie(store, cookie_url, cookie_line);
CreateAndConnectStandardWithCookies(url.spec(), url.host(), url.path(),
                                    NoSubProtocols(), GetParam().origin,
                                    GetParam().cookies, std::string(),
                                    std::string());

then your test simply becomes

static const WebSocketStreamUseCookieTestParameters kTestCookies[] = {
  { "ws://www.example.com",
    "http://www.example.com",
    "http://www.example.com",
    "test-cookie",
    "Cookie: test-cookie" },
  { ... },
  { ... },
};

INSTANTIATE_TEST_CASE_P(
  ...,
  ::testing::ValuesIn(kTestCookies));

+                                              GURL("http://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),

[Ryan Sleevi, 2015/01/24 04:13:08]

The amount of copy-pasta for thees \r\n makes it seem like you should just be
handling it in your class initializer, unless you're explicitly trying to test
bad cookies (and you should be)

+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("https://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("ws://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("wss://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        // Non-secure cookies for wss
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("http://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("https://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("ws://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("wss://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Cookie: test-cookie\r\n"),
+
+        // Secure-cookies for ws
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("https://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie; secure",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("wss://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie; secure",
+                                              ""),
+
+        // Secure-cookies for wss
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("https://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie; secure",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("wss://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie; secure",
+                                              "Cookie: test-cookie\r\n"),
+
+        // Non-secure cookies for ws (sharing domain)
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("http://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("https://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("ws://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("wss://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        // Non-secure cookies for wss (sharing domain)
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("http://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("https://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("ws://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("wss://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie; Domain=example.com",
+                                              "Cookie: test-cookie\r\n"),
+
+        // Secure-cookies for ws (sharing domain)
+        WebSocketStreamUseCookieTestParameter(
+            GURL("ws://www.example.com"),
+            GURL("https://www2.example.com"),
+            "http://www.example.com",
+            "test-cookie; Domain=example.com; secure",
+            ""),

[Ryan Sleevi, 2015/01/24 04:13:08]

s/""/std::string()/

+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("ws://www.example.com"),
+            GURL("wss://www2.example.com"),
+            "http://www.example.com",
+            "test-cookie; Domain=example.com; secure",
+            ""),
+
+        // Secure-cookies for wss (sharing domain)
+        WebSocketStreamUseCookieTestParameter(
+            GURL("wss://www.example.com"),
+            GURL("https://www2.example.com"),
+            "https://www.example.com",
+            "test-cookie; Domain=example.com; secure",
+            "Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("wss://www.example.com"),
+            GURL("wss://www2.example.com"),
+            "https://www.example.com",
+            "test-cookie; Domain=example.com; secure",
+            "Cookie: test-cookie\r\n"),
+
+        // Non-matching cookies for ws
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("http://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("https://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("ws://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("wss://www2.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        // Non-matching cookies for wss
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("http://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("https://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("ws://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              ""),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("wss://www2.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "")));
+
+TEST_P(WebSocketStreamSetCookieTest, SetCookie) {
+  // In case we use a TLS connection.
+  ssl_data_.push_back(new SSLSocketDataProvider(ASYNC, OK));
+
+  const GURL& url = GetParam().url;
+  const GURL& cookie_url = GetParam().cookie_url;
+  const std::string& origin = GetParam().origin;
+  const std::string& cookie_line = GetParam().cookie_line;
+  const std::string& cookies = GetParam().cookies;
+
+  const std::string response = base::StringPrintf(
+      "HTTP/1.1 101 Switching Protocols\r\n"
+      "Upgrade: websocket\r\n"
+      "Connection: Upgrade\r\n"
+      "%s"
+      "Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n"
+      "\r\n",
+      cookies.c_str());
+
+  CookieStore* store =
+      url_request_context_host_.GetURLRequestContext()->cookie_store();
+
+  CreateAndConnectCustomResponse(url.spec(), url.host(), url.path(),
+                                 NoSubProtocols(), origin, "", response);
+
+  RunUntilIdle();
+  EXPECT_FALSE(has_failed());
+
+  EXPECT_EQ(cookie_line, GetCookies(store, cookie_url));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    WebSocketStreamSetCookieTest,
+    WebSocketStreamSetCookieTest,
+    ::testing::Values(
+        // Cookies for ws

[Adam Rice, 2015/01/23 12:58:00]

Nitpick: the cookie comes *from* ws (or is "set by" ws) and then is looked up
via some other protocol.

+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("http://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("https://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("ws://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("wss://www.example.com"),
+                                              "http://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        // Cookies for wss

[Adam Rice, 2015/01/23 12:58:00]

Maybe we should test that the "Secure" attribute works when using Set-Cookie
from wss? I'm not sure, because obviously since we use the same code path as
HTTP it has to work. But it would be more thorough to test it. We might also
want to test that the "Path" attribute works correctly... or not? I don't know.

+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("http://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("https://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("ws://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("wss://www.example.com"),
+                                              "https://www.example.com",
+                                              "test-cookie",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        // cookies for ws (sharing domain)
+        WebSocketStreamUseCookieTestParameter(
+            GURL("ws://www.example.com"),
+            GURL("http://www2.example.com"),
+            "http://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("ws://www.example.com"),
+            GURL("https://www2.example.com"),
+            "http://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("ws://www.example.com"),
+            GURL("ws://www2.example.com"),
+            "http://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("ws://www.example.com"),
+            GURL("wss://www2.example.com"),
+            "http://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        // cookies for wss (sharing domain)
+        WebSocketStreamUseCookieTestParameter(
+            GURL("wss://www.example.com"),
+            GURL("http://www2.example.com"),
+            "https://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("wss://www.example.com"),
+            GURL("https://www2.example.com"),
+            "https://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("wss://www.example.com"),
+            GURL("ws://www2.example.com"),
+            "https://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(
+            GURL("wss://www.example.com"),
+            GURL("wss://www2.example.com"),
+            "https://www.example.com",
+            "test-cookie",
+            "Set-Cookie: test-cookie; Domain=example.com\r\n"),
+
+        // Non-matching cookies for ws
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("http://www2.example.com"),
+                                              "http://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("https://www2.example.com"),
+                                              "http://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("ws://www2.example.com"),
+                                              "http://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("ws://www.example.com"),
+                                              GURL("wss://www2.example.com"),
+                                              "http://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        // Non-matching cookies for wss
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("http://www2.example.com"),
+                                              "https://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("https://www2.example.com"),
+                                              "https://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("ws://www2.example.com"),
+                                              "https://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n"),
+
+        WebSocketStreamUseCookieTestParameter(GURL("wss://www.example.com"),
+                                              GURL("wss://www2.example.com"),
+                                              "https://www.example.com",
+                                              "",
+                                              "Set-Cookie: test-cookie\r\n")));
+
 }  // namespace
 }  // namespace net