Refactor CastSocket code for the following:

chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc

Index: chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc
===================================================================
--- chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc	(revision 240465)
+++ chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc	(working copy)
@@ -4,6 +4,10 @@
 
 #include "chrome/browser/extensions/api/cast_channel/cast_socket.h"
 
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/sys_byteorder.h"
 #include "chrome/browser/extensions/api/cast_channel/cast_channel.pb.h"
 #include "chrome/browser/extensions/api/cast_channel/cast_message_util.h"
 #include "net/base/address_list.h"
@@ -24,6 +28,15 @@
 using ::testing::Return;
 using ::testing::SaveArg;
 
+namespace {
+const char* kTestData[4] = {
+    "Hello, World!",
+    "Goodbye, World!",
+    "Hello, Sky!",
+    "Goodbye, Volcano!",
+};
+}  // namespace
+
 namespace extensions {
 namespace api {
 namespace cast_channel {
@@ -36,37 +49,45 @@
                                const MessageInfo& message));
 };
 
-class MockTCPClientSocket : public net::TCPClientSocket {
+class MockTCPSocket : public net::TCPClientSocket {
  public:
-  explicit MockTCPClientSocket() :
-      TCPClientSocket(net::AddressList(), NULL, net::NetLog::Source()) { }
-  virtual ~MockTCPClientSocket() { }
+  explicit MockTCPSocket(const net::MockConnect& connect_data) :
+      TCPClientSocket(net::AddressList(), NULL, net::NetLog::Source()),
+      connect_data_(connect_data) { }
 
-  MOCK_METHOD1(Connect, int(const net::CompletionCallback& callback));
-  MOCK_METHOD2(SetKeepAlive, bool(bool, int));
-  MOCK_METHOD1(SetNoDelay, bool(bool));
-  MOCK_METHOD3(Read, int(net::IOBuffer* buf, int buf_len,
-                         const net::CompletionCallback& callback));
-  MOCK_METHOD3(Write, int(net::IOBuffer* buf, int buf_len,
-                          const net::CompletionCallback& callback));
-  MOCK_METHOD0(Disconnect, void());
-};
+  virtual int Connect(const net::CompletionCallback& callback) OVERRIDE {
+    if (connect_data_.mode == net::ASYNC) {
+      CHECK_NE(connect_data_.result, net::ERR_IO_PENDING);
+      base::MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(callback, connect_data_.result));
+      return net::ERR_IO_PENDING;
+    } else {
+      return connect_data_.result;
+    }
+  }
 
