Use sockets with unread data if they've never been used before.

net/socket/client_socket_pool_base_unittest.cc

 // Copyright (c) 2012 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/socket/client_socket_pool_base.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 98 matching lines @@
   ~TestSocketParams() {}
 
   const bool ignore_limits_;
 };
 typedef ClientSocketPoolBase<TestSocketParams> TestClientSocketPoolBase;
 
 class MockClientSocket : public StreamSocket {
  public:
   explicit MockClientSocket(net::NetLog* net_log)
       : connected_(false),
+        idle_(true),
         net_log_(BoundNetLog::Make(net_log, net::NetLog::SOURCE_SOCKET)),
         was_used_to_convey_data_(false) {
   }
 
+  // Sets whether the socket has unread data. If false, the next call to Read()
+  // will return 1 byte and IsConnectedAndIdle() will return false.
+  void set_idle(bool idle) { idle_ = idle; }

[mmenke, 2014/03/26 15:18:32]

I don't think "idle" is the right name for this.  Maybe just
"set_has_unread_data" or pending data or somesuch.

[davidben, 2014/03/26 16:58:29]

Done.

+
   // Socket implementation.
   virtual int Read(
       IOBuffer* /* buf */, int len,
       const CompletionCallback& /* callback */) OVERRIDE {
+    if (!idle_ && len > 0) {
+      idle_ = true;
+      was_used_to_convey_data_ = true;
+      return 1;
+    }
     return ERR_UNEXPECTED;
   }
 
   virtual int Write(
       IOBuffer* /* buf */, int len,
       const CompletionCallback& /* callback */) OVERRIDE {
     was_used_to_convey_data_ = true;
     return len;
   }
   virtual bool SetReceiveBufferSize(int32 size) OVERRIDE { return true; }
   virtual bool SetSendBufferSize(int32 size) OVERRIDE { return true; }
 
   // StreamSocket implementation.
   virtual int Connect(const CompletionCallback& callback) OVERRIDE {
     connected_ = true;
     return OK;
   }
 
   virtual void Disconnect() OVERRIDE { connected_ = false; }
   virtual bool IsConnected() const OVERRIDE { return connected_; }
-  virtual bool IsConnectedAndIdle() const OVERRIDE { return connected_; }
+  virtual bool IsConnectedAndIdle() const OVERRIDE {
+    return connected_ && idle_;
+  }
 
   virtual int GetPeerAddress(IPEndPoint* /* address */) const OVERRIDE {
     return ERR_UNEXPECTED;
   }
 
   virtual int GetLocalAddress(IPEndPoint* /* address */) const OVERRIDE {
     return ERR_UNEXPECTED;
   }
 
   virtual const BoundNetLog& NetLog() const OVERRIDE {
@@ skipping 11 matching lines @@
   }
   virtual NextProto GetNegotiatedProtocol() const OVERRIDE {
     return kProtoUnknown;
   }
   virtual bool GetSSLInfo(SSLInfo* ssl_info) OVERRIDE {
     return false;
   }
 
  private:
   bool connected_;
+  bool idle_;
   BoundNetLog net_log_;
   bool was_used_to_convey_data_;
 
   DISALLOW_COPY_AND_ASSIGN(MockClientSocket);
 };
 
 class TestConnectJob;
 
 class MockClientSocketFactory : public ClientSocketFactory {
  public:
@@ skipping 49 matching lines @@
   enum JobType {
     kMockJob,
     kMockFailingJob,
     kMockPendingJob,
     kMockPendingFailingJob,
     kMockWaitingJob,
     kMockRecoverableJob,
     kMockPendingRecoverableJob,
     kMockAdditionalErrorStateJob,
     kMockPendingAdditionalErrorStateJob,
+    kMockUnreadDataJob,
   };
 
   // The kMockPendingJob uses a slight delay before allowing the connect
   // to complete.
   static const int kPendingConnectDelay = 2;
 
   TestConnectJob(JobType job_type,
                  const std::string& group_name,
                  const TestClientSocketPoolBase::Request& request,
                  base::TimeDelta timeout_duration,
@@ skipping 107 matching lines @@
         store_additional_error_state_ = true;
         base::MessageLoop::current()->PostDelayedTask(
             FROM_HERE,
             base::Bind(base::IgnoreResult(&TestConnectJob::DoConnect),
                        weak_factory_.GetWeakPtr(),
                        false /* error */,
                        true  /* async */,
                        false /* recoverable */),
             base::TimeDelta::FromMilliseconds(2));
         return ERR_IO_PENDING;
+      case kMockUnreadDataJob: {
+        int ret = DoConnect(true /* successful */, false /* sync */,
+                            false /* recoverable */);
+        static_cast<MockClientSocket*>(socket())->set_idle(false);
+        return ret;
+      }
       default:
         NOTREACHED();
         SetSocket(scoped_ptr<StreamSocket>());
         return ERR_FAILED;
     }
   }
 
   int DoConnect(bool succeed, bool was_async, bool recoverable) {
     int result = OK;
     if (succeed) {
@@ skipping 3149 matching lines @@
                                          callback1.callback(),
                                          pool_.get(),
                                          BoundNetLog()));
 
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   ASSERT_EQ(OK, callback1.WaitForResult());
 
