diff options
author | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-27 21:27:29 +0000 |
---|---|---|
committer | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-27 21:27:29 +0000 |
commit | 4d3b05dee1fe0de431cbc45bba0885673162359c (patch) | |
tree | 73bc3d3565693bb2eb9012458a0921c25d2d17f0 /net | |
parent | 031c396f71d36aa193c262be346a0632f93d3f9b (diff) | |
download | chromium_src-4d3b05dee1fe0de431cbc45bba0885673162359c.zip chromium_src-4d3b05dee1fe0de431cbc45bba0885673162359c.tar.gz chromium_src-4d3b05dee1fe0de431cbc45bba0885673162359c.tar.bz2 |
Switch on socket late binding - Take 2.
Re-enable socket late binding. The mac valgrind errors happened due to threading bugs in test_shell_tests. The ui thread would TearDown() the test object, which deleted the TestURLRequestContext, which eventually deletes the TCPClientSocketPool, which deletes its ConnectJobs. However, those ConnectJobs might be running simultaneously on the io thread. Therefore, we have a race condition. This change fixes that.
Histograms for the 4.0.266.0 dev channel release indicate the following changes for late binding:
(a) Net.TCPSocketType shows a decrease (from 41.85% to 39.29%) in used of newly connected sockets. Part of this decrease is due to using previously used sockets more often (increase from 58.15% to 58.53%), but is primarily due to being able to use sockets that were connected, but not immediately handed over to a socket request (increased from 0 [not supported without late binding] to 2.18%).
(b) Net.SocketIdleTimeBeforeNextUse_ReusedSocket indicates that reused sockets are getting used more quickly than before, with a decrease of mean idle time from 11.65 seconds to 11.34 seconds.
(c) Net.Transaction_Connected_Under_10 indicates shows that the mean for time until the first byte of the transaction response decreased from 1585ms to 1481ms.
The code change deletes the old non socket late binding code paths, cleaning up the code significantly. It also deletes duplicated tests in ClientSocketPoolBase which covered both pathways. A TCPClientSocketPool test had to be updated as well.
BUG=http://crbug.com/30354.
Review URL: http://codereview.chromium.org/549093
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@37311 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/socket/client_socket_pool_base.cc | 165 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base.h | 39 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base_unittest.cc | 539 | ||||
-rw-r--r-- | net/socket/tcp_client_socket_pool.cc | 5 | ||||
-rw-r--r-- | net/socket/tcp_client_socket_pool.h | 1 | ||||
-rw-r--r-- | net/socket/tcp_client_socket_pool_unittest.cc | 8 |
6 files changed, 72 insertions, 685 deletions
diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc index 037c9ff..9b14f4b 100644 --- a/net/socket/client_socket_pool_base.cc +++ b/net/socket/client_socket_pool_base.cc @@ -32,17 +32,14 @@ const int kMaxNumLoadLogEntries = 50; namespace net { ConnectJob::ConnectJob(const std::string& group_name, - const ClientSocketHandle* key_handle, base::TimeDelta timeout_duration, Delegate* delegate, LoadLog* load_log) : group_name_(group_name), - key_handle_(key_handle), timeout_duration_(timeout_duration), delegate_(delegate), load_log_(load_log) { DCHECK(!group_name.empty()); - DCHECK(key_handle); DCHECK(delegate); } @@ -93,8 +90,6 @@ void ConnectJob::OnTimeout() { namespace internal { -bool ClientSocketPoolBaseHelper::g_late_binding = false; - ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper( int max_sockets, int max_sockets_per_group, @@ -120,14 +115,14 @@ ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper( } ClientSocketPoolBaseHelper::~ClientSocketPoolBaseHelper() { - if (g_late_binding) - CancelAllConnectJobs(); + CancelAllConnectJobs(); + // Clean up any idle sockets. Assert that we have no remaining active // sockets or pending requests. They should have all been cleaned up prior // to the manager being destroyed. CloseIdleSockets(); DCHECK(group_map_.empty()); - DCHECK(connect_job_map_.empty()); + DCHECK_EQ(0, connecting_socket_count_); if (network_change_notifier_) network_change_notifier_->RemoveObserver(this); @@ -156,7 +151,7 @@ ClientSocketPoolBaseHelper::RemoveRequestFromQueue( const Request* req = *it; LoadLog::EndEvent(req->load_log(), - LoadLog::TYPE_SOCKET_POOL_WAITING_IN_QUEUE); + LoadLog::TYPE_SOCKET_POOL_WAITING_IN_QUEUE); pending_requests->erase(it); return req; @@ -208,17 +203,13 @@ int ClientSocketPoolBaseHelper::RequestSocket( // See if we already have enough connect jobs or sockets that will be released // soon. - if (g_late_binding && group.HasReleasingSockets()) { + if (group.HasReleasingSockets()) { InsertRequestIntoQueue(request, &group.pending_requests); return ERR_IO_PENDING; } // We couldn't find a socket to reuse, so allocate and connect a new one. - - // If we aren't using late binding, the job lines up with a request so - // just write directly into the request's LoadLog. - scoped_refptr<LoadLog> job_load_log = g_late_binding ? - new LoadLog(kMaxNumLoadLogEntries) : request->load_log(); + scoped_refptr<LoadLog> job_load_log = new LoadLog(kMaxNumLoadLogEntries); scoped_ptr<ConnectJob> connect_job( connect_job_factory_->NewConnectJob(group_name, *request, this, @@ -226,7 +217,7 @@ int ClientSocketPoolBaseHelper::RequestSocket( int rv = connect_job->Connect(); - if (g_late_binding && rv != ERR_IO_PENDING && request->load_log()) + if (rv != ERR_IO_PENDING && request->load_log()) request->load_log()->Append(job_load_log); if (rv == OK) { @@ -236,14 +227,7 @@ int ClientSocketPoolBaseHelper::RequestSocket( connecting_socket_count_++; ConnectJob* job = connect_job.release(); - if (g_late_binding) { - CHECK(!ContainsKey(connect_job_map_, handle)); - InsertRequestIntoQueue(request, &group.pending_requests); - } else { - group.connecting_requests[handle] = request; - CHECK(!ContainsKey(connect_job_map_, handle)); - connect_job_map_[handle] = job; - } + InsertRequestIntoQueue(request, &group.pending_requests); group.jobs.insert(job); } else if (group.IsEmpty()) { group_map_.erase(group_name); @@ -266,29 +250,14 @@ void ClientSocketPoolBaseHelper::CancelRequest( LoadLog::AddEvent(req->load_log(), LoadLog::TYPE_CANCELLED); LoadLog::EndEvent(req->load_log(), LoadLog::TYPE_SOCKET_POOL); delete req; - if (g_late_binding && - group.jobs.size() > group.pending_requests.size() + 1) { + if (group.jobs.size() > group.pending_requests.size() + 1) { // TODO(willchan): Cancel the job in the earliest LoadState. - RemoveConnectJob(handle, *group.jobs.begin(), &group); + RemoveConnectJob(*group.jobs.begin(), &group); OnAvailableSocketSlot(group_name, &group); } return; } } - - if (!g_late_binding) { - // It's invalid to cancel a non-existent request. - CHECK(ContainsKey(group.connecting_requests, handle)); - - RequestMap::iterator map_it = group.connecting_requests.find(handle); - if (map_it != group.connecting_requests.end()) { - scoped_refptr<LoadLog> log(map_it->second->load_log()); - LoadLog::AddEvent(log, LoadLog::TYPE_CANCELLED); - LoadLog::EndEvent(log, LoadLog::TYPE_SOCKET_POOL); - RemoveConnectJob(handle, NULL, &group); - OnAvailableSocketSlot(group_name, &group); - } - } } void ClientSocketPoolBaseHelper::ReleaseSocket(const std::string& group_name, @@ -327,22 +296,11 @@ LoadState ClientSocketPoolBaseHelper::GetLoadState( // Can't use operator[] since it is non-const. const Group& group = group_map_.find(group_name)->second; - // Search connecting_requests for matching handle. - RequestMap::const_iterator map_it = group.connecting_requests.find(handle); - if (map_it != group.connecting_requests.end()) { - ConnectJobMap::const_iterator job_it = connect_job_map_.find(handle); - if (job_it == connect_job_map_.end()) { - NOTREACHED(); - return LOAD_STATE_IDLE; - } - return job_it->second->GetLoadState(); - } - // Search pending_requests for matching handle. RequestQueue::const_iterator it = group.pending_requests.begin(); for (size_t i = 0; it != group.pending_requests.end(); ++it, ++i) { if ((*it)->handle() == handle) { - if (g_late_binding && i < group.jobs.size()) { + if (i < group.jobs.size()) { LoadState max_state = LOAD_STATE_IDLE; for (ConnectJobSet::const_iterator job_it = group.jobs.begin(); job_it != group.jobs.end(); ++job_it) { @@ -480,67 +438,38 @@ void ClientSocketPoolBaseHelper::OnConnectJobComplete( CHECK(group_it != group_map_.end()); Group& group = group_it->second; - const ClientSocketHandle* const key_handle = job->key_handle(); scoped_ptr<ClientSocket> socket(job->ReleaseSocket()); - if (g_late_binding) { - scoped_refptr<LoadLog> job_load_log(job->load_log()); - RemoveConnectJob(key_handle, job, &group); + scoped_refptr<LoadLog> job_load_log(job->load_log()); + RemoveConnectJob(job, &group); + + LoadLog::EndEvent(job_load_log, LoadLog::TYPE_SOCKET_POOL); - scoped_ptr<const Request> r; + if (result == OK) { + DCHECK(socket.get()); if (!group.pending_requests.empty()) { - r.reset(RemoveRequestFromQueue( + scoped_ptr<const Request> r(RemoveRequestFromQueue( group.pending_requests.begin(), &group.pending_requests)); - if (r->load_log()) r->load_log()->Append(job_load_log); - - LoadLog::EndEvent(r->load_log(), LoadLog::TYPE_SOCKET_POOL); - } - - if (result == OK) { - DCHECK(socket.get()); - if (r.get()) { - HandOutSocket( - socket.release(), false /* unused socket */, r->handle(), - base::TimeDelta(), &group); - r->callback()->Run(result); - } else { - AddIdleSocket(socket.release(), false /* unused socket */, &group); - OnAvailableSocketSlot(group_name, &group); - } + HandOutSocket( + socket.release(), false /* unused socket */, r->handle(), + base::TimeDelta(), &group); + r->callback()->Run(result); } else { - DCHECK(!socket.get()); - if (r.get()) - r->callback()->Run(result); - MaybeOnAvailableSocketSlot(group_name); + AddIdleSocket(socket.release(), false /* unused socket */, &group); + OnAvailableSocketSlot(group_name, &group); } - - return; - } - - RequestMap* request_map = &group.connecting_requests; - RequestMap::iterator it = request_map->find(key_handle); - CHECK(it != request_map->end()); - const Request* request = it->second; - ClientSocketHandle* const handle = request->handle(); - CompletionCallback* const callback = request->callback(); - - LoadLog::EndEvent(request->load_log(), LoadLog::TYPE_SOCKET_POOL); - - RemoveConnectJob(key_handle, job, &group); - - if (result != OK) { + } else { DCHECK(!socket.get()); - callback->Run(result); // |group| is not necessarily valid after this. - // |group| may be invalid after the callback, we need to search - // |group_map_| again. + if (!group.pending_requests.empty()) { + scoped_ptr<const Request> r(RemoveRequestFromQueue( + group.pending_requests.begin(), &group.pending_requests)); + if (r->load_log()) + r->load_log()->Append(job_load_log); + r->callback()->Run(result); + } MaybeOnAvailableSocketSlot(group_name); - } else { - DCHECK(socket.get()); - HandOutSocket(socket.release(), false /* not reused */, handle, - base::TimeDelta(), &group); - callback->Run(result); } } @@ -548,30 +477,13 @@ void ClientSocketPoolBaseHelper::OnIPAddressChanged() { CloseIdleSockets(); } -void ClientSocketPoolBaseHelper::EnableLateBindingOfSockets(bool enabled) { - g_late_binding = enabled; -} - -void ClientSocketPoolBaseHelper::RemoveConnectJob( - const ClientSocketHandle* handle, const ConnectJob *job, Group* group) { +void ClientSocketPoolBaseHelper::RemoveConnectJob(const ConnectJob *job, + Group* group) { CHECK(connecting_socket_count_ > 0); connecting_socket_count_--; - if (g_late_binding) { - DCHECK(job); - delete job; - } else { - ConnectJobMap::iterator it = connect_job_map_.find(handle); - CHECK(it != connect_job_map_.end()); - job = it->second; - delete job; - connect_job_map_.erase(it); - RequestMap::iterator map_it = group->connecting_requests.find(handle); - CHECK(map_it != group->connecting_requests.end()); - const Request* request = map_it->second; - delete request; - group->connecting_requests.erase(map_it); - } + DCHECK(job); + delete job; if (group) { DCHECK(ContainsKey(group->jobs, job)); @@ -659,6 +571,7 @@ void ClientSocketPoolBaseHelper::AddIdleSocket( void ClientSocketPoolBaseHelper::CancelAllConnectJobs() { for (GroupMap::iterator i = group_map_.begin(); i != group_map_.end();) { Group& group = i->second; + connecting_socket_count_ -= group.jobs.size(); STLDeleteElements(&group.jobs); // Delete group if no longer needed. @@ -679,8 +592,4 @@ bool ClientSocketPoolBaseHelper::ReachedMaxSocketsLimit() const { } // namespace internal -void EnableLateBindingOfSockets(bool enabled) { - internal::ClientSocketPoolBaseHelper::EnableLateBindingOfSockets(enabled); -} - } // namespace net diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h index 8132065..8d3fba7 100644 --- a/net/socket/client_socket_pool_base.h +++ b/net/socket/client_socket_pool_base.h @@ -65,7 +65,6 @@ class ConnectJob { // A |timeout_duration| of 0 corresponds to no timeout. ConnectJob(const std::string& group_name, - const ClientSocketHandle* key_handle, base::TimeDelta timeout_duration, Delegate* delegate, LoadLog* load_log); @@ -73,7 +72,6 @@ class ConnectJob { // Accessors const std::string& group_name() const { return group_name_; } - const ClientSocketHandle* key_handle() const { return key_handle_; } LoadLog* load_log() { return load_log_; } // Releases |socket_| to the client. On connection error, this should return @@ -102,8 +100,6 @@ class ConnectJob { void OnTimeout(); const std::string group_name_; - // Temporarily needed until we switch to late binding. - const ClientSocketHandle* const key_handle_; const base::TimeDelta timeout_duration_; // Timer to abort jobs that take too long. base::OneShotTimer<ConnectJob> timer_; @@ -209,14 +205,6 @@ class ClientSocketPoolBaseHelper // NetworkChangeNotifier::Observer methods: virtual void OnIPAddressChanged(); - // Enables late binding of sockets. In this mode, socket requests are - // decoupled from socket connection jobs. A socket request may initiate a - // socket connection job, but there is no guarantee that that socket - // connection will service the request (for example, a released socket may - // service the request sooner, or a higher priority request may come in - // afterward and receive the socket from the job). - static void EnableLateBindingOfSockets(bool enabled); - // For testing. bool may_have_stalled_group() const { return may_have_stalled_group_; } @@ -267,7 +255,7 @@ class ClientSocketPoolBaseHelper bool IsEmpty() const { return active_socket_count == 0 && idle_sockets.empty() && jobs.empty() && - pending_requests.empty() && connecting_requests.empty(); + pending_requests.empty(); } bool HasAvailableSocketSlot(int max_sockets_per_group) const { @@ -286,7 +274,6 @@ class ClientSocketPoolBaseHelper std::deque<IdleSocket> idle_sockets; std::set<const ConnectJob*> jobs; RequestQueue pending_requests; - RequestMap connecting_requests; int active_socket_count; // number of active sockets used by clients // Number of sockets being released within one loop through the MessageLoop. int num_releasing_sockets; @@ -294,7 +281,6 @@ class ClientSocketPoolBaseHelper typedef std::map<std::string, Group> GroupMap; - typedef std::map<const ClientSocketHandle*, ConnectJob*> ConnectJobMap; typedef std::set<const ConnectJob*> ConnectJobSet; static void InsertRequestIntoQueue(const Request* r, @@ -321,14 +307,8 @@ class ClientSocketPoolBaseHelper CleanupIdleSockets(false); } - // Removes the ConnectJob corresponding to |handle| from the - // |connect_job_map_| or |connect_job_set_| depending on whether or not late - // binding is enabled. |job| must be non-NULL when late binding is - // enabled. Also updates |group| if non-NULL. When late binding is disabled, - // this will also delete the Request from |group->connecting_requests|. - void RemoveConnectJob(const ClientSocketHandle* handle, - const ConnectJob* job, - Group* group); + // Removes |job| from |connect_job_set_|. Also updates |group| if non-NULL. + void RemoveConnectJob(const ConnectJob* job, Group* group); // Same as OnAvailableSocketSlot except it looks up the Group first to see if // it's there. @@ -362,8 +342,6 @@ class ClientSocketPoolBaseHelper GroupMap group_map_; - ConnectJobMap connect_job_map_; - // Timer used to periodically prune idle sockets that timed out or can't be // reused. base::RepeatingTimer<ClientSocketPoolBaseHelper> timer_; @@ -406,9 +384,6 @@ class ClientSocketPoolBaseHelper const scoped_ptr<ConnectJobFactory> connect_job_factory_; const scoped_refptr<NetworkChangeNotifier> network_change_notifier_; - - // Controls whether or not we use late binding of sockets. - static bool g_late_binding; }; } // namespace internal @@ -578,14 +553,6 @@ class ClientSocketPoolBase { DISALLOW_COPY_AND_ASSIGN(ClientSocketPoolBase); }; -// Enables late binding of sockets. In this mode, socket requests are -// decoupled from socket connection jobs. A socket request may initiate a -// socket connection job, but there is no guarantee that that socket -// connection will service the request (for example, a released socket may -// service the request sooner, or a higher priority request may come in -// afterward and receive the socket from the job). -void EnableLateBindingOfSockets(bool enabled); - } // namespace net #endif // NET_SOCKET_CLIENT_SOCKET_POOL_BASE_H_ diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc index 7acebbd..495a7f2 100644 --- a/net/socket/client_socket_pool_base_unittest.cc +++ b/net/socket/client_socket_pool_base_unittest.cc @@ -115,8 +115,7 @@ class TestConnectJob : public ConnectJob { ConnectJob::Delegate* delegate, MockClientSocketFactory* client_socket_factory, LoadLog* load_log) - : ConnectJob(group_name, request.handle(), timeout_duration, - delegate, load_log), + : ConnectJob(group_name, timeout_duration, delegate, load_log), job_type_(job_type), client_socket_factory_(client_socket_factory), method_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), @@ -427,8 +426,6 @@ class ClientSocketPoolBaseTest : public ClientSocketPoolTest { pool_ = NULL; requests_.reset(); - EnableLateBindingOfSockets(false); - ClientSocketPoolTest::TearDown(); } @@ -516,29 +513,6 @@ TEST_F(ClientSocketPoolBaseTest, BasicSynchronous) { ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); } -TEST_F(ClientSocketPoolBaseTest, BasicAsynchronous) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); - TestSocketRequest req(&request_order_, &completion_count_); - int rv = InitHandle(req.handle(), "a", LOW, &req, pool_.get(), log); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(LOAD_STATE_CONNECTING, pool_->GetLoadState("a", req.handle())); - EXPECT_EQ(OK, req.WaitForResult()); - EXPECT_TRUE(req.handle()->is_initialized()); - EXPECT_TRUE(req.handle()->socket()); - req.handle()->Reset(); - - EXPECT_EQ(4u, log->entries().size()); - ExpectLogContains(log, 0, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 1, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 2, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_END); - ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); -} - TEST_F(ClientSocketPoolBaseTest, InitConnectionFailure) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); @@ -558,27 +532,6 @@ TEST_F(ClientSocketPoolBaseTest, InitConnectionFailure) { ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); } -TEST_F(ClientSocketPoolBaseTest, InitConnectionAsynchronousFailure) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingFailingJob); - scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); - TestSocketRequest req(&request_order_, &completion_count_); - EXPECT_EQ(ERR_IO_PENDING, - InitHandle(req.handle(), "a", kDefaultPriority, &req, - pool_.get(), log)); - EXPECT_EQ(LOAD_STATE_CONNECTING, pool_->GetLoadState("a", req.handle())); - EXPECT_EQ(ERR_CONNECTION_FAILED, req.WaitForResult()); - - EXPECT_EQ(4u, log->entries().size()); - ExpectLogContains(log, 0, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 1, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 2, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_END); - ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); -} - TEST_F(ClientSocketPoolBaseTest, TotalLimit) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); @@ -881,26 +834,6 @@ TEST_F(ClientSocketPoolBaseTest, CancelRequestClearGroup) { req.handle()->Reset(); } -TEST_F(ClientSocketPoolBaseTest, TwoRequestsCancelOne) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - TestSocketRequest req(&request_order_, &completion_count_); - TestSocketRequest req2(&request_order_, &completion_count_); - - EXPECT_EQ(ERR_IO_PENDING, - InitHandle(req.handle(), "a", kDefaultPriority, &req, - pool_.get(), NULL)); - EXPECT_EQ(ERR_IO_PENDING, - InitHandle(req2.handle(), "a", kDefaultPriority, &req2, - pool_.get(), NULL)); - - req.handle()->Reset(); - - EXPECT_EQ(OK, req2.WaitForResult()); - req2.handle()->Reset(); -} - TEST_F(ClientSocketPoolBaseTest, ConnectCancelConnect) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); @@ -1135,93 +1068,6 @@ TEST_F(ClientSocketPoolBaseTest, CancelActiveRequestThenRequestSocket) { EXPECT_EQ(2, client_socket_factory_.allocation_count()); } -// A pending asynchronous job completes, which will free up a socket slot. The -// next job finishes synchronously. The callback for the asynchronous job -// should be first though. -TEST_F(ClientSocketPoolBaseTest, PendingJobCompletionOrder) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - // First two jobs are async. - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingFailingJob); - - // Start job 1 (async error). - TestSocketRequest req1(&request_order_, &completion_count_); - int rv = InitHandle(req1.handle(), "a", kDefaultPriority, &req1, - pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - - // Start job 2 (async error). - TestSocketRequest req2(&request_order_, &completion_count_); - rv = InitHandle(req2.handle(), "a", kDefaultPriority, &req2, - pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - - // The pending job is sync. - connect_job_factory_->set_job_type(TestConnectJob::kMockJob); - - // Request 3 does not have a ConnectJob yet. It's just pending. - TestSocketRequest req3(&request_order_, &completion_count_); - rv = InitHandle( - req3.handle(), "a", kDefaultPriority, &req3, pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - - EXPECT_EQ(ERR_CONNECTION_FAILED, req1.WaitForResult()); - EXPECT_EQ(ERR_CONNECTION_FAILED, req2.WaitForResult()); - EXPECT_EQ(OK, req3.WaitForResult()); - - ASSERT_EQ(3U, request_order_.size()); - - // After job 1 finishes unsuccessfully, it will try to process the pending - // requests queue, so it starts up job 3 for request 3. This job - // synchronously succeeds, so the request order is 1, 3, 2. - EXPECT_EQ(&req1, request_order_[0]); - EXPECT_EQ(&req2, request_order_[2]); - EXPECT_EQ(&req3, request_order_[1]); -} - -// When a ConnectJob is coupled to a request, even if a free socket becomes -// available, the request will be serviced by the ConnectJob. -TEST_F(ClientSocketPoolBaseTest, ReleaseSockets) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - EnableLateBindingOfSockets(false); - - // Start job 1 (async OK) - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - - TestSocketRequest req1(&request_order_, &completion_count_); - int rv = InitHandle(req1.handle(), "a", kDefaultPriority, &req1, - pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(OK, req1.WaitForResult()); - - // Job 1 finished OK. Start job 2 (also async OK). Release socket 1. - connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob); - - TestSocketRequest req2(&request_order_, &completion_count_); - rv = InitHandle(req2.handle(), "a", kDefaultPriority, &req2, - pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - req1.handle()->Reset(); - MessageLoop::current()->RunAllPending(); // Run the DoReleaseSocket() - - // Job 2 is pending. Start request 3 (which has no associated job since it - // will use the idle socket). - - TestSocketRequest req3(&request_order_, &completion_count_); - rv = InitHandle(req3.handle(), "a", kDefaultPriority, &req3, - pool_.get(), NULL); - EXPECT_EQ(OK, rv); - - EXPECT_FALSE(req2.handle()->socket()); - client_socket_factory_.SignalJobs(); - EXPECT_EQ(OK, req2.WaitForResult()); - - ASSERT_EQ(2U, request_order_.size()); - EXPECT_EQ(&req1, request_order_[0]); - EXPECT_EQ(&req2, request_order_[1]); - EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a")); -} - // Regression test for http://crbug.com/17985. TEST_F(ClientSocketPoolBaseTest, GroupWithPendingRequestsIsNotEmpty) { const int kMaxSockets = 3; @@ -1261,70 +1107,10 @@ class ClientSocketPoolBaseTest_LateBinding : public ClientSocketPoolBaseTest { protected: virtual void SetUp() { ClientSocketPoolBaseTest::SetUp(); - EnableLateBindingOfSockets(true); } }; -// Even though a timeout is specified, it doesn't time out on a synchronous -// completion. -TEST_F(ClientSocketPoolBaseTest_LateBinding, - ConnectJob_NoTimeoutOnSynchronousCompletion) { - TestConnectJobDelegate delegate; - ClientSocketHandle ignored; - TestClientSocketPoolBase::Request request(&ignored, NULL, LOWEST, NULL, - NULL); - scoped_ptr<TestConnectJob> job( - new TestConnectJob(TestConnectJob::kMockJob, - "a", - request, - base::TimeDelta::FromMicroseconds(1), - &delegate, - &client_socket_factory_, - NULL)); - EXPECT_EQ(OK, job->Connect()); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, ConnectJob_TimedOut) { - TestConnectJobDelegate delegate; - ClientSocketHandle ignored; - TestClientSocketPoolBase::Request request(&ignored, NULL, LOWEST, NULL, - NULL); - // Deleted by TestConnectJobDelegate. - TestConnectJob* job = - new TestConnectJob(TestConnectJob::kMockPendingJob, - "a", - request, - base::TimeDelta::FromMicroseconds(1), - &delegate, - &client_socket_factory_, - NULL); - ASSERT_EQ(ERR_IO_PENDING, job->Connect()); - PlatformThread::Sleep(1); - EXPECT_EQ(ERR_TIMED_OUT, delegate.WaitForResult()); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, BasicSynchronous) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - TestCompletionCallback callback; - ClientSocketHandle handle; - scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); - EXPECT_EQ(OK, InitHandle(&handle, "a", kDefaultPriority, &callback, - pool_.get(), log)); - EXPECT_TRUE(handle.is_initialized()); - EXPECT_TRUE(handle.socket()); - handle.Reset(); - - EXPECT_EQ(4u, log->entries().size()); - ExpectLogContains(log, 0, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 1, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 2, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_END); - ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, BasicAsynchronous) { +TEST_F(ClientSocketPoolBaseTest, BasicAsynchronous) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); @@ -1339,38 +1125,22 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, BasicAsynchronous) { req.handle()->Reset(); EXPECT_EQ(6u, log->entries().size()); - ExpectLogContains(log, 0, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 1, LoadLog::TYPE_SOCKET_POOL_WAITING_IN_QUEUE, - LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 2, LoadLog::TYPE_SOCKET_POOL_WAITING_IN_QUEUE, - LoadLog::PHASE_END); - ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 4, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_END); - ExpectLogContains(log, 5, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, InitConnectionFailure) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockFailingJob); - TestSocketRequest req(&request_order_, &completion_count_); - scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); - EXPECT_EQ(ERR_CONNECTION_FAILED, - InitHandle(req.handle(), "a", kDefaultPriority, &req, - pool_.get(), log)); - - EXPECT_EQ(4u, log->entries().size()); - ExpectLogContains(log, 0, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 1, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_BEGIN); - ExpectLogContains(log, 2, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, - LoadLog::PHASE_END); - ExpectLogContains(log, 3, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); + EXPECT_TRUE(LogContains( + *log, 0, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_BEGIN)); + EXPECT_TRUE(LogContains( + *log, 1, LoadLog::TYPE_SOCKET_POOL_WAITING_IN_QUEUE, + LoadLog::PHASE_BEGIN)); + EXPECT_TRUE(LogContains( + *log, 2, LoadLog::TYPE_SOCKET_POOL_WAITING_IN_QUEUE, LoadLog::PHASE_END)); + EXPECT_TRUE(LogContains( + *log, 3, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, LoadLog::PHASE_BEGIN)); + EXPECT_TRUE(LogContains( + *log, 4, LoadLog::TYPE_SOCKET_POOL_CONNECT_JOB, LoadLog::PHASE_END)); + EXPECT_TRUE(LogContains( + *log, 5, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END)); } -TEST_F(ClientSocketPoolBaseTest_LateBinding, +TEST_F(ClientSocketPoolBaseTest, InitConnectionAsynchronousFailure) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); @@ -1396,71 +1166,7 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, ExpectLogContains(log, 5, LoadLog::TYPE_SOCKET_POOL, LoadLog::PHASE_END); } -TEST_F(ClientSocketPoolBaseTest_LateBinding, PendingRequests) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOWEST)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", MEDIUM)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", HIGHEST)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOW)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOWEST)); - - ReleaseAllConnections(KEEP_ALIVE); - - EXPECT_EQ(kDefaultMaxSocketsPerGroup, - client_socket_factory_.allocation_count()); - EXPECT_EQ(requests_.size() - kDefaultMaxSocketsPerGroup, completion_count_); - - EXPECT_EQ(1, GetOrderOfRequest(1)); - EXPECT_EQ(2, GetOrderOfRequest(2)); - EXPECT_EQ(6, GetOrderOfRequest(3)); - EXPECT_EQ(4, GetOrderOfRequest(4)); - EXPECT_EQ(3, GetOrderOfRequest(5)); - EXPECT_EQ(5, GetOrderOfRequest(6)); - EXPECT_EQ(7, GetOrderOfRequest(7)); - - // Make sure we test order of all requests made. - EXPECT_EQ(kIndexOutOfBounds, GetOrderOfRequest(8)); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, PendingRequests_NoKeepAlive) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOWEST)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", MEDIUM)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", HIGHEST)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOW)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOWEST)); - - ReleaseAllConnections(NO_KEEP_ALIVE); - - for (size_t i = kDefaultMaxSocketsPerGroup; i < requests_.size(); ++i) - EXPECT_EQ(OK, requests_[i]->WaitForResult()); - - EXPECT_EQ(static_cast<int>(requests_.size()), - client_socket_factory_.allocation_count()); - EXPECT_EQ(requests_.size() - kDefaultMaxSocketsPerGroup, completion_count_); -} - -// This test will start up a RequestSocket() and then immediately Cancel() it. -// The pending connect job will be cancelled and should not call back into -// ClientSocketPoolBase. -TEST_F(ClientSocketPoolBaseTest_LateBinding, CancelRequestClearGroup) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - TestSocketRequest req(&request_order_, &completion_count_); - EXPECT_EQ(ERR_IO_PENDING, - InitHandle(req.handle(), "a", kDefaultPriority, &req, - pool_.get(), NULL)); - req.handle()->Reset(); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, TwoRequestsCancelOne) { +TEST_F(ClientSocketPoolBaseTest, TwoRequestsCancelOne) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); @@ -1511,67 +1217,7 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, TwoRequestsCancelOne) { } -TEST_F(ClientSocketPoolBaseTest_LateBinding, ConnectCancelConnect) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - ClientSocketHandle handle; - TestCompletionCallback callback; - TestSocketRequest req(&request_order_, &completion_count_); - - EXPECT_EQ(ERR_IO_PENDING, - InitHandle(&handle, "a", kDefaultPriority, &callback, - pool_.get(), NULL)); - - handle.Reset(); - - TestCompletionCallback callback2; - EXPECT_EQ(ERR_IO_PENDING, - InitHandle(&handle, "a", kDefaultPriority, &callback2, - pool_.get(), NULL)); - - EXPECT_EQ(OK, callback2.WaitForResult()); - EXPECT_FALSE(callback.have_result()); - - handle.Reset(); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, CancelRequest) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOWEST)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", MEDIUM)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", HIGHEST)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOW)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", LOWEST)); - - // Cancel a request. - size_t index_to_cancel = kDefaultMaxSocketsPerGroup + 2; - EXPECT_FALSE(requests_[index_to_cancel]->handle()->is_initialized()); - requests_[index_to_cancel]->handle()->Reset(); - - ReleaseAllConnections(KEEP_ALIVE); - - EXPECT_EQ(kDefaultMaxSocketsPerGroup, - client_socket_factory_.allocation_count()); - EXPECT_EQ(requests_.size() - kDefaultMaxSocketsPerGroup - 1, - completion_count_); - - EXPECT_EQ(1, GetOrderOfRequest(1)); - EXPECT_EQ(2, GetOrderOfRequest(2)); - EXPECT_EQ(5, GetOrderOfRequest(3)); - EXPECT_EQ(3, GetOrderOfRequest(4)); - EXPECT_EQ(kRequestNotFound, GetOrderOfRequest(5)); // Canceled request. - EXPECT_EQ(4, GetOrderOfRequest(6)); - EXPECT_EQ(6, GetOrderOfRequest(7)); - - // Make sure we test order of all requests made. - EXPECT_EQ(kIndexOutOfBounds, GetOrderOfRequest(8)); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, CancelRequestLimitsJobs) { +TEST_F(ClientSocketPoolBaseTest, CancelRequestLimitsJobs) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); @@ -1593,114 +1239,9 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, CancelRequestLimitsJobs) { EXPECT_EQ(kDefaultMaxSocketsPerGroup - 1, pool_->NumConnectJobsInGroup("a")); } -TEST_F(ClientSocketPoolBaseTest_LateBinding, RequestPendingJobTwice) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - ClientSocketHandle handle; - RequestSocketCallback callback( - &handle, pool_.get(), connect_job_factory_, - TestConnectJob::kMockPendingJob); - int rv = InitHandle( - &handle, "a", kDefaultPriority, &callback, pool_.get(), NULL); - ASSERT_EQ(ERR_IO_PENDING, rv); - - EXPECT_EQ(OK, callback.WaitForResult()); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, RequestPendingJobThenSynchronous) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - ClientSocketHandle handle; - RequestSocketCallback callback( - &handle, pool_.get(), connect_job_factory_, TestConnectJob::kMockJob); - int rv = InitHandle( - &handle, "a", kDefaultPriority, &callback, - pool_.get(), NULL); - ASSERT_EQ(ERR_IO_PENDING, rv); - - EXPECT_EQ(OK, callback.WaitForResult()); -} - -// Make sure that pending requests get serviced after active requests get -// cancelled. -TEST_F(ClientSocketPoolBaseTest_LateBinding, - CancelActiveRequestWithPendingRequests) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - - // Now, kDefaultMaxSocketsPerGroup requests should be active. - // Let's cancel them. - for (int i = 0; i < kDefaultMaxSocketsPerGroup; ++i) { - ASSERT_FALSE(requests_[i]->handle()->is_initialized()); - requests_[i]->handle()->Reset(); - } - - // Let's wait for the rest to complete now. - for (size_t i = kDefaultMaxSocketsPerGroup; i < requests_.size(); ++i) { - EXPECT_EQ(OK, requests_[i]->WaitForResult()); - requests_[i]->handle()->Reset(); - } - - EXPECT_EQ(requests_.size() - kDefaultMaxSocketsPerGroup, completion_count_); -} - -// Make sure that pending requests get serviced after active requests fail. -TEST_F(ClientSocketPoolBaseTest_LateBinding, - FailingActiveRequestWithPendingRequests) { - const int kMaxSockets = 5; - CreatePool(kMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingFailingJob); - - const int kNumberOfRequests = 2 * kDefaultMaxSocketsPerGroup + 1; - ASSERT_LE(kNumberOfRequests, kMaxSockets); // Otherwise the test hangs. - - // Queue up all the requests - for (int i = 0; i < kNumberOfRequests; ++i) - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - - for (int i = 0; i < kNumberOfRequests; ++i) - EXPECT_EQ(ERR_CONNECTION_FAILED, requests_[i]->WaitForResult()); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, - CancelActiveRequestThenRequestSocket) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - - connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); - - TestSocketRequest req(&request_order_, &completion_count_); - int rv = InitHandle(req.handle(), "a", kDefaultPriority, &req, - pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - - // Cancel the active request. - req.handle()->Reset(); - - rv = InitHandle(req.handle(), "a", kDefaultPriority, &req, - pool_.get(), NULL); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(OK, req.WaitForResult()); - - EXPECT_FALSE(req.handle()->is_reused()); - EXPECT_EQ(1U, completion_count_); - EXPECT_EQ(2, client_socket_factory_.allocation_count()); -} - // When requests and ConnectJobs are not coupled, the request will get serviced // by whatever comes first. -TEST_F(ClientSocketPoolBaseTest_LateBinding, ReleaseSockets) { +TEST_F(ClientSocketPoolBaseTest, ReleaseSockets) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); // Start job 1 (async OK) @@ -1747,7 +1288,7 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, ReleaseSockets) { // The requests are not coupled to the jobs. So, the requests should finish in // their priority / insertion order. -TEST_F(ClientSocketPoolBaseTest_LateBinding, PendingJobCompletionOrder) { +TEST_F(ClientSocketPoolBaseTest, PendingJobCompletionOrder) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); // First two jobs are async. connect_job_factory_->set_job_type(TestConnectJob::kMockPendingFailingJob); @@ -1780,7 +1321,7 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, PendingJobCompletionOrder) { EXPECT_EQ(&req3, request_order_[2]); } -TEST_F(ClientSocketPoolBaseTest_LateBinding, DISABLED_LoadState) { +TEST_F(ClientSocketPoolBaseTest, DISABLED_LoadState) { CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); connect_job_factory_->set_job_type( TestConnectJob::kMockAdvancingLoadStateJob); @@ -1801,43 +1342,7 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, DISABLED_LoadState) { EXPECT_EQ(LOAD_STATE_WAITING_FOR_CACHE, req2.handle()->GetLoadState()); } -// Regression test for http://crbug.com/17985. -TEST_F(ClientSocketPoolBaseTest_LateBinding, - GroupWithPendingRequestsIsNotEmpty) { - const int kMaxSockets = 3; - const int kMaxSocketsPerGroup = 2; - CreatePool(kMaxSockets, kMaxSocketsPerGroup); - - const RequestPriority kHighPriority = HIGHEST; - - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - EXPECT_EQ(OK, StartRequest("a", kDefaultPriority)); - - // This is going to be a pending request in an otherwise empty group. - EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority)); - - // Reach the maximum socket limit. - EXPECT_EQ(OK, StartRequest("b", kDefaultPriority)); - - // Create a stalled group with high priorities. - EXPECT_EQ(ERR_IO_PENDING, StartRequest("c", kHighPriority)); - EXPECT_EQ(ERR_IO_PENDING, StartRequest("c", kHighPriority)); - EXPECT_TRUE(pool_->base()->may_have_stalled_group()); - - // Release the first two sockets from "a", which will make room - // for requests from "c". After that "a" will have no active sockets - // and one pending request. - EXPECT_TRUE(ReleaseOneConnection(KEEP_ALIVE)); - EXPECT_TRUE(ReleaseOneConnection(KEEP_ALIVE)); - - // Closing idle sockets should not get us into trouble, but in the bug - // we were hitting a CHECK here. - EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a")); - pool_->CloseIdleSockets(); - EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a")); -} - -TEST_F(ClientSocketPoolBaseTest_LateBinding, CleanupTimedOutIdleSockets) { +TEST_F(ClientSocketPoolBaseTest, CleanupTimedOutIdleSockets) { CreatePoolWithIdleTimeouts( kDefaultMaxSockets, kDefaultMaxSocketsPerGroup, base::TimeDelta(), // Time out unused sockets immediately. diff --git a/net/socket/tcp_client_socket_pool.cc b/net/socket/tcp_client_socket_pool.cc index a4e5bec..823cce7 100644 --- a/net/socket/tcp_client_socket_pool.cc +++ b/net/socket/tcp_client_socket_pool.cc @@ -34,13 +34,12 @@ static const int kTCPConnectJobTimeoutInSeconds = 240; // 4 minutes. TCPConnectJob::TCPConnectJob( const std::string& group_name, const HostResolver::RequestInfo& resolve_info, - const ClientSocketHandle* handle, base::TimeDelta timeout_duration, ClientSocketFactory* client_socket_factory, HostResolver* host_resolver, Delegate* delegate, LoadLog* load_log) - : ConnectJob(group_name, handle, timeout_duration, delegate, load_log), + : ConnectJob(group_name, timeout_duration, delegate, load_log), resolve_info_(resolve_info), client_socket_factory_(client_socket_factory), ALLOW_THIS_IN_INITIALIZER_LIST( @@ -162,7 +161,7 @@ ConnectJob* TCPClientSocketPool::TCPConnectJobFactory::NewConnectJob( ConnectJob::Delegate* delegate, LoadLog* load_log) const { return new TCPConnectJob( - group_name, request.params(), request.handle(), + group_name, request.params(), base::TimeDelta::FromSeconds(kTCPConnectJobTimeoutInSeconds), client_socket_factory_, host_resolver_, delegate, load_log); } diff --git a/net/socket/tcp_client_socket_pool.h b/net/socket/tcp_client_socket_pool.h index 4da6ea8..6e36ec3 100644 --- a/net/socket/tcp_client_socket_pool.h +++ b/net/socket/tcp_client_socket_pool.h @@ -26,7 +26,6 @@ class TCPConnectJob : public ConnectJob { public: TCPConnectJob(const std::string& group_name, const HostResolver::RequestInfo& resolve_info, - const ClientSocketHandle* handle, base::TimeDelta timeout_duration, ClientSocketFactory* client_socket_factory, HostResolver* host_resolver, diff --git a/net/socket/tcp_client_socket_pool_unittest.cc b/net/socket/tcp_client_socket_pool_unittest.cc index 500725a..95b37f1 100644 --- a/net/socket/tcp_client_socket_pool_unittest.cc +++ b/net/socket/tcp_client_socket_pool_unittest.cc @@ -509,7 +509,15 @@ class RequestSocketCallback : public CallbackRunner< Tuple1<int> > { ASSERT_EQ(OK, params.a); if (!within_callback_) { + // Don't allow reuse of the socket. Disconnect it and then release it and + // run through the MessageLoop once to get it completely released. + handle_->socket()->Disconnect(); handle_->Reset(); + { + MessageLoop::ScopedNestableTaskAllower nestable( + MessageLoop::current()); + MessageLoop::current()->RunAllPending(); + } within_callback_ = true; int rv = handle_->Init( "a", HostResolver::RequestInfo("www.google.com", 80), LOWEST, |