-class MockSSLClientSocket : public net::MockClientSocket {
- public:
-  MockSSLClientSocket() : MockClientSocket(net::BoundNetLog()) { }
-  virtual ~MockSSLClientSocket() { }
+  virtual bool SetKeepAlive(bool a, int b) OVERRIDE {

[mark a. foltz, 2013/12/13 01:02:02]

Please preserve parameter names from the original declaration so the reader can
tell what they mean, without having to hunt for that declaration.

[Munjal (Google), 2013/12/14 00:33:42]

Done.

+    // Always return true in tests
+    return true;
+  }
 
-  MOCK_METHOD1(Connect, int(const net::CompletionCallback& callback));
-  MOCK_METHOD3(Read, int(net::IOBuffer* buf, int buf_len,
-                         const net::CompletionCallback& callback));
-  MOCK_METHOD3(Write, int(net::IOBuffer* buf, int buf_len,
-                          const net::CompletionCallback& callback));
-  MOCK_METHOD0(Disconnect, void());
-  MOCK_CONST_METHOD0(WasEverUsed, bool());
-  MOCK_CONST_METHOD0(UsingTCPFastOpen, bool());
-  MOCK_CONST_METHOD0(WasNpnNegotiated, bool());
-  MOCK_METHOD1(GetSSLInfo, bool(net::SSLInfo*));
+  virtual bool SetNoDelay(bool b) OVERRIDE {

[mark a. foltz, 2013/12/13 01:02:02]

Ditto.

[Munjal (Google), 2013/12/14 00:33:42]

Done.

+    // Always return true in tests
+    return true;
+  }
+
+  MOCK_METHOD3(Read,
+               int(net::IOBuffer*, int, const net::CompletionCallback&));
+  MOCK_METHOD3(Write,
+               int(net::IOBuffer*, int, const net::CompletionCallback&));
+
+  virtual void Disconnect() OVERRIDE {
+    // Do nothing in tests
+  }
+
+ private:
+  net::MockConnect connect_data_;
 };
 
 class CompleteHandler {
@@ -95,106 +116,98 @@
 
   explicit TestCastSocket(MockCastSocketDelegate* delegate,
                           const std::string& url) :
-    CastSocket("abcdefg", GURL(url), delegate,
-               &capturing_net_log_),
-    mock_tcp_socket_(new MockTCPClientSocket()),
-    mock_ssl_socket_(new MockSSLClientSocket()),
-    owns_tcp_socket_(true),
-    owns_ssl_socket_(true),
-    extract_cert_result_(true),
-    send_auth_challenge_result_(net::ERR_IO_PENDING),
-    read_auth_challenge_reply_result_(net::ERR_IO_PENDING),
-    challenge_reply_result_(true) {
+      CastSocket("abcdefg", GURL(url), delegate,
+                 &capturing_net_log_),
+      ip_(CreateIPEndPoint()),
+      connect_index_(0),
+      extract_cert_result_(true),
+      verify_challenge_result_(true) {
   }
 
-  virtual ~TestCastSocket() {
-    if (owns_tcp_socket_) {
-      DCHECK(mock_tcp_socket_);
-      delete mock_tcp_socket_;
-    }
-    if (owns_ssl_socket_) {
-      DCHECK(mock_ssl_socket_);
-      delete mock_ssl_socket_;
-    }
+  static net::IPEndPoint CreateIPEndPoint() {
+    net::IPAddressNumber number;
+    number.push_back(192);
+    number.push_back(0);
+    number.push_back(0);
+    number.push_back(1);
+    return net::IPEndPoint(number, 8009);
   }
 
-  virtual void Close(const net::CompletionCallback& callback) OVERRIDE {
-    if (!owns_tcp_socket_)
-      mock_tcp_socket_ = NULL;
-    if (!owns_ssl_socket_)
-      mock_ssl_socket_ = NULL;
-    CastSocket::Close(callback);
+  virtual ~TestCastSocket() {
   }
 
-  void CreateNewSockets() {
-    owns_tcp_socket_ = true;
-    mock_tcp_socket_ = new MockTCPClientSocket();
-    owns_ssl_socket_ = true;
-    mock_ssl_socket_ = new MockSSLClientSocket();
+  // Helpers to set mock results for various operations.
+  void SetupTcp1Connect(net::IoMode mode, int result) {
+    tcp_connect_data_[0].reset(new net::MockConnect(mode, result));
   }
-
+  void SetupSsl1Connect(net::IoMode mode, int result) {
+    ssl_connect_data_[0].reset(new net::MockConnect(mode, result));
+  }
+  void SetupTcp2Connect(net::IoMode mode, int result) {
+    tcp_connect_data_[1].reset(new net::MockConnect(mode, result));
+  }
+  void SetupSsl2Connect(net::IoMode mode, int result) {
+    ssl_connect_data_[1].reset(new net::MockConnect(mode, result));
+  }
+  void AddWriteResult(const net::MockWrite& write) {
+    writes_.push_back(write);
+  }
+  void AddReadResult(const net::MockRead& read) {
+    reads_.push_back(read);
+  }
   void SetExtractCertResult(bool value) {
     extract_cert_result_ = value;
   }
-
-  void SetSendAuthChallengeResult(int result) {
-    send_auth_challenge_result_ = result;
+  void SetVerifyChallengeResult(bool value) {
+    verify_challenge_result_ = value;
   }
 
-  void SetReadAuthChallengeReplyResult(int result) {
-    read_auth_challenge_reply_result_ = result;
+ private:
+  virtual scoped_ptr<net::TCPClientSocket> CreateTcpSocket() OVERRIDE {
+    net::MockConnect* connect_data = tcp_connect_data_[connect_index_].get();
+    connect_data->peer_addr = ip_;
+    return scoped_ptr<net::TCPClientSocket>(new MockTCPSocket(*connect_data));
   }
 
-  void SetChallengeReplyResult(bool value) {
-    challenge_reply_result_ = value;
-  }
+  virtual scoped_ptr<net::SSLClientSocket> CreateSslSocket(
+      scoped_ptr<net::StreamSocket> socket) OVERRIDE {
+    net::MockConnect* connect_data = ssl_connect_data_[connect_index_].get();
+    connect_data->peer_addr = ip_;
+    ++connect_index_;
 
-  MockTCPClientSocket* mock_tcp_socket_;
-  MockSSLClientSocket* mock_ssl_socket_;
-
- protected:
-  virtual scoped_ptr<net::TCPClientSocket> CreateTcpSocket() OVERRIDE {
-    owns_tcp_socket_ = false;
-    return scoped_ptr<net::TCPClientSocket>(mock_tcp_socket_);
+    ssl_data_.reset(new net::StaticSocketDataProvider(

[mark a. foltz, 2013/12/13 01:02:02]

This is a weird API.

- Why doesn't the SSDP have reads_ and writes_ members? 
- Why is it taking a pointer to the vectors' internal array?  This violates
abstraction and creates shared ownership that's probably not necessary.

[Munjal (Google), 2013/12/14 00:33:42]

SSDP requires array of reads and writes in the ctor. It does not have any public
methods to add those later since (I think) it is an array internally.

The reason we use a vector is that SSDP does not take ownership of the arrays we
pass in. It wants us to keep the ownership. So we have ot anyway use some data
structure to deallcoate those arrays after tests are done. I chose vector
instead fo scoped_array so that the tests get semantics of AddXResult(...)
instead of having to pass in all the results at once. This makes the tests more
readable.

+        reads_.data(), reads_.size(), writes_.data(), writes_.size()));
+    ssl_data_->set_connect_data(*connect_data);
+    return scoped_ptr<net::SSLClientSocket>(
+        new net::MockTCPClientSocket(

[mark a. foltz, 2013/12/13 01:02:02]

Did you mean MockTCPSocket?

[Munjal (Google), 2013/12/14 00:33:42]

No. net::MockTCPSocket (in socket_test_util.h/.cc) inherits from
net::SSLClientSocket. It is bad naming but that is how it is. I added a comment
to that regard though to make it clear that it is intentional.

+            net::AddressList(), &capturing_net_log_, ssl_data_.get()));
   }
 
-  virtual scoped_ptr<net::SSLClientSocket> CreateSslSocket() OVERRIDE {
-    owns_ssl_socket_ = false;
-    return scoped_ptr<net::SSLClientSocket>(mock_ssl_socket_);
-  }
-
   virtual bool ExtractPeerCert(std::string* cert) OVERRIDE {
     if (extract_cert_result_)
       cert->assign("dummy_test_cert");
     return extract_cert_result_;
   }
 
-  virtual int SendAuthChallenge() OVERRIDE {
-    return send_auth_challenge_result_;
-  }
-
-  virtual int ReadAuthChallengeReply() OVERRIDE {
-    return read_auth_challenge_reply_result_;
-  }
-
   virtual bool VerifyChallengeReply() OVERRIDE {
-    return challenge_reply_result_;
+    return verify_challenge_result_;
   }
 
- private:
   net::CapturingNetLog capturing_net_log_;
-  // Whether this object or the parent owns |mock_tcp_socket_|.
-  bool owns_tcp_socket_;
-  // Whether this object or the parent owns |mock_ssl_socket_|.
-  bool owns_ssl_socket_;
+  net::IPEndPoint ip_;
+  // Simulated connect data
+  scoped_ptr<net::MockConnect> tcp_connect_data_[2];
+  scoped_ptr<net::MockConnect> ssl_connect_data_[2];

[mark a. foltz, 2013/12/13 01:02:02]

It's unfortunate the use of the net:: mocks requires all this extra bookkeeping
(in essence this is reinventing GMock).

[Munjal (Google), 2013/12/14 00:33:42]

Actually this particular array of two sockets is not due totheuse of net::
mocks. This is to avoid the weird ownership model in the old code where it kept
a flag (like owns_ssl_socket_) to figure out whether we own the socket or not.
The new code fixes that because we don't need to hold on to the socket pointer
to set expectations on it or invoke callbacks.  So in that regard the new
approach is better.

+  // Simulated read / write data
+  std::vector<net::MockWrite> writes_;
+  std::vector<net::MockRead> reads_;
+  scoped_ptr<net::SocketDataProvider> ssl_data_;
+  // Number of times Connect method is called
+  size_t connect_index_;
   // Simulated result of peer cert extraction.
   bool extract_cert_result_;
-  // Simulated result to be returned by SendAuthChallenge.
-  int send_auth_challenge_result_;
-  // Simulated result to be returned by ReadAuthChallengeReply.
-  int read_auth_challenge_reply_result_;
   // Simulated result of verifying challenge reply.
-  bool challenge_reply_result_;
+  bool verify_challenge_result_;
 };
 
 class CastSocketTest : public testing::Test {
@@ -203,11 +216,30 @@
   virtual ~CastSocketTest() {}
 
   virtual void SetUp() OVERRIDE {
-    test_message_.namespace_ = "urn:test";
-    test_message_.source_id = "1";
-    test_message_.destination_id = "2";
-    test_message_.data.reset(new base::StringValue("Hello, World!"));
-    ASSERT_TRUE(MessageInfoToCastMessage(test_message_, &test_proto_));
+    // Create a few test messages
+    for (size_t i = 0; i < arraysize(test_messages_); i++) {
+      CreateStringMessage(&test_messages_[i],
+                          "urn:cast", "1", "2", kTestData[i]);
+      ASSERT_TRUE(MessageInfoToCastMessage(
+          test_messages_[i], &test_protos_[i]));
+      ASSERT_TRUE(CastSocket::Serialize(test_protos_[i], &test_proto_strs_[i]));
+    }
+
+    // Create a test auth request.
+    CastMessage request;
+    CreateAuthChallengeMessage(&request);
+    ASSERT_TRUE(CastSocket::Serialize(request, &auth_request_));
+
+    // Create a test auth reply.
+    MessageInfo reply;
+    CreateBinaryMessage(&reply,
+                        "urn:x-cast:com.google.cast.tp.deviceauth",
+                        "sender-0",
+                        "receiver-0",
+                        "abcd");
+    CastMessage reply_msg;
+    ASSERT_TRUE(MessageInfoToCastMessage(reply, &reply_msg));
+    ASSERT_TRUE(CastSocket::Serialize(reply_msg, &auth_reply_));
   }
 
   virtual void TearDown() OVERRIDE {
@@ -215,7 +247,29 @@
     socket_->Close(base::Bind(&CompleteHandler::OnCloseComplete,
                               base::Unretained(&handler_)));
   }
+  static void CreateStringMessage(MessageInfo* msg,
+                                  std::string ns,
+                                  std::string src,
+                                  std::string dst,
+                                  std::string data) {
+    msg->namespace_ = ns;
+    msg->source_id = src;
+    msg->destination_id = dst;
+    msg->data.reset(new base::StringValue(data));
+  }
 
+  static void CreateBinaryMessage(MessageInfo* msg,
+                                  std::string ns,
+                                  std::string src,
+                                  std::string dst,
+                                  std::string data) {
+    msg->namespace_ = ns;
+    msg->source_id = src;
+    msg->destination_id = dst;
+    msg->data.reset(base::BinaryValue::CreateWithCopiedBuffer(
+        data.c_str(), data.size()));
+  }
+
   void CreateCastSocket() {
     socket_ = TestCastSocket::Create(&mock_delegate_);
   }
@@ -224,85 +278,36 @@
     socket_ = TestCastSocket::CreateSecure(&mock_delegate_);
   }
 
-  // Sets an expectation that TCPClientSocket::Connect is called and
-  // returns |result| and stores the callback passed to it in |callback|.
-  void ExpectTcpConnect(net::CompletionCallback* callback, int result) {
-    EXPECT_CALL(mock_tcp_socket(), Connect(A<const net::CompletionCallback&>()))
-        .Times(1)
-        .WillOnce(DoAll(SaveArg<0>(callback), Return(result)));
-    EXPECT_CALL(mock_tcp_socket(), SetKeepAlive(_, _))
-        .WillOnce(Return(true));
-  }
-
-  // Same as ExpectTcpConnect but to return net::ERR_IO_PENDING.
-  void ExpectTcpConnectPending(net::CompletionCallback* callback) {
-    ExpectTcpConnect(callback, net::ERR_IO_PENDING);
-  }
-
-  // Sets an expectation that SSLClientSocket::Connect is called and
-  // returns |result| and stores the callback passed to it in |callback|.
-  void ExpectSslConnect(net::CompletionCallback* callback, int result) {
-    EXPECT_CALL(mock_ssl_socket(), Connect(A<const net::CompletionCallback&>()))
-        .Times(1)
-        .WillOnce(DoAll(SaveArg<0>(callback), Return(result)));
-  }
-
-  // Same as ExpectSslConnect but to return net::ERR_IO_PENDING.
-  void ExpectSslConnectPending(net::CompletionCallback* callback) {
-    ExpectSslConnect(callback, net::ERR_IO_PENDING);
-  }
-
-  // Sets an expectation that SSLClientSocket::Read is called |times| number
-  // of times and returns net::ERR_IO_PENDING.
-  void ExpectSslRead(int times) {
-    EXPECT_CALL(mock_ssl_socket(), Read(A<net::IOBuffer*>(),
-                                        A<int>(),
-                                        A<const net::CompletionCallback&>()))
-        .Times(times)
-        .WillOnce(Return(net::ERR_IO_PENDING));
-  }
-
   // Sets up CastSocket::Connect to succeed.
-  // Connecting the socket also starts the read loop; we expect the call to
-  // Read(), but never fire the read callback.
+  // Connecting the socket also starts the read loop; so we add a mock
+  // read result that returns IO_PENDING and callback is never fired.
   void ConnectHelper() {
-    net::CompletionCallback connect_callback1;
-    net::CompletionCallback connect_callback2;
+    socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+    socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::OK);
+    socket_->AddReadResult(net::MockRead(net::ASYNC, net::ERR_IO_PENDING));
 
-    ExpectTcpConnect(&connect_callback1, net::OK);
-    ExpectSslConnect(&connect_callback2, net::OK);
     EXPECT_CALL(handler_, OnConnectComplete(net::OK));
-    ExpectSslRead(1);
-
     socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
                                 base::Unretained(&handler_)));
-
-    EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
-    EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+    RunPendingTasks();
   }
 
  protected:
-  MockTCPClientSocket& mock_tcp_socket() {
-    MockTCPClientSocket* mock_socket = socket_->mock_tcp_socket_;
-    DCHECK(mock_socket);
-    return *mock_socket;
+  // Runs all pending tasks in the message loop.
+  void RunPendingTasks() {
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
   }
 
-  MockSSLClientSocket& mock_ssl_socket() {
-    MockSSLClientSocket* mock_socket = socket_->mock_ssl_socket_;
-    DCHECK(mock_socket);
-    return *mock_socket;
-  }
-
-  void CallOnChallengeEvent(int result) {
-    socket_->OnChallengeEvent(result);
-  }
-
+  base::MessageLoop message_loop_;
   MockCastSocketDelegate mock_delegate_;
   scoped_ptr<TestCastSocket> socket_;
   CompleteHandler handler_;
-  MessageInfo test_message_;
-  CastMessage test_proto_;
+  MessageInfo test_messages_[arraysize(kTestData)];
+  CastMessage test_protos_[arraysize(kTestData)];
+  std::string test_proto_strs_[arraysize(kTestData)];
+  std::string auth_request_;
+  std::string auth_reply_;
 };
 
 // Tests URL parsing and validation.
@@ -335,6 +340,8 @@
 TEST_F(CastSocketTest, TestConnectAndClose) {
   CreateCastSocket();
   ConnectHelper();
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 
   EXPECT_CALL(handler_, OnCloseComplete(net::OK));
   socket_->Close(base::Bind(&CompleteHandler::OnCloseComplete,
@@ -348,19 +355,14 @@
 // - SSL connection succeeds (async)
 TEST_F(CastSocketTest, TestConnect) {
   CreateCastSocket();
+  socket_->SetupTcp1Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl1Connect(net::ASYNC, net::OK);
+  socket_->AddReadResult(net::MockRead(net::ASYNC, net::ERR_IO_PENDING));
 
-  net::CompletionCallback connect_callback1;
-  net::CompletionCallback connect_callback2;
-
-  ExpectTcpConnectPending(&connect_callback1);
-  ExpectSslConnectPending(&connect_callback2);
   EXPECT_CALL(handler_, OnConnectComplete(net::OK));
-  ExpectSslRead(1);
-
   socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
                               base::Unretained(&handler_)));
-  connect_callback1.Run(net::OK);
-  connect_callback2.Run(net::OK);
+  RunPendingTasks();
 
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
@@ -372,37 +374,19 @@
 // - Cert is extracted successfully
 // - Second TCP connection succeeds (async)
 // - Second SSL connection succeeds (async)
-TEST_F(CastSocketTest, TestTwoStepConnect) {
+TEST_F(CastSocketTest, TestConnectTwoStep) {
   CreateCastSocket();
+  socket_->SetupTcp1Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl1Connect(net::ASYNC, net::ERR_CERT_AUTHORITY_INVALID);
+  socket_->SetupTcp2Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl2Connect(net::ASYNC, net::OK);
+  socket_->AddReadResult(net::MockRead(net::ASYNC, net::ERR_IO_PENDING));
 
-  // Expectations for the initial connect call
-  net::CompletionCallback tcp_connect_callback1;
-  net::CompletionCallback ssl_connect_callback1;
-
-  ExpectTcpConnectPending(&tcp_connect_callback1);
-  ExpectSslConnectPending(&ssl_connect_callback1);
-
-  // Start connect flow
+  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
   socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
                               base::Unretained(&handler_)));
-  tcp_connect_callback1.Run(net::OK);
+  RunPendingTasks();
 
-  // Expectations for the second connect call
-  socket_->CreateNewSockets();
-  net::CompletionCallback tcp_connect_callback2;
-  net::CompletionCallback ssl_connect_callback2;
-  ExpectTcpConnectPending(&tcp_connect_callback2);
-  ExpectSslConnectPending(&ssl_connect_callback2);
-  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
-  ExpectSslRead(1);
-
-  // Trigger callbacks for the first connect
-  ssl_connect_callback1.Run(net::ERR_CERT_AUTHORITY_INVALID);
-
-  // Trigger callbacks for the second connect
-  tcp_connect_callback2.Run(net::OK);
-  ssl_connect_callback2.Run(net::OK);
-
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
@@ -414,379 +398,659 @@
 // - Second TCP connection succeeds (async)
 // - Second SSL connection fails (async)
 // - The flow should NOT be tried again
-TEST_F(CastSocketTest, TestMaxTwoConnectAttempts) {
+TEST_F(CastSocketTest, TestConnectMaxTwoAttempts) {
   CreateCastSocket();
+  socket_->SetupTcp1Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl1Connect(net::ASYNC, net::ERR_CERT_AUTHORITY_INVALID);
+  socket_->SetupTcp2Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl2Connect(net::ASYNC, net::ERR_CERT_AUTHORITY_INVALID);
 
-  net::CompletionCallback tcp_connect_callback1;
-  net::CompletionCallback ssl_connect_callback1;
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_CERT_AUTHORITY_INVALID));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
 
-  // Expectations for the initial connect call
-  ExpectTcpConnectPending(&tcp_connect_callback1);
-  ExpectSslConnectPending(&ssl_connect_callback1);
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
 
-  // Start connect flow
+// Tests that the following connection flow works:
+// - TCP connection succeeds (async)
+// - SSL connection fails with cert error (async)
+// - Cert is extracted successfully
+// - Second TCP connection succeeds (async)
+// - Second SSL connection succeeds (async)
+// - Challenge request is sent (async)
+// - Challenge response is received (async)
+// - Credentials are verified successfuly
+TEST_F(CastSocketTest, TestConnectFullSecureFlowAsync) {
+  CreateCastSocketSecure();
+
+  socket_->SetupTcp1Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl1Connect(net::ASYNC, net::ERR_CERT_AUTHORITY_INVALID);
+  socket_->SetupTcp2Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl2Connect(net::ASYNC, net::OK);
+  socket_->AddWriteResult(net::MockWrite(net::ASYNC, auth_request_.size()));
+  size_t body_size = auth_reply_.length() - 4;
+  const char* reply_data = auth_reply_.c_str();
+  socket_->AddReadResult(net::MockRead(net::ASYNC, reply_data, 4));
+  socket_->AddReadResult(net::MockRead(net::ASYNC, reply_data + 4, body_size));
+  socket_->AddReadResult(net::MockRead(net::ASYNC, net::ERR_IO_PENDING));
+
+  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
   socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
                               base::Unretained(&handler_)));
-  tcp_connect_callback1.Run(net::OK);
+  RunPendingTasks();
 
-  socket_->CreateNewSockets();
-  net::CompletionCallback tcp_connect_callback2;
-  net::CompletionCallback ssl_connect_callback2;
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
 
-  // Expectations for the second connect call
-  ExpectTcpConnectPending(&tcp_connect_callback2);
-  ExpectSslConnectPending(&ssl_connect_callback2);
-  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_CERT_AUTHORITY_INVALID));
+// Same as TestFullSecureConnectionFlowAsync, but operations are synchronous.
+TEST_F(CastSocketTest, TestConnectFullSecureFlowSync) {
+  CreateCastSocketSecure();
 
-  // Trigger callbacks for the first connect
-  ssl_connect_callback1.Run(net::ERR_CERT_AUTHORITY_INVALID);
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::ERR_CERT_AUTHORITY_INVALID);
+  socket_->SetupTcp2Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl2Connect(net::SYNCHRONOUS, net::OK);
+  socket_->AddWriteResult(net::MockWrite(
+      net::SYNCHRONOUS, auth_request_.size()));
+  size_t body_size = auth_reply_.length() - 4;

[mark a. foltz, 2013/12/13 01:02:02]

Please add a utility function for this expression that is repeated several times
below.

Would also prefer that you use the defined constant for the header size.

[Munjal (Google), 2013/12/14 00:33:42]

Done.

+  const char* reply_data = auth_reply_.c_str();
+  socket_->AddReadResult(net::MockRead(net::SYNCHRONOUS, reply_data, 4));
+  socket_->AddReadResult(net::MockRead(
+      net::SYNCHRONOUS, reply_data + 4, body_size));
+  socket_->AddReadResult(net::MockRead(net::ASYNC, net::ERR_IO_PENDING));
 
-  // Trigger callbacks for the second connect
-  tcp_connect_callback2.Run(net::OK);
-  ssl_connect_callback2.Run(net::ERR_CERT_AUTHORITY_INVALID);
+  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
 
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
+
+// Test connection error - TCP connect fails (async)
+TEST_F(CastSocketTest, TestConnectTcpConnectErrorAsync) {
+  CreateCastSocketSecure();
+
+  socket_->SetupTcp1Connect(net::ASYNC, net::ERR_FAILED);
+
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
 }
 
-// Test that when cert extraction fails the connection flow stops.
-TEST_F(CastSocketTest, TestCertExtractionFailure) {
-  CreateCastSocket();
+// Test connection error - TCP connect fails (sync)
+TEST_F(CastSocketTest, TestConnectTcpConnectErrorSync) {
+  CreateCastSocketSecure();
 
-  net::CompletionCallback connect_callback1;
-  net::CompletionCallback connect_callback2;
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::ERR_FAILED);
 
-  ExpectTcpConnectPending(&connect_callback1);
-  ExpectSslConnectPending(&connect_callback2);
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
 
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
+
+// Test connection error - SSL connect fails (async)
+TEST_F(CastSocketTest, TestConnectSslConnectErrorAsync) {
+  CreateCastSocketSecure();
+
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::ERR_FAILED);
+
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
   socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
                               base::Unretained(&handler_)));
-  connect_callback1.Run(net::OK);
+  RunPendingTasks();
 
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
+
+// Test connection error - SSL connect fails (async)
+TEST_F(CastSocketTest, TestConnectSslConnectErrorSync) {
+  CreateCastSocketSecure();
+
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::ASYNC, net::ERR_FAILED);
+
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
+
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
+
+// Test connection error - cert extraction error (async)
+TEST_F(CastSocketTest, TestConnectCertExtractionErrorAsync) {
+  CreateCastSocket();
+  socket_->SetupTcp1Connect(net::ASYNC, net::OK);
+  socket_->SetupSsl1Connect(net::ASYNC, net::ERR_CERT_AUTHORITY_INVALID);
+  // Set cert extraction to fail
+  socket_->SetExtractCertResult(false);
+
   EXPECT_CALL(handler_, OnConnectComplete(net::ERR_CERT_AUTHORITY_INVALID));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
 
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
+
+// Test connection error - cert extraction error (sync)
+TEST_F(CastSocketTest, TestConnectCertExtractionErrorSync) {
+  CreateCastSocket();
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::ERR_CERT_AUTHORITY_INVALID);
   // Set cert extraction to fail
   socket_->SetExtractCertResult(false);
-  // Attempt to connect results in ERR_CERT_AUTHORTY_INVALID
-  connect_callback2.Run(net::ERR_CERT_AUTHORITY_INVALID);
 
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_CERT_AUTHORITY_INVALID));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
 }
 
