Fix test crashes caused by toxic use of ListenSocket.

ListenSocket doesn't like it if delegates delete instances from within DidFoo() callbacks.  Instead of releasing references to sockets from within ConfigurableConnection::SendWithOptions (which is invoked indirectly by way of HTTPTestServer::DidRead), post a task to do so.  Handle the fact that there may be connections in an HTTPTestServer instance for which the underlying socket_ is NULL by harvesting them in HTTPTestServer::FindConnection().  As a result of all of this, it's possible that DidRead or DidClose may be called with a ListenSocket* that is no longer present in the server's connection list.  This isn't an error condition, so simply handle it gracefully in Did{Read,Close}.

BUG=120458
TEST=nothing special.  chrome_frame_tests.exe should stop crashing so much.


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

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

2 files changed, 26 insertions, 7 deletions

diff --git a/chrome_frame/test/test_server.cc b/chrome_frame/test/test_server.cc
index f7a3abd..a4875fb 100644
--- a/chrome_frame/test/test_server.cc
+++ b/chrome_frame/test/test_server.cc
@@ -246,10 +246,18 @@ HTTPTestServer::~HTTPTestServer() {
 std::list<scoped_refptr<ConfigurableConnection>>::iterator
 HTTPTestServer::FindConnection(const net::ListenSocket* socket) {
   ConnectionList::iterator it;
-  for (it = connection_list_.begin(); it != connection_list_.end(); ++it) {
-    if ((*it)->socket_ == socket) {
-      break;
+  // Scan through the list searching for the desired socket. Along the way,
+  // erase any connections for which the corresponding socket has already been
+  // forgotten about as a result of all data having been sent.
+  for (it = connection_list_.begin(); it != connection_list_.end(); ) {
+    ConfigurableConnection* connection = it->get();
+    if (connection->socket_ == NULL) {
+      connection_list_.erase(it++);
+      continue;
     }
+    if (connection->socket_ == socket)
+      break;
+    ++it;
   }
 
   return it;
@@ -290,8 +298,8 @@ void HTTPTestServer::DidRead(net::ListenSocket* socket,
 
 void HTTPTestServer::DidClose(net::ListenSocket* socket) {
   ConnectionList::iterator it = FindConnection(socket);
-  DCHECK(it != connection_list_.end());
-  connection_list_.erase(it);
+  if (it != connection_list_.end())
+    connection_list_.erase(it);
 }
 
 std::wstring HTTPTestServer::Resolve(const std::wstring& path) {
@@ -336,6 +344,10 @@ void ConfigurableConnection::SendChunk() {
   }
 }
 
+void ConfigurableConnection::Close() {
+  socket_ = NULL;
+}
+
 void ConfigurableConnection::Send(const std::string& headers,
                                   const std::string& content) {
   SendOptions options(SendOptions::IMMEDIATE, 0, 0);
@@ -359,7 +371,11 @@ void ConfigurableConnection::SendWithOptions(const std::string& headers,
     socket_->Send(headers);
     socket_->Send(content_length_header, true);
     socket_->Send(content);
-    socket_ = 0;  // close the connection.
+    // Post a task to close the socket since ListenSocket doesn't like instances
+    // to go away from within its callbacks.
+    MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(&ConfigurableConnection::Close, this));
+
     return;
   }
 
diff --git a/chrome_frame/test/test_server.h b/chrome_frame/test/test_server.h
index 1b60645..aa706d8 100644
--- a/chrome_frame/test/test_server.h
+++ b/chrome_frame/test/test_server.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.
 
@@ -360,6 +360,9 @@ class ConfigurableConnection : public base::RefCounted<ConfigurableConnection> {
   // next chunk of |data_|.
   void SendChunk();
 
+  // Closes the connection by releasing this instance's reference on its socket.
+  void Close();
+
   scoped_refptr<net::ListenSocket> socket_;
   Request r_;
   SendOptions options_;