summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--net/base/tcp_client_socket_pool.cc62
-rw-r--r--net/base/tcp_client_socket_pool.h10
-rw-r--r--net/url_request/url_request_unittest.cc17
-rw-r--r--net/url_request/url_request_unittest.h5
4 files changed, 35 insertions, 59 deletions
diff --git a/net/base/tcp_client_socket_pool.cc b/net/base/tcp_client_socket_pool.cc
index 7dadd4c..d35c71a 100644
--- a/net/base/tcp_client_socket_pool.cc
+++ b/net/base/tcp_client_socket_pool.cc
@@ -45,20 +45,15 @@ TCPClientSocketPool::ConnectingSocket::ConnectingSocket(
callback_(this,
&TCPClientSocketPool::ConnectingSocket::OnIOComplete)),
pool_(pool),
- resolver_(pool->GetHostResolver()),
- canceled_(false) {
- CHECK(!ContainsKey(pool_->connecting_socket_map_, handle));
- pool_->connecting_socket_map_[handle] = this;
-}
+ resolver_(pool->GetHostResolver()) {}
TCPClientSocketPool::ConnectingSocket::~ConnectingSocket() {
- if (!canceled_)
- pool_->connecting_socket_map_.erase(handle_);
+ // We don't worry about cancelling the host resolution and TCP connect, since
+ // ~SingleRequestHostResolver and ~ClientSocket will take care of it.
}
int TCPClientSocketPool::ConnectingSocket::Connect(
const HostResolver::RequestInfo& resolve_info) {
- CHECK(!canceled_);
int rv = resolver_.Resolve(resolve_info, &addresses_, &callback_);
if (rv != ERR_IO_PENDING)
rv = OnIOCompleteInternal(rv, true /* synchronous */);
@@ -73,30 +68,14 @@ int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
int result, bool synchronous) {
CHECK(result != ERR_IO_PENDING);
- if (canceled_) {
- // We got canceled, so bail out.
- delete this;
- return result;
- }
-
GroupMap::iterator group_it = pool_->group_map_.find(group_name_);
- if (group_it == pool_->group_map_.end()) {
- // The request corresponding to this ConnectingSocket has been canceled.
- // Stop bothering with it.
- delete this;
- return result;
- }
+ CHECK(group_it != pool_->group_map_.end());
Group& group = group_it->second;
RequestMap* request_map = &group.connecting_requests;
RequestMap::iterator it = request_map->find(handle_);
- if (it == request_map->end()) {
- // The request corresponding to this ConnectingSocket has been canceled.
- // Stop bothering with it.
- delete this;
- return result;
- }
+ CHECK(it != request_map->end());
if (result == OK && it->second.load_state == LOAD_STATE_RESOLVING_HOST) {
it->second.load_state = LOAD_STATE_CONNECTING;
@@ -108,6 +87,7 @@ int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
}
if (result == OK) {
+ CHECK(it->second.load_state == LOAD_STATE_CONNECTING);
CHECK(connect_start_time_ != base::Time());
base::TimeDelta connect_duration =
base::Time::Now() - connect_start_time_;
@@ -141,17 +121,10 @@ int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
if (!synchronous)
request.callback->Run(result);
- delete this;
+ pool_->RemoveConnectingSocket(handle_); // will delete |this|.
return result;
}
-void TCPClientSocketPool::ConnectingSocket::Cancel() {
- CHECK(!canceled_);
- CHECK(ContainsKey(pool_->connecting_socket_map_, handle_));
- pool_->connecting_socket_map_.erase(handle_);
- canceled_ = true;
-}
-
TCPClientSocketPool::TCPClientSocketPool(
int max_sockets_per_group,
HostResolver* host_resolver,
@@ -168,6 +141,7 @@ TCPClientSocketPool::~TCPClientSocketPool() {
// to the manager being destroyed.
CloseIdleSockets();
DCHECK(group_map_.empty());
+ DCHECK(connecting_socket_map_.empty());
}
// InsertRequestIntoQueue inserts the request into the queue based on
@@ -219,20 +193,16 @@ int TCPClientSocketPool::RequestSocket(
// We couldn't find a socket to reuse, so allocate and connect a new one.
- // First, we need to make sure we aren't already servicing a request for this
- // handle (which could happen if we requested, canceled, and then requested
- // with the same handle).
- if (ContainsKey(connecting_socket_map_, handle))
- connecting_socket_map_[handle]->Cancel();
-
CHECK(callback);
Request r(handle, callback, priority, resolve_info,
LOAD_STATE_RESOLVING_HOST);
group_map_[group_name].connecting_requests[handle] = r;
- // connecting_socket will delete itself.
+ CHECK(!ContainsKey(connecting_socket_map_, handle));
+
ConnectingSocket* connecting_socket =
new ConnectingSocket(group_name, handle, client_socket_factory_, this);
+ connecting_socket_map_[handle] = connecting_socket;
int rv = connecting_socket->Connect(resolve_info);
return rv;
}
@@ -257,6 +227,8 @@ void TCPClientSocketPool::CancelRequest(const std::string& group_name,
RequestMap::iterator map_it = group.connecting_requests.find(handle);
if (map_it != group.connecting_requests.end()) {
+ RemoveConnectingSocket(handle);
+
group.connecting_requests.erase(map_it);
group.active_socket_count--;
@@ -419,4 +391,12 @@ void TCPClientSocketPool::DoReleaseSocket(const std::string& group_name,
}
}
+void TCPClientSocketPool::RemoveConnectingSocket(
+ const ClientSocketHandle* handle) {
+ ConnectingSocketMap::iterator it = connecting_socket_map_.find(handle);
+ CHECK(it != connecting_socket_map_.end());
+ delete it->second;
+ connecting_socket_map_.erase(it);
+}
+
} // namespace net
diff --git a/net/base/tcp_client_socket_pool.h b/net/base/tcp_client_socket_pool.h
index ad75585..33d2a5b 100644
--- a/net/base/tcp_client_socket_pool.h
+++ b/net/base/tcp_client_socket_pool.h
@@ -154,7 +154,6 @@ class TCPClientSocketPool : public ClientSocketPool {
scoped_refptr<TCPClientSocketPool> pool_;
SingleRequestHostResolver resolver_;
AddressList addresses_;
- bool canceled_;
// The time the Connect() method was called (if it got called).
base::Time connect_start_time_;
@@ -162,6 +161,9 @@ class TCPClientSocketPool : public ClientSocketPool {
DISALLOW_COPY_AND_ASSIGN(ConnectingSocket);
};
+ typedef std::map<const ClientSocketHandle*, ConnectingSocket*>
+ ConnectingSocketMap;
+
virtual ~TCPClientSocketPool();
static void InsertRequestIntoQueue(const Request& r,
@@ -184,11 +186,15 @@ class TCPClientSocketPool : public ClientSocketPool {
CleanupIdleSockets(false);
}
+ // Removes the ConnectingSocket corresponding to |handle| from the
+ // |connecting_socket_map_|.
+ void RemoveConnectingSocket(const ClientSocketHandle* handle);
+
ClientSocketFactory* const client_socket_factory_;
GroupMap group_map_;
- std::map<const ClientSocketHandle*, ConnectingSocket*> connecting_socket_map_;
+ ConnectingSocketMap connecting_socket_map_;
// Timer used to periodically prune idle sockets that timed out or can't be
// reused.
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 3af95e2..18ff111 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -45,10 +45,7 @@ namespace {
class URLRequestHttpCacheContext : public URLRequestContext {
public:
URLRequestHttpCacheContext() {
- // TODO(eroman): we turn off host caching to see if synchronous
- // host resolving interacts poorly with client socket pool. [experiment]
- // http://crbug.com/13952
- host_resolver_ = new net::HostResolver(0, 0);
+ host_resolver_ = new net::HostResolver;
proxy_service_ = net::ProxyService::CreateNull();
http_transaction_factory_ =
new net::HttpCache(
@@ -318,8 +315,7 @@ TEST_F(HTTPSRequestTest, MAYBE_HTTPSExpiredTest) {
}
}
-// http://crbug.com/13952
-TEST_F(URLRequestTest, DISABLED_CancelTest) {
+TEST_F(URLRequestTest, CancelTest) {
TestDelegate d;
{
TestURLRequest r(GURL("http://www.google.com/"), &d);
@@ -342,8 +338,7 @@ TEST_F(URLRequestTest, DISABLED_CancelTest) {
#endif
}
-// http://crbug.com/13952
-TEST_F(URLRequestTest, DISABLED_CancelTest2) {
+TEST_F(URLRequestTest, CancelTest2) {
scoped_refptr<HTTPTestServer> server =
HTTPTestServer::CreateServer(L"", NULL);
ASSERT_TRUE(NULL != server.get());
@@ -372,8 +367,7 @@ TEST_F(URLRequestTest, DISABLED_CancelTest2) {
#endif
}
-// http://crbug.com/13952
-TEST_F(URLRequestTest, DISABLED_CancelTest3) {
+TEST_F(URLRequestTest, CancelTest3) {
scoped_refptr<HTTPTestServer> server =
HTTPTestServer::CreateServer(L"", NULL);
ASSERT_TRUE(NULL != server.get());
@@ -401,8 +395,7 @@ TEST_F(URLRequestTest, DISABLED_CancelTest3) {
#endif
}
-// http://crbug.com/13952
-TEST_F(URLRequestTest, DISABLED_CancelTest4) {
+TEST_F(URLRequestTest, CancelTest4) {
scoped_refptr<HTTPTestServer> server =
HTTPTestServer::CreateServer(L"", NULL);
ASSERT_TRUE(NULL != server.get());
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index fa676f8..eddb51d 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -43,10 +43,7 @@ using base::TimeDelta;
class TestURLRequestContext : public URLRequestContext {
public:
TestURLRequestContext() {
- // TODO(eroman): we turn off host caching to see if synchronous
- // host resolving interacts poorly with client socket pool. [experiment]
- // http://crbug.com/13952
- host_resolver_ = new net::HostResolver(0, 0);
+ host_resolver_ = new net::HostResolver;
proxy_service_ = net::ProxyService::CreateNull();
http_transaction_factory_ =
net::HttpNetworkLayer::CreateFactory(host_resolver_,
generated by cgit v1.1 at 2025-01-20 22:31:21 +0000