-// Tests that the following connection flow works:
-// - TCP connection succeeds (async)
-// - SSL connection fails with cert error (async)
-// - Cert is extracted successfully
-// - Second TCP connection succeeds (async)
-// - Second SSL connection succeeds (async)
-// - Challenge request is sent (async)
-// - Challenge response is received (async)
-// - Credentials are verified successfuly
-TEST_F(CastSocketTest, TestFullSecureConnectionFlowAsync) {
+// Test connectiion error - challenge send fails

[mark a. foltz, 2013/12/13 01:02:02]

connection, here and several places below.

[Munjal (Google), 2013/12/14 00:33:42]

Done.

+TEST_F(CastSocketTest, TestConnectChallengeSendError) {
   CreateCastSocketSecure();
 
-  net::CompletionCallback tcp_connect_callback1;
-  net::CompletionCallback ssl_connect_callback1;
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, net::ERR_FAILED));
 
-  // Expectations for the initial connect call
-  ExpectTcpConnectPending(&tcp_connect_callback1);
-  ExpectSslConnectPending(&ssl_connect_callback1);
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
 
-  // Start connect flow
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
+
+// Test connectiion error - challenge reply receive fails
+TEST_F(CastSocketTest, TestConnectChallengeReplyReceiveError) {
+  CreateCastSocketSecure();
+
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, net::OK));
+  socket_->AddReadResult(net::MockRead(net::SYNCHRONOUS, net::ERR_FAILED));
+
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
   socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
                               base::Unretained(&handler_)));
