Fixed some minor bugs with ChromeAsyncSocket.

BUG=45612
TEST=new unit tests

Review URL: http://codereview.chromium.org/2966011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52447 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 44 insertions, 2 deletions

diff --git a/chrome/browser/sync/tools/chrome_async_socket.cc b/chrome/browser/sync/tools/chrome_async_socket.cc
index 52952db..e5d662f 100644
--- a/chrome/browser/sync/tools/chrome_async_socket.cc
+++ b/chrome/browser/sync/tools/chrome_async_socket.cc
@@ -249,7 +249,14 @@ void ChromeAsyncSocket::ProcessReadDone(int status) {
 
 bool ChromeAsyncSocket::Read(char* data, size_t len, size_t* len_read) {
   if (!IsOpen() && (state_ != STATE_TLS_CONNECTING)) {
-    LOG(DFATAL) << "Read() called on non-open non-tls-connecting socket";
+    // Read() may be called on a closed socket if a previous read
+    // causes a socket close (e.g., client sends wrong password and
+    // server terminates connection).
+    //
+    // TODO(akalin): Fix handling of this on the libjingle side.
+    if (state_ != STATE_CLOSED) {
+      LOG(DFATAL) << "Read() called on non-open non-tls-connecting socket";
+    }
     DoNonNetError(ERROR_WRONGSTATE);
     return false;
   }
@@ -457,6 +464,7 @@ void ChromeAsyncSocket::ProcessSSLConnectDone(int status) {
   DCHECK_EQ(write_state_, IDLE);
   if (status != net::OK) {
     DoNetErrorFromStatus(status);
+    DoClose();
     return;
   }
   state_ = STATE_TLS_OPEN;
diff --git a/chrome/browser/sync/tools/chrome_async_socket_unittest.cc b/chrome/browser/sync/tools/chrome_async_socket_unittest.cc
index 225a215..bf33ea5 100644
--- a/chrome/browser/sync/tools/chrome_async_socket_unittest.cc
+++ b/chrome/browser/sync/tools/chrome_async_socket_unittest.cc
@@ -97,7 +97,6 @@ class ChromeAsyncSocketTest
     : public testing::Test,
       public sigslot::has_slots<> {
  protected:
-  // TODO(akalin): test SSL states other than connection success.
   ChromeAsyncSocketTest()
       : ssl_socket_data_provider_(true, net::OK),
         capturing_net_log_(net::CapturingNetLog::kUnbounded),
@@ -529,6 +528,12 @@ TEST_F(ChromeAsyncSocketTest, EmptyRead) {
 
 TEST_F(ChromeAsyncSocketTest, WrongRead) {
   EXPECT_DEBUG_DEATH({
+    async_socket_data_provider_.set_connect_data(
+        net::MockConnect(true, net::OK));
+    EXPECT_TRUE(chrome_async_socket_.Connect(addr_));
+    ExpectNonErrorState(ChromeAsyncSocket::STATE_CONNECTING);
+    ExpectNoSignal();
+
     char buf[4096];
     size_t len_read;
     EXPECT_FALSE(chrome_async_socket_.Read(buf, sizeof(buf), &len_read));
@@ -538,6 +543,15 @@ TEST_F(ChromeAsyncSocketTest, WrongRead) {
   }, "non-open");
 }
 
+TEST_F(ChromeAsyncSocketTest, WrongReadClosed) {
+  char buf[4096];
+  size_t len_read;
+  EXPECT_FALSE(chrome_async_socket_.Read(buf, sizeof(buf), &len_read));
+  ExpectErrorState(ChromeAsyncSocket::STATE_CLOSED,
+                   ChromeAsyncSocket::ERROR_WRONGSTATE);
+  EXPECT_TRUE(chrome_async_socket_.Close());
+}
+
 const char kReadData[] = "mydatatoread";
 
 TEST_F(ChromeAsyncSocketTest, Read) {
@@ -823,6 +837,26 @@ TEST_F(ChromeAsyncSocketTest, DoubleSSLConnect) {
   }, "wrong state");
 }
 
+TEST_F(ChromeAsyncSocketTest, FailedSSLConnect) {
+  ssl_socket_data_provider_.connect =
+      net::MockConnect(true, net::ERR_CERT_COMMON_NAME_INVALID),
+
+  async_socket_data_provider_.AddRead(net::MockRead(kReadData));
+  DoOpenClosed();
+  ExpectReadSignal();
+
+  EXPECT_TRUE(chrome_async_socket_.StartTls("fakedomain.com"));
+  message_loop_.RunAllPending();
+  ExpectSignalSocketState(
+      SignalSocketState(
+          SIGNAL_CLOSE, ChromeAsyncSocket::STATE_CLOSED,
+          ChromeAsyncSocket::ERROR_WINSOCK,
+          net::ERR_CERT_COMMON_NAME_INVALID));
+
+  EXPECT_TRUE(chrome_async_socket_.Close());
+  ExpectClosed();
+}
+
 TEST_F(ChromeAsyncSocketTest, ReadDuringSSLConnecting) {
   async_socket_data_provider_.AddRead(net::MockRead(kReadData));
   DoOpenClosed();