Fixes to socket_test_util.cc.

- Removes ref-counting from DelayedSocketData and OrderedSocketData.
- Calls CompleteRead (which reads from underlying data) only if
  a Socket::Read call is "blocked" (previously returned with ERR_IO_PENDING).
- Removes unsafe pointers to the handed out sockets (the factory does not own them and cannot guarantee their lifetime).

BUG=110626
TEST=./net_unittests --gtest_filter=Spdy*:Http*:Socket*:WebSocket*:Ftp*


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

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

2 files changed, 27 insertions, 57 deletions

diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 275a403..baca959 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -253,6 +253,7 @@ DelayedSocketData::DelayedSocketData(
     MockWrite* writes, size_t writes_count)
     : StaticSocketDataProvider(reads, reads_count, writes, writes_count),
       write_delay_(write_delay),
+      read_in_progress_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   DCHECK_GE(write_delay_, 0);
 }
@@ -262,6 +263,7 @@ DelayedSocketData::DelayedSocketData(
     size_t reads_count, MockWrite* writes, size_t writes_count)
     : StaticSocketDataProvider(reads, reads_count, writes, writes_count),
       write_delay_(write_delay),
+      read_in_progress_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   DCHECK_GE(write_delay_, 0);
   set_connect_data(connect);
@@ -271,20 +273,23 @@ DelayedSocketData::~DelayedSocketData() {
 }
 
 void DelayedSocketData::ForceNextRead() {
+  DCHECK(read_in_progress_);
   write_delay_ = 0;
   CompleteRead();
 }
 
 MockRead DelayedSocketData::GetNextRead() {
-  if (write_delay_ > 0)
-    return MockRead(true, ERR_IO_PENDING);
-  return StaticSocketDataProvider::GetNextRead();
+  MockRead out = MockRead(true, ERR_IO_PENDING);
+  if (write_delay_ <= 0)
+    out = StaticSocketDataProvider::GetNextRead();
+  read_in_progress_ = (out.result == ERR_IO_PENDING);
+  return out;
 }
 
 MockWriteResult DelayedSocketData::OnWrite(const std::string& data) {
   MockWriteResult rv = StaticSocketDataProvider::OnWrite(data);
   // Now that our write has completed, we can allow reads to continue.
-  if (!--write_delay_)
+  if (!--write_delay_ && read_in_progress_)
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&DelayedSocketData::CompleteRead,
@@ -295,12 +300,13 @@ MockWriteResult DelayedSocketData::OnWrite(const std::string& data) {
 
 void DelayedSocketData::Reset() {
   set_socket(NULL);
+  read_in_progress_ = false;
   weak_factory_.InvalidateWeakPtrs();
   StaticSocketDataProvider::Reset();
 }
 
 void DelayedSocketData::CompleteRead() {
-  if (socket())
+  if (socket() && read_in_progress_)
     socket()->OnReadComplete(GetNextRead());
 }
 
@@ -353,6 +359,7 @@ MockRead OrderedSocketData::GetNextRead() {
     NET_TRACE(INFO, "  *** ") << "Stage " << sequence_number_ - 1
                               << ": Read " << read_index();
     DumpMockRead(next_read);
+    blocked_ = (next_read.result == ERR_IO_PENDING);
     return StaticSocketDataProvider::GetNextRead();
   }
   NET_TRACE(INFO, "  *** ") << "Stage " << sequence_number_ - 1
@@ -394,7 +401,7 @@ void OrderedSocketData::Reset() {
 }
 
 void OrderedSocketData::CompleteRead() {
-  if (socket()) {
+  if (socket() && blocked_) {
     NET_TRACE(INFO, "  *** ") << "Stage " << sequence_number_;
     socket()->OnReadComplete(GetNextRead());
   }
@@ -582,18 +589,6 @@ void MockClientSocketFactory::ResetNextMockIndexes() {
   mock_ssl_data_.ResetNextIndex();
 }
 
-MockTCPClientSocket* MockClientSocketFactory::GetMockTCPClientSocket(
-    size_t index) const {
-  DCHECK_LT(index, tcp_client_sockets_.size());
-  return tcp_client_sockets_[index];
-}
-
-MockSSLClientSocket* MockClientSocketFactory::GetMockSSLClientSocket(
-    size_t index) const {
-  DCHECK_LT(index, ssl_client_sockets_.size());
-  return ssl_client_sockets_[index];
-}
-
 DatagramClientSocket* MockClientSocketFactory::CreateDatagramClientSocket(
     DatagramSocket::BindType bind_type,
     const RandIntCallback& rand_int_cb,
@@ -602,7 +597,6 @@ DatagramClientSocket* MockClientSocketFactory::CreateDatagramClientSocket(
   SocketDataProvider* data_provider = mock_data_.GetNext();
   MockUDPClientSocket* socket = new MockUDPClientSocket(data_provider, net_log);
   data_provider->set_socket(socket);
-  udp_client_sockets_.push_back(socket);
   return socket;
 }
 
@@ -614,7 +608,6 @@ StreamSocket* MockClientSocketFactory::CreateTransportClientSocket(
   MockTCPClientSocket* socket =
       new MockTCPClientSocket(addresses, net_log, data_provider);
   data_provider->set_socket(socket);
-  tcp_client_sockets_.push_back(socket);
   return socket;
 }
 
@@ -627,7 +620,6 @@ SSLClientSocket* MockClientSocketFactory::CreateSSLClientSocket(
   MockSSLClientSocket* socket =
       new MockSSLClientSocket(transport_socket, host_and_port, ssl_config,
                               ssl_host_info, mock_ssl_data_.GetNext());
-  ssl_client_sockets_.push_back(socket);
   return socket;
 }
 
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 393053b..a9739cd 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -273,16 +273,15 @@ struct SSLSocketDataProvider {
 
 // A DataProvider where the client must write a request before the reads (e.g.
 // the response) will complete.
-class DelayedSocketData : public StaticSocketDataProvider,
-                          public base::RefCounted<DelayedSocketData> {
+class DelayedSocketData : public StaticSocketDataProvider {
  public:
   // |write_delay| the number of MockWrites to complete before allowing
   //               a MockRead to complete.
   // |reads| the list of MockRead completions.
   // |writes| the list of MockWrite completions.
   // Note: All MockReads and MockWrites must be async.
-  // Note: The MockRead and MockWrite lists musts end with a EOF
-  //       e.g. a MockRead(true, 0, 0);
+  // Note: For stream sockets, the MockRead list must end with a EOF, e.g., a
+  //       MockRead(true, 0, 0);
   DelayedSocketData(int write_delay,
                     MockRead* reads, size_t reads_count,
                     MockWrite* writes, size_t writes_count);
@@ -293,8 +292,8 @@ class DelayedSocketData : public StaticSocketDataProvider,
   //               a MockRead to complete.
   // |writes| the list of MockWrite completions.
   // Note: All MockReads and MockWrites must be async.
-  // Note: The MockRead and MockWrite lists musts end with a EOF
-  //       e.g. a MockRead(true, 0, 0);
+  // Note: For stream sockets, the MockRead list must end with a EOF, e.g., a
+  //       MockRead(true, 0, 0);
   DelayedSocketData(const MockConnect& connect, int write_delay,
                     MockRead* reads, size_t reads_count,
                     MockWrite* writes, size_t writes_count);
@@ -310,6 +309,7 @@ class DelayedSocketData : public StaticSocketDataProvider,
 
  private:
   int write_delay_;
+  bool read_in_progress_;
   base::WeakPtrFactory<DelayedSocketData> weak_factory_;
 };
 
@@ -324,23 +324,23 @@ class DelayedSocketData : public StaticSocketDataProvider,
 // of data before a complete message has arrived, and provides support for
 // testing server push when the request is issued while the response is in the
 // middle of being received.
-class OrderedSocketData : public StaticSocketDataProvider,
-                          public base::RefCounted<OrderedSocketData> {
+class OrderedSocketData : public StaticSocketDataProvider {
  public:
   // |reads| the list of MockRead completions.
   // |writes| the list of MockWrite completions.
   // Note: All MockReads and MockWrites must be async.
-  // Note: The MockRead and MockWrite lists musts end with a EOF
-  //       e.g. a MockRead(true, 0, 0);
+  // Note: For stream sockets, the MockRead list must end with a EOF, e.g., a
+  //       MockRead(true, 0, 0);
   OrderedSocketData(MockRead* reads, size_t reads_count,
                     MockWrite* writes, size_t writes_count);
+  virtual ~OrderedSocketData();
 
   // |connect| the result for the connect phase.
   // |reads| the list of MockRead completions.
   // |writes| the list of MockWrite completions.
   // Note: All MockReads and MockWrites must be async.
-  // Note: The MockRead and MockWrite lists musts end with a EOF
-  //       e.g. a MockRead(true, 0, 0);
+  // Note: For stream sockets, the MockRead list must end with a EOF, e.g., a
+  //       MockRead(true, 0, 0);
   OrderedSocketData(const MockConnect& connect,
                     MockRead* reads, size_t reads_count,
                     MockWrite* writes, size_t writes_count);
@@ -355,9 +355,6 @@ class OrderedSocketData : public StaticSocketDataProvider,
   virtual void CompleteRead() OVERRIDE;
 
  private:
-  friend class base::RefCounted<OrderedSocketData>;
-  virtual ~OrderedSocketData();
-
   int sequence_number_;
   int loop_stop_stage_;
   bool blocked_;
@@ -530,23 +527,9 @@ class MockClientSocketFactory : public ClientSocketFactory {
   void AddSSLSocketDataProvider(SSLSocketDataProvider* socket);
   void ResetNextMockIndexes();
 
-  // Return |index|-th MockTCPClientSocket (starting from 0) that the factory
-  // created.
-  MockTCPClientSocket* GetMockTCPClientSocket(size_t index) const;
-
-  // Return |index|-th MockSSLClientSocket (starting from 0) that the factory
-  // created.
-  MockSSLClientSocket* GetMockSSLClientSocket(size_t index) const;
-
   SocketDataProviderArray<SocketDataProvider>& mock_data() {
     return mock_data_;
   }
-  std::vector<MockTCPClientSocket*>& tcp_client_sockets() {
-    return tcp_client_sockets_;
-  }
-  std::vector<MockUDPClientSocket*>& udp_client_sockets() {
-    return udp_client_sockets_;
-  }
 
   // ClientSocketFactory
   virtual DatagramClientSocket* CreateDatagramClientSocket(
@@ -569,11 +552,6 @@ class MockClientSocketFactory : public ClientSocketFactory {
  private:
   SocketDataProviderArray<SocketDataProvider> mock_data_;
   SocketDataProviderArray<SSLSocketDataProvider> mock_ssl_data_;
-
-  // Store pointers to handed out sockets in case the test wants to get them.
-  std::vector<MockUDPClientSocket*> udp_client_sockets_;
-  std::vector<MockTCPClientSocket*> tcp_client_sockets_;
-  std::vector<MockSSLClientSocket*> ssl_client_sockets_;
 };
 
 class MockClientSocket : public SSLClientSocket {