-  // Make sure if a preconneced socket is not fully connected when a request
+  // Make sure if a preconnected socket is not fully connected when a request
   // starts, it has a connect start time.
   TestLoadTimingInfoConnectedNotReused(handle1);
   handle1.Reset();
 
   EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
 }
 
 // Checks that fully connected preconnect jobs have no connect times, and are
 // marked as reused.
 TEST_F(ClientSocketPoolBaseTest, ConnectedPreconnectJobsHaveNoConnectTimes) {
@@ skipping 158 matching lines @@
   ASSERT_EQ(OK, callback.WaitForResult());
 
   // The hung connect job should still be there, but everything else should be
   // complete.
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a"));
 }
 
+// Tests that a preconnect that starts out with unread data can still be used.
+// http://crbug.com/334467
+TEST_F(ClientSocketPoolBaseTest, PreconnectWithUnreadData) {
+  CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
+  connect_job_factory_->set_job_type(TestConnectJob::kMockUnreadDataJob);
+
+  pool_->RequestSockets("a", &params_, 1, BoundNetLog());
+
+  ASSERT_TRUE(pool_->HasGroup("a"));
+  EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
+
+  // Fail future jobs to be sure that handle receives the preconnected socket
+  // rather than closing it and making a new one.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockFailingJob);
+  ClientSocketHandle handle;
+  TestCompletionCallback callback;
+  EXPECT_EQ(OK, handle.Init("a",
+                             params_,
+                             DEFAULT_PRIORITY,
+                             callback.callback(),
+                             pool_.get(),
+                             BoundNetLog()));
+
+  ASSERT_TRUE(pool_->HasGroup("a"));
+  EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+
+  // Drain the pending read.
+  EXPECT_EQ(1, handle.socket()->Read(NULL, 1, CompletionCallback()));
+
+  TestLoadTimingInfoConnectedReused(handle);
+  handle.Reset();
+
+  // The socket should be usable now that it's idle again.
+  EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
+}
+
 class MockLayeredPool : public HigherLayeredPool {
  public:
   MockLayeredPool(TestClientSocketPool* pool,
                   const std::string& group_name)
       : pool_(pool),
         group_name_(group_name),
         can_release_connection_(true) {
     pool_->AddHigherLayeredPool(this);
   }
 
@@ skipping 325 matching lines @@
   request(1)->handle()->Reset();
   ASSERT_EQ(1, pool_->NumConnectJobsInGroup("a"));
 
   EXPECT_EQ(OK, request(2)->WaitForResult());
   EXPECT_FALSE(request(1)->have_result());
 }
 
 }  // namespace
 
 }  // namespace net