Added connection timeout functionality to CastSocket.

chrome/browser/extensions/api/cast_channel/cast_socket_unittest.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 "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"
 #include "net/base/capturing_net_log.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/socket/socket_test_util.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/tcp_client_socket.h"
 #include "net/ssl/ssl_info.h"
 #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;
 using ::testing::Return;
 using ::testing::SaveArg;
 
 namespace {
 const char* kTestData[4] = {
     "Hello, World!",
     "Goodbye, World!",
@@ skipping 38 matching lines @@
   MOCK_METHOD2(OnMessage, void(const CastSocket* socket,
                                const MessageInfo& message));
 };
 
 class MockTCPSocket : public net::TCPClientSocket {
  public:
   explicit MockTCPSocket(const net::MockConnect& connect_data) :
       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(
           FROM_HERE,
           base::Bind(callback, connect_data_.result));
       return net::ERR_IO_PENDING;
     } else {
       return connect_data_.result;
     }
   }
@@ skipping 12 matching lines @@
                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_;
+  bool do_nothing_;
 };
 
 class CompleteHandler {
  public:
   CompleteHandler() {}
   MOCK_METHOD1(OnCloseComplete, void(int result));
   MOCK_METHOD1(OnConnectComplete, void(int result));
   MOCK_METHOD1(OnWriteComplete, void(int result));
  private:
   DISALLOW_COPY_AND_ASSIGN(CompleteHandler);
 };
 
 class TestCastSocket : public CastSocket {
  public:
   static scoped_ptr<TestCastSocket> Create(
       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;
     number.push_back(192);
     number.push_back(0);
     number.push_back(0);
     number.push_back(1);
     return net::IPEndPoint(number, 8009);
   }
 
@@ skipping 11 matching lines @@
   }
   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 SetupTcp1ConnectUnresponsive() {
+    tcp_unresponsive_ = true;
+  }
   void AddWriteResult(const net::MockWrite& write) {
     writes_.push_back(write);
   }
   void AddWriteResult(net::IoMode mode, int result) {
     AddWriteResult(net::MockWrite(mode, result));
   }
   void AddWriteResultForMessage(net::IoMode mode, const std::string& msg) {
     AddWriteResult(mode, msg.size());
   }
   void AddWriteResultForMessage(net::IoMode mode,
@@ skipping 36 matching lines @@
 
   void SetExtractCertResult(bool value) {
     extract_cert_result_ = value;
   }
   void SetVerifyChallengeResult(bool value) {
     verify_challenge_result_ = value;
   }
 
  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(
       scoped_ptr<net::StreamSocket> socket) OVERRIDE {
     net::MockConnect* connect_data = ssl_connect_data_[connect_index_].get();
     connect_data->peer_addr = ip_;
     ++connect_index_;
 
     ssl_data_.reset(new net::StaticSocketDataProvider(
         reads_.data(), reads_.size(), writes_.data(), writes_.size()));
@@ skipping 22 matching lines @@
   // 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 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 {
  public:
   CastSocketTest() {}
   virtual ~CastSocketTest() {}
 
   virtual void SetUp() OVERRIDE {
     // Create a few test messages
     for (size_t i = 0; i < arraysize(test_messages_); i++) {
@@ skipping 28 matching lines @@
   }
 
   void CreateCastSocket() {
     socket_ = TestCastSocket::Create(&mock_delegate_);
   }
 
   void CreateCastSocketSecure() {
     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.
   void ConnectHelper() {
     socket_->SetupTcp1Connect(net::SYNCHRONOUS, net::OK);
     socket_->SetupSsl1Connect(net::SYNCHRONOUS, net::OK);
     socket_->AddReadResult(net::ASYNC, net::ERR_IO_PENDING);
 
     EXPECT_CALL(handler_, OnConnectComplete(net::OK));
     socket_->Connect(base::Bind(&CompleteHandler::OnConnectComplete,
@@ skipping 170 matching lines @@
 
   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 - 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();
 
   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_)));
   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 connection error - SSL connect fails (sync)
 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();
@@ skipping 443 matching lines @@
   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