-  tcp_connect_callback1.Run(net::OK);
+  RunPendingTasks();
 
-  socket_->CreateNewSockets();
-  net::CompletionCallback tcp_connect_callback2;
-  net::CompletionCallback ssl_connect_callback2;
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
 
-  // Expectations for the second connect call
-  ExpectTcpConnectPending(&tcp_connect_callback2);
-  ExpectSslConnectPending(&ssl_connect_callback2);
-  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
+// Test connectiion error - challenge reply verification fails
+TEST_F(CastSocketTest, TestConnectChallengeVerificationFails) {
+  CreateCastSocketSecure();
 
-  // Trigger callbacks for the first connect
-  ssl_connect_callback1.Run(net::ERR_CERT_AUTHORITY_INVALID);
+  socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::OK);
+  socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, net::OK));
+  size_t body_size = auth_reply_.length() - 4;
+  const char* reply_data = auth_reply_.c_str();
+  socket_->AddReadResult(net::MockRead(net::SYNCHRONOUS, reply_data, 4));
+  socket_->AddReadResult(net::MockRead(
+      net::SYNCHRONOUS, reply_data + 4, body_size));
+  socket_->SetVerifyChallengeResult(false);
 
-  // Trigger callbacks for the second connect
-  tcp_connect_callback2.Run(net::OK);
-  ssl_connect_callback2.Run(net::OK);
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_FAILED));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
 
