Added connection timeout functionality to CastSocket.

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

Index: chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc
diff --git a/chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc b/chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc
index d9e4aee5f49dbc0b3e828c00602002a071065b54..88fbdf6049e191db711265dc6ef7e42696a9045a 100644
--- a/chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc
+++ b/chrome/browser/extensions/api/cast_channel/cast_socket_unittest.cc
@@ -21,6 +21,9 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+const int64 kDistantTimeoutMillis = 100000;  // 100 seconds (never hit).
+const int64 kImmediateTimeoutMillis = 100;     // 0.001 seconds.
+
 using ::testing::_;
 using ::testing::A;
 using ::testing::DoAll;
@@ -79,7 +82,17 @@ class MockTCPSocket : public net::TCPClientSocket {
       TCPClientSocket(net::AddressList(), NULL, net::NetLog::Source()),
       connect_data_(connect_data) { }
 
+  explicit MockTCPSocket(bool do_nothing) :
+      TCPClientSocket(net::AddressList(), NULL, net::NetLog::Source()) {
+    CHECK(do_nothing);
+    do_nothing_ = true;
+  }
+
   virtual int Connect(const net::CompletionCallback& callback) OVERRIDE {
+    if (do_nothing_) {
+      return net::ERR_IO_PENDING;
+    }
+
     if (connect_data_.mode == net::ASYNC) {
       CHECK_NE(connect_data_.result, net::ERR_IO_PENDING);
       base::MessageLoop::current()->PostTask(
@@ -112,6 +125,7 @@ class MockTCPSocket : public net::TCPClientSocket {
 
  private:
   net::MockConnect connect_data_;
+  bool do_nothing_;
 };
 
 class CompleteHandler {
@@ -130,26 +144,37 @@ class TestCastSocket : public CastSocket {
       MockCastSocketDelegate* delegate) {
     return scoped_ptr<TestCastSocket>(
         new TestCastSocket(delegate, CreateIPEndPoint(),
-                           CHANNEL_AUTH_TYPE_SSL));
+                           CHANNEL_AUTH_TYPE_SSL,
+                           kDistantTimeoutMillis));
   }
 
   static scoped_ptr<TestCastSocket> CreateSecure(
       MockCastSocketDelegate* delegate) {
     return scoped_ptr<TestCastSocket>(
         new TestCastSocket(delegate, CreateIPEndPoint(),
-                           CHANNEL_AUTH_TYPE_SSL_VERIFIED));
+                           CHANNEL_AUTH_TYPE_SSL_VERIFIED,
+                           kDistantTimeoutMillis));
+  }
+
+  static scoped_ptr<TestCastSocket> CreateDoomedToTimeout(

[mark a. foltz, 2014/07/15 22:18:16]

Instead of fudging timeouts the cleaner way to test time-dependent behavior is
to use a mock clock.  Maybe another reviewer can point to a code sample on how
to do that.

[Kevin M, 2014/07/16 22:59:42]

Done. New approach uses Timer.

+      MockCastSocketDelegate* delegate) {
+    return scoped_ptr<TestCastSocket>(
+        new TestCastSocket(delegate, CreateIPEndPoint(),
+                           CHANNEL_AUTH_TYPE_SSL_VERIFIED,
+                           kImmediateTimeoutMillis));
   }
 
   explicit TestCastSocket(MockCastSocketDelegate* delegate,
                           const net::IPEndPoint& ip_endpoint,
-                          ChannelAuthType channel_auth) :
+                          ChannelAuthType channel_auth,
+                          int64 timeout_ms) :
         CastSocket("abcdefg", ip_endpoint, channel_auth, delegate,
-                   &capturing_net_log_),
+                   &capturing_net_log_, timeout_ms),
       ip_(ip_endpoint),
       connect_index_(0),
       extract_cert_result_(true),