-  // Trigger callbacks for auth events.
-  CallOnChallengeEvent(net::OK);  // Sent challenge
-  CallOnChallengeEvent(net::OK);  // Received reply
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
+}
 
+// Test write success - single message (async)

[mark a. foltz, 2013/12/13 01:02:02]

The Write tests all share similar code to set up the test case.
Consider creating a subclass of CastSocketTest and factoring this code into its
Setup() method.

For bonus points you could move the test cases themselves into a base class and
have two subclasses (CastSocketAsyncWriteTest, CastSocketSyncWriteTest) that
pass the corresponding flag into the mock socket to test asynchronous or
synchronous completion.

[Munjal (Google), 2013/12/14 00:33:42]

Are you talking about the calls to AddWriteResult by "setup code"?

The reason I like the current style is that the test is mostly self-contained 
and readable. It is easy to figure out what the test is doing. If we abstract
too many things out of the test it will compromise that readability.

Having said that I added a couple of more helpers to more easily add write and
read results to the mock socket.

+TEST_F(CastSocketTest, TestWriteAsync) {
+  CreateCastSocket();
+  const std::string& msg = test_proto_strs_[0];
+  size_t msg_size = msg.size();
+  socket_->AddWriteResult(net::MockWrite(net::ASYNC, msg_size));
+  ConnectHelper();
+
+  EXPECT_CALL(handler_, OnWriteComplete(msg_size));
+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Same as TestFullSecureConnectionFlowAsync, but operations are  synchronous.
-TEST_F(CastSocketTest, TestFullSecureConnectionFlowSync) {
-  CreateCastSocketSecure();
+// Test write success - single message (sync)
+TEST_F(CastSocketTest, TestWriteSync) {
+  CreateCastSocket();
+  const std::string& msg = test_proto_strs_[0];
+  size_t msg_size = msg.size();
+  socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, msg_size));
+  ConnectHelper();
 
-  net::CompletionCallback tcp_connect_callback;
-  net::CompletionCallback ssl_connect_callback;
+  EXPECT_CALL(handler_, OnWriteComplete(msg_size));
+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+  RunPendingTasks();
 
-  // Expectations for the connect calls
-  ExpectTcpConnect(&tcp_connect_callback, net::OK);
-  ExpectSslConnect(&ssl_connect_callback, net::OK);
-  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
 
-  socket_->SetSendAuthChallengeResult(net::OK);
-  socket_->SetReadAuthChallengeReplyResult(net::OK);
+// Test write success - single message sent in multiple chunks (async)
+TEST_F(CastSocketTest, TestWriteChunkedAsync) {
+  CreateCastSocket();
+  const std::string& msg = test_proto_strs_[0];
+  size_t msg_size = msg.size();
+  size_t ch_size = 2;  // chunk size
+  for (size_t offset = 0; offset < msg_size; offset += ch_size) {
+    if (offset + ch_size > msg_size)
+      ch_size = msg_size - offset;
+    socket_->AddWriteResult(net::MockWrite(net::ASYNC, ch_size));
+  }
+  ConnectHelper();
 
-  // Start connect flow
-  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
-                              base::Unretained(&handler_)));
+  EXPECT_CALL(handler_, OnWriteComplete(msg_size));
+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+  RunPendingTasks();
 
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Tests writing a single message where the completion is signaled via
-// callback.
-TEST_F(CastSocketTest, TestWriteViaCallback) {
+// Test write success - single message sent in multiple chunks (sync)
+TEST_F(CastSocketTest, TestWriteChunkedSync) {
   CreateCastSocket();
+  const std::string& msg = test_proto_strs_[0];
+  size_t msg_size = msg.size();
+  size_t ch_size = 2;  // chunk size
+  for (size_t offset = 0; offset < msg_size; offset += ch_size) {
+    if (offset + ch_size > msg_size)
+      ch_size = msg_size - offset;
+    socket_->AddWriteResult(net::MockWrite(net::ASYNC, ch_size));
+  }
   ConnectHelper();
 
-  net::CompletionCallback write_callback;
-
-  EXPECT_CALL(mock_ssl_socket(),
-              Write(A<net::IOBuffer*>(),
-                    39,
-                    A<const net::CompletionCallback&>()))
-    .Times(1)
-    .WillOnce(DoAll(SaveArg<2>(&write_callback),
-                    Return(net::ERR_IO_PENDING)));
-  EXPECT_CALL(handler_, OnWriteComplete(39));
-  socket_->SendMessage(test_message_,
+  EXPECT_CALL(handler_, OnWriteComplete(msg_size));
+  socket_->SendMessage(test_messages_[0],
                        base::Bind(&CompleteHandler::OnWriteComplete,
                                   base::Unretained(&handler_)));
-  write_callback.Run(39);
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Tests writing a single message where the Write() returns directly.
-TEST_F(CastSocketTest, TestWrite) {
+// Test write success - multiple messages (async)
+TEST_F(CastSocketTest, TestWriteManyAsync) {
   CreateCastSocket();
+
+  for (size_t i = 0; i < arraysize(test_messages_); i++) {
+    size_t msg_size = test_proto_strs_[i].size();
+    socket_->AddWriteResult(net::MockWrite(net::ASYNC, msg_size));
+    EXPECT_CALL(handler_, OnWriteComplete(msg_size));
+  }
   ConnectHelper();
 
-  EXPECT_CALL(mock_ssl_socket(),
-              Write(A<net::IOBuffer*>(),
-                    39,
-                    A<const net::CompletionCallback&>()))
-    .Times(1)
-    .WillOnce(Return(39));
-  EXPECT_CALL(handler_, OnWriteComplete(39));
-  socket_->SendMessage(test_message_,
-                      base::Bind(&CompleteHandler::OnWriteComplete,
-                                 base::Unretained(&handler_)));
+  for (size_t i = 0; i < arraysize(test_messages_); i++) {
+    socket_->SendMessage(test_messages_[i],
+                         base::Bind(&CompleteHandler::OnWriteComplete,
+                                    base::Unretained(&handler_)));
+  }
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Tests writing multiple messages.
-TEST_F(CastSocketTest, TestWriteMany) {
+// Test write success - multiple messages (sync)
+TEST_F(CastSocketTest, TestWriteManySync) {
   CreateCastSocket();
-  ConnectHelper();
-  std::string messages[4];
-  messages[0] = "Hello, World!";
-  messages[1] = "Goodbye, World!";
-  messages[2] = "Hello, Sky!";
-  messages[3] = "Goodbye, Volcano!";
-  int sizes[4] = {39, 41, 37, 43};
-  MessageInfo message_info[4];
-  net::CompletionCallback write_callback;
 
-  for (int i = 0; i < 4; i++) {
-    EXPECT_CALL(mock_ssl_socket(),
-                Write(A<net::IOBuffer*>(),
-                      sizes[i],
-                      A<const net::CompletionCallback&>()))
-      .WillRepeatedly(DoAll(SaveArg<2>(&write_callback),
-                            Return(net::ERR_IO_PENDING)));
-    EXPECT_CALL(handler_, OnWriteComplete(sizes[i]));
+  for (size_t i = 0; i < arraysize(test_messages_); i++) {
+    size_t msg_size = test_proto_strs_[i].size();
+    socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, msg_size));
+    EXPECT_CALL(handler_, OnWriteComplete(msg_size));
   }
+  ConnectHelper();
 
-  for (int i = 0; i < 4; i++) {
-    message_info[i].namespace_ = "urn:test";
-    message_info[i].source_id = "1";
-    message_info[i].destination_id = "2";
-    message_info[i].data.reset(new base::StringValue(messages[i]));
-    socket_->SendMessage(message_info[i],
+  for (size_t i = 0; i < arraysize(test_messages_); i++) {
+    socket_->SendMessage(test_messages_[i],
                          base::Bind(&CompleteHandler::OnWriteComplete,
                                     base::Unretained(&handler_)));
   }
-  for (int i = 0; i < 4; i++) {
-    write_callback.Run(sizes[i]);
-  }
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Tests error on writing.
-TEST_F(CastSocketTest, TestWriteError) {
+// Test write error - not connected
+TEST_F(CastSocketTest, TestWriteErrorNotConnected) {
   CreateCastSocket();
+
+  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_FAILED));
+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+
+  EXPECT_EQ(cast_channel::READY_STATE_NONE, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
+
+// Test write error - very large message
+TEST_F(CastSocketTest, TestWriteErrorLargeMessage) {
+  CreateCastSocket();
   ConnectHelper();
-  net::CompletionCallback write_callback;
 
-  EXPECT_CALL(mock_ssl_socket(),
-              Write(A<net::IOBuffer*>(),
-                    39,
-                    A<const net::CompletionCallback&>()))
-    .Times(1)
-    .WillOnce(DoAll(SaveArg<2>(&write_callback),
-                    Return(net::ERR_SOCKET_NOT_CONNECTED)));
-  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_SOCKET_NOT_CONNECTED));
+  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_FAILED));
+  test_messages_[0].data.reset(
+      new base::StringValue(std::string(65537, 'a')));

[mark a. foltz, 2013/12/13 01:02:02]

FYI, the maximum length allowed for a message is 64KB.

This is enforced at the API layer though so it's okay to use a larger message
here.

[Munjal (Google), 2013/12/14 00:33:42]

Yeah, this is to test the case when the message is too large. It helps us test
one more code path in the implementation.

+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+
+}
+
+// Test write error - network error (sync)
+TEST_F(CastSocketTest, TestWriteNetworkErrorSync) {
+  CreateCastSocket();
+  socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, net::ERR_FAILED));
+  ConnectHelper();
+
+  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_FAILED));
   EXPECT_CALL(mock_delegate_,
               OnError(socket_.get(), cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
-  socket_->SendMessage(test_message_,
+  socket_->SendMessage(test_messages_[0],
                        base::Bind(&CompleteHandler::OnWriteComplete,
                                   base::Unretained(&handler_)));
+  RunPendingTasks();
+
   EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_SOCKET_ERROR, socket_->error_state());
 }
 
-// Tests reading a single message.
-TEST_F(CastSocketTest, TestRead) {
+// Test write error - network error (async)
+TEST_F(CastSocketTest, TestWriteErrorAsync) {
   CreateCastSocket();
+  socket_->AddWriteResult(net::MockWrite(net::ASYNC, net::ERR_FAILED));
+  ConnectHelper();
 
-  net::CompletionCallback connect_callback1;
-  net::CompletionCallback connect_callback2;
-  net::CompletionCallback read_callback;
+  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_FAILED));
+  EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(), cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+  RunPendingTasks();
 
-  std::string message_data;
-  ASSERT_TRUE(CastSocket::Serialize(test_proto_, &message_data));
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_SOCKET_ERROR, socket_->error_state());
+}
 
-  ExpectTcpConnect(&connect_callback1, net::ERR_IO_PENDING);
-  ExpectSslConnect(&connect_callback2, net::ERR_IO_PENDING);
+// Test write error - 0 bytes written should be considered an error
+TEST_F(CastSocketTest, TestWriteErrorZeroBytesWritten) {
+  CreateCastSocket();
+  socket_->AddWriteResult(net::MockWrite(net::SYNCHRONOUS, 0));
+  ConnectHelper();
 
-  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
-  EXPECT_CALL(mock_ssl_socket(), Read(A<net::IOBuffer*>(),
-                                      A<int>(),
-                                      A<const net::CompletionCallback&>()))
-      .Times(3)
-      .WillRepeatedly(DoAll(SaveArg<2>(&read_callback),
-                            Return(net::ERR_IO_PENDING)));
+  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_FAILED));
+  EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(), cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
+  socket_->SendMessage(test_messages_[0],
+                       base::Bind(&CompleteHandler::OnWriteComplete,
+                                  base::Unretained(&handler_)));
+  RunPendingTasks();
 
-  // Expect the test message to be read and invoke the delegate.
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_SOCKET_ERROR, socket_->error_state());
+}
+
+

[mark a. foltz, 2013/12/13 01:02:02]

Extra newlines

[Munjal (Google), 2013/12/14 00:33:42]

Done.

+
+// Test that when an error occurrs in one write, write callback is invoked for
+// all pending writes with the error
+TEST_F(CastSocketTest, TestWriteErrorWithMultiplePendingWritesAsync) {
+  CreateCastSocket();
+  socket_->AddWriteResult(net::MockWrite(net::ASYNC,
+                                         net::ERR_SOCKET_NOT_CONNECTED));
+  ConnectHelper();
+
+  const int num_writes = arraysize(test_messages_);
+  EXPECT_CALL(handler_, OnWriteComplete(net::ERR_SOCKET_NOT_CONNECTED))
+      .Times(num_writes);
   EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(), cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
+  for (int i = 0; i < num_writes; i++) {
+    socket_->SendMessage(test_messages_[i],
+                         base::Bind(&CompleteHandler::OnWriteComplete,
+                                    base::Unretained(&handler_)));
+  }
+  RunPendingTasks();
+
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_SOCKET_ERROR, socket_->error_state());
+}
+
+// Test read success - single message (async)
+TEST_F(CastSocketTest, TestReadAsync) {
+  CreateCastSocket();

[mark a. foltz, 2013/12/13 01:02:02]

Similar comments about creating a base class to hold the content of the read
test cases, with subclasses to handle the async vs. sync cases.

+  const std::string& message = test_proto_strs_[0];
+  const char* data = message.c_str();
+  size_t body_size = message.size() - 4;
+  socket_->AddReadResult(net::MockRead(net::ASYNC, data, 4));
+  socket_->AddReadResult(net::MockRead(net::ASYNC, data + 4, body_size));
+  EXPECT_CALL(mock_delegate_,
               OnMessage(socket_.get(), A<const MessageInfo&>()));
 
-  // Connect the socket.
-  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
-                              base::Unretained(&handler_)));
-  connect_callback1.Run(net::OK);
-  connect_callback2.Run(net::OK);
+  ConnectHelper();
 