-      verify_challenge_result_(true) {
-  }
+      verify_challenge_result_(true),
+      tcp_unresponsive_(false) {}
 
   static net::IPEndPoint CreateIPEndPoint() {
     net::IPAddressNumber number;
@@ -181,6 +206,9 @@ class TestCastSocket : public CastSocket {
   void SetupSsl2Connect(net::IoMode mode, int result) {
     ssl_connect_data_[1].reset(new net::MockConnect(mode, result));
   }
+  void SetupTcp1ConnectUnresponsive() {
+    tcp_unresponsive_ = true;
+  }
   void AddWriteResult(const net::MockWrite& write) {
     writes_.push_back(write);
   }
@@ -237,9 +265,13 @@ class TestCastSocket : public CastSocket {
 
  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));
+    if (tcp_unresponsive_) {
+      return scoped_ptr<net::TCPClientSocket>(new MockTCPSocket(true));
+    } else {
+      net::MockConnect* connect_data = tcp_connect_data_[connect_index_].get();
+      connect_data->peer_addr = ip_;
+      return scoped_ptr<net::TCPClientSocket>(new MockTCPSocket(*connect_data));
+    }
   }
 
   virtual scoped_ptr<net::SSLClientSocket> CreateSslSocket(
@@ -282,6 +314,8 @@ class TestCastSocket : public CastSocket {
   bool extract_cert_result_;
   // Simulated result of verifying challenge reply.
   bool verify_challenge_result_;
+  // If true, makes TCP connection process stall. For timeout testing.
+  bool tcp_unresponsive_;
 };
 
 class CastSocketTest : public testing::Test {
@@ -330,6 +364,10 @@ class CastSocketTest : public testing::Test {
     socket_ = TestCastSocket::CreateSecure(&mock_delegate_);
   }
 
+  void CreateCastSocketUnresponsive() {
+    socket_ = TestCastSocket::CreateDoomedToTimeout(&mock_delegate_);
+  }
+
   // Sets up CastSocket::Connect to succeed.
   // Connecting the socket also starts the read loop; so we add a mock
   // read result that returns IO_PENDING and callback is never fired.
@@ -520,6 +558,31 @@ TEST_F(CastSocketTest, TestConnectTcpConnectErrorSync) {
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
 }
 
+// Test connection error - timeout
+TEST_F(CastSocketTest, TestConnectTcpTimeoutError) {
+  CreateCastSocketUnresponsive();
+
+  socket_->SetupTcp1ConnectUnresponsive();
+  EXPECT_CALL(handler_, OnConnectComplete(net::ERR_TIMED_OUT));
+  EXPECT_CALL(mock_delegate_,
+              OnError(socket_.get(),
+                      cast_channel::CHANNEL_ERROR_CONNECT_ERROR));
+  socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
+                              base::Unretained(&handler_)));
+  RunPendingTasks();
+  EXPECT_EQ(cast_channel::READY_STATE_CONNECTING, socket_->ready_state());
+  EXPECT_EQ(cast_channel::CHANNEL_ERROR_NONE, socket_->error_state());
+
+  // Allow for the timeout to fire by sleeping (plus some padding to protect
+  // against timing-related flakiness).

[mark a. foltz, 2014/07/15 22:18:16]

A mock clock would definitely be better here.

[Kevin M, 2014/07/16 22:59:42]

Done.

+  base::PlatformThread::Sleep(
+      base::TimeDelta::FromMilliseconds(kImmediateTimeoutMillis + 200));
+  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();
@@ -536,7 +599,7 @@ TEST_F(CastSocketTest, TestConnectSslConnectErrorAsync) {
   EXPECT_EQ(cast_channel::CHANNEL_ERROR_CONNECT_ERROR, socket_->error_state());
 }
 
-// Test connection error - SSL connect fails (async)
+// Test connection error - SSL connect fails (sync)
 TEST_F(CastSocketTest, TestConnectSslConnectErrorSync) {
   CreateCastSocketSecure();