-  // Put the test header and message into the io_buffers and invoke the read
-  // callbacks.
-  memcpy(socket_->header_read_buffer_->StartOfBuffer(),
-         message_data.c_str(), 4);
-  read_callback.Run(4);
-  memcpy(socket_->body_read_buffer_->StartOfBuffer(),
-         message_data.c_str() + 4, 35);
-  read_callback.Run(35);
-
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Tests reading multiple messages.
-TEST_F(CastSocketTest, TestReadMany) {
+// Test read success - single message (sync)
+TEST_F(CastSocketTest, TestReadSync) {
   CreateCastSocket();
+  const std::string& message = test_proto_strs_[0];
+  const char* data = message.c_str();
+  size_t body_size = message.size() - 4;
+  socket_->AddReadResult(net::MockRead(net::SYNCHRONOUS, data, 4));
+  socket_->AddReadResult(net::MockRead(
+      net::SYNCHRONOUS, data + 4, body_size));
+  EXPECT_CALL(mock_delegate_,
+              OnMessage(socket_.get(), A<const MessageInfo&>()));
 
-  net::CompletionCallback connect_callback1;
-  net::CompletionCallback connect_callback2;
-  net::CompletionCallback read_callback;
+  ConnectHelper();
 
-  std::string messages[4];
-  messages[0] = "Hello, World!";
-  messages[1] = "Goodbye, World!";
-  messages[2] = "Hello, Sky!";
-  messages[3] = "Goodbye, Volcano!";
-  int sizes[4] = {35, 37, 33, 39};
-  std::string message_data[4];
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
 
-  // Set up test data
-  for (int i = 0; i < 4; i++) {
-    test_proto_.set_payload_utf8(messages[i]);
-    ASSERT_TRUE(CastSocket::Serialize(test_proto_, &message_data[i]));
+// Test read success - single message received in multiple chunks (async)
+TEST_F(CastSocketTest, TestReadChunkedAsync) {
+  CreateCastSocket();
+  const std::string& message = test_proto_strs_[0];
+  size_t msg_size = message.size();
+  const char* data = message.c_str();
+  size_t ch_size = 2;  // chunk size
+  for (size_t offset = 0; offset < msg_size; offset += ch_size) {
+    if (offset + ch_size > msg_size)
+      ch_size = msg_size - offset;
+    socket_->AddReadResult(net::MockRead(net::ASYNC, data + offset, ch_size));
   }
+  EXPECT_CALL(mock_delegate_,
+              OnMessage(socket_.get(), A<const MessageInfo&>()));
+  ConnectHelper();
 
-  ExpectTcpConnect(&connect_callback1, net::ERR_IO_PENDING);
-  ExpectSslConnect(&connect_callback2, net::ERR_IO_PENDING);
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
 
-  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
-  EXPECT_CALL(mock_ssl_socket(), Read(A<net::IOBuffer*>(),
-                                      A<int>(),
-                                      A<const net::CompletionCallback&>()))
-    .Times(9)
-    .WillRepeatedly(DoAll(SaveArg<2>(&read_callback),
-                          Return(net::ERR_IO_PENDING)));
+// Test read success - single message received in multiple chunks (sync)
+TEST_F(CastSocketTest, TestReadChunkedSync) {
+  CreateCastSocket();
+  const std::string& message = test_proto_strs_[0];
+  size_t msg_size = message.size();
+  const char* data = message.c_str();
+  size_t ch_size = 2;  // chunk size
+  for (size_t offset = 0; offset < msg_size; offset += ch_size) {
+    if (offset + ch_size > msg_size)
+      ch_size = msg_size - offset;
+    socket_->AddReadResult(net::MockRead(
+        net::SYNCHRONOUS, data + offset, ch_size));
+  }
+  EXPECT_CALL(mock_delegate_,
+              OnMessage(socket_.get(), A<const MessageInfo&>()));
+  ConnectHelper();
 
-  // Expect the test messages to be read and invoke the delegate.
-  EXPECT_CALL(mock_delegate_, OnMessage(socket_.get(),
-                                        A<const MessageInfo&>()))
-    .Times(4);
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
 
-  // Connect the socket.
-  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
-                              base::Unretained(&handler_)));
-  connect_callback1.Run(net::OK);
-  connect_callback2.Run(net::OK);
+// Test read success - multiple messages (async)
+TEST_F(CastSocketTest, TestReadManyAsync) {
+  CreateCastSocket();
 
-  // Put the test headers and messages into the io_buffer and invoke the read
-  // callbacks.
-  for (int i = 0; i < 4; i++) {
-    memcpy(socket_->header_read_buffer_->StartOfBuffer(),
-           message_data[i].c_str(), 4);
-    read_callback.Run(4);
-    memcpy(socket_->body_read_buffer_->StartOfBuffer(),
-           message_data[i].c_str() + 4, sizes[i]);
-    read_callback.Run(sizes[i]);
+  size_t num_reads = arraysize(test_proto_strs_);
+  for (size_t i = 0; i < num_reads; i++) {
+    const char* data = test_proto_strs_[i].c_str();
+    size_t body_size = test_proto_strs_[i].size() - 4;
+    socket_->AddReadResult(net::MockRead(net::ASYNC, data, 4));
+    socket_->AddReadResult(net::MockRead(net::ASYNC, data + 4, body_size));
   }
 
+  EXPECT_CALL(mock_delegate_,
+              OnMessage(socket_.get(), A<const MessageInfo&>()))
+      .Times(num_reads);
+
+  ConnectHelper();
+
   EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
 }
 
-// Tests error on reading.
-TEST_F(CastSocketTest, TestReadError) {
+// Test read success - multiple messages (sync)
+TEST_F(CastSocketTest, TestReadManySync) {
   CreateCastSocket();
 
-  net::CompletionCallback connect_callback1;
-  net::CompletionCallback connect_callback2;
-  net::CompletionCallback read_callback;
+  size_t num_reads = arraysize(test_proto_strs_);
+  for (size_t i = 0; i < num_reads; i++) {
+    const char* data = test_proto_strs_[i].c_str();
+    size_t body_size = test_proto_strs_[i].size() - 4;
+    socket_->AddReadResult(net::MockRead(net::SYNCHRONOUS, data, 4));
+    socket_->AddReadResult(net::MockRead(
+        net::SYNCHRONOUS, data + 4, body_size));
+  }
 
-  ExpectTcpConnect(&connect_callback1, net::ERR_IO_PENDING);
-  ExpectSslConnect(&connect_callback2, net::ERR_IO_PENDING);
+  EXPECT_CALL(mock_delegate_,
+              OnMessage(socket_.get(), A<const MessageInfo&>()))
+      .Times(num_reads);
 
-  EXPECT_CALL(handler_, OnConnectComplete(net::OK));
-  EXPECT_CALL(mock_ssl_socket(), Read(A<net::IOBuffer*>(),
-                                      A<int>(),
-                                      A<const net::CompletionCallback&>()))
-    .WillOnce(DoAll(SaveArg<2>(&read_callback),
-                    Return(net::ERR_IO_PENDING)));
+  ConnectHelper();
+
+  EXPECT_EQ(cast_channel::READY_STATE_OPEN, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+}
+
+// Test read error - network error (async)
+TEST_F(CastSocketTest, TestReadErrorAsync) {
+  CreateCastSocket();
+  socket_->AddReadResult(net::MockRead(
+      net::ASYNC, net::ERR_SOCKET_NOT_CONNECTED));
+
   EXPECT_CALL(mock_delegate_,
-              OnError(socket_.get(), cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
+              OnError(socket_.get(),
+                      cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
+  ConnectHelper();
 
-  // Connect the socket.
-  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
-                              base::Unretained(&handler_)));
-  connect_callback1.Run(net::OK);
-  connect_callback2.Run(net::OK);
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_SOCKET_ERROR, socket_->error_state());
+}
 
-  // Cause an error.
-  read_callback.Run(net::ERR_SOCKET_NOT_CONNECTED);
+// Test read error - network error (sync)
+TEST_F(CastSocketTest, TestReadErrorSync) {
+  CreateCastSocket();
+  socket_->AddReadResult(net::MockRead(
+      net::SYNCHRONOUS, net::ERR_SOCKET_NOT_CONNECTED));
 
+  EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(),
+                      cast_channel::CHANNEL_ERROR_SOCKET_ERROR));
+  ConnectHelper();
+
   EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_SOCKET_ERROR, socket_->error_state());
 }
 
+// Test read error - header parse error
+TEST_F(CastSocketTest, TestReadHeaderParseError) {
+  CreateCastSocket();
+  uint32 body_size = base::HostToNet32(65537);
+  char header[sizeof(body_size)];
+  memcpy(&header, &body_size, arraysize(header));

[mark a. foltz, 2013/12/13 01:02:02]

There shouldn't be a need to duplicate the code to construct a message header +
payload between here, below and in cast_socket.cc.  Please add a TODO to factor
out a utility method into cast_message_util.cc.

[Munjal (Google), 2013/12/14 00:33:42]

Done.

+  socket_->AddReadResult(net::MockRead(
+      net::SYNCHRONOUS, header, arraysize(header)));
+  EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(),
+                      cast_channel::CHANNEL_ERROR_INVALID_MESSAGE));
+  ConnectHelper();
+
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_INVALID_MESSAGE,
+            socket_->error_state());
+}
+
+// Test read error - body parse error
+TEST_F(CastSocketTest, TestReadBodyParseError) {
+  CreateCastSocket();
+  char body[] = "some body";
+  uint32 body_size = base::HostToNet32(arraysize(body));
+  char header[sizeof(body_size)];
+  memcpy(&header, &body_size, arraysize(header));
+  socket_->AddReadResult(net::MockRead(
+      net::SYNCHRONOUS, header, arraysize(header)));
+  socket_->AddReadResult(net::MockRead(net::SYNCHRONOUS, body, body_size));
+  EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(),
+                      cast_channel::CHANNEL_ERROR_INVALID_MESSAGE));
+  ConnectHelper();
+
+  EXPECT_EQ(cast_channel::READY_STATE_CLOSED, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_INVALID_MESSAGE,
+            socket_->error_state());
+}
+
 }  // namespace cast_channel
 }  // namespace api
 }  // namespace extensions