diff options
Diffstat (limited to 'net/socket')
-rw-r--r-- | net/socket/client_socket_handle.cc | 19 | ||||
-rw-r--r-- | net/socket/client_socket_handle.h | 5 | ||||
-rw-r--r-- | net/socket/client_socket_pool.h | 21 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base.cc | 101 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base.h | 53 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base_unittest.cc | 223 | ||||
-rw-r--r-- | net/socket/socks_client_socket_pool.cc | 27 | ||||
-rw-r--r-- | net/socket/socks_client_socket_pool.h | 12 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_pool.cc | 33 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_pool.h | 10 | ||||
-rw-r--r-- | net/socket/transport_client_socket_pool.cc | 12 | ||||
-rw-r--r-- | net/socket/transport_client_socket_pool.h | 3 |
12 files changed, 33 insertions, 486 deletions
diff --git a/net/socket/client_socket_handle.cc b/net/socket/client_socket_handle.cc index af3c78a..8cc0e90 100644 --- a/net/socket/client_socket_handle.cc +++ b/net/socket/client_socket_handle.cc @@ -17,8 +17,6 @@ namespace net { ClientSocketHandle::ClientSocketHandle() : is_initialized_(false), - pool_(NULL), - layered_pool_(NULL), is_reused_(false), ALLOW_THIS_IN_INITIALIZER_LIST(callback_( base::Bind(&ClientSocketHandle::OnIOComplete, @@ -54,10 +52,6 @@ void ClientSocketHandle::ResetInternal(bool cancel) { group_name_.clear(); is_reused_ = false; user_callback_.Reset(); - if (layered_pool_) { - pool_->RemoveLayeredPool(layered_pool_); - layered_pool_ = NULL; - } pool_ = NULL; idle_time_ = base::TimeDelta(); init_time_ = base::TimeTicks(); @@ -81,19 +75,6 @@ LoadState ClientSocketHandle::GetLoadState() const { return pool_->GetLoadState(group_name_, this); } -bool ClientSocketHandle::IsPoolStalled() const { - return pool_->IsStalled(); -} - -void ClientSocketHandle::AddLayeredPool(LayeredPool* layered_pool) { - CHECK(layered_pool); - CHECK(!layered_pool_); - if (pool_) { - pool_->AddLayeredPool(layered_pool); - layered_pool_ = layered_pool; - } -} - void ClientSocketHandle::OnIOComplete(int result) { CompletionCallback callback = user_callback_; user_callback_.Reset(); diff --git a/net/socket/client_socket_handle.h b/net/socket/client_socket_handle.h index 0bdb8ac..0b9a652 100644 --- a/net/socket/client_socket_handle.h +++ b/net/socket/client_socket_handle.h @@ -92,10 +92,6 @@ class NET_EXPORT ClientSocketHandle { // initialized the ClientSocketHandle. LoadState GetLoadState() const; - bool IsPoolStalled() const; - - void AddLayeredPool(LayeredPool* layered_pool); - // Returns true when Init() has completed successfully. bool is_initialized() const { return is_initialized_; } @@ -168,7 +164,6 @@ class NET_EXPORT ClientSocketHandle { bool is_initialized_; ClientSocketPool* pool_; - LayeredPool* layered_pool_; scoped_ptr<StreamSocket> socket_; std::string group_name_; bool is_reused_; diff --git a/net/socket/client_socket_pool.h b/net/socket/client_socket_pool.h index a13534a..92f0c413 100644 --- a/net/socket/client_socket_pool.h +++ b/net/socket/client_socket_pool.h @@ -29,17 +29,6 @@ class ClientSocketHandle; class ClientSocketPoolHistograms; class StreamSocket; -// ClientSocketPools are layered. This defines an interface for lower level -// socket pools to communicate with higher layer pools. -class NET_EXPORT LayeredPool { - public: - virtual ~LayeredPool() {} - - // Instructs the LayeredPool to close an idle connection. Return true if one - // was closed. - virtual bool CloseOneIdleConnection() = 0; -}; - // A ClientSocketPool is used to restrict the number of sockets open at a time. // It also maintains a list of idle persistent sockets. // @@ -121,10 +110,6 @@ class NET_EXPORT ClientSocketPool { // the pool. Does not flush any pools wrapped by |this|. virtual void Flush() = 0; - // Returns true if a there is currently a request blocked on the - // per-pool (not per-host) max socket limit. - virtual bool IsStalled() const = 0; - // Called to close any idle connections held by the connection manager. virtual void CloseIdleSockets() = 0; @@ -138,12 +123,6 @@ class NET_EXPORT ClientSocketPool { virtual LoadState GetLoadState(const std::string& group_name, const ClientSocketHandle* handle) const = 0; - // Adds a LayeredPool on top of |this|. - virtual void AddLayeredPool(LayeredPool* layered_pool) = 0; - - // Removes a LayeredPool from |this|. - virtual void RemoveLayeredPool(LayeredPool* layered_pool) = 0; - // Retrieves information on the current state of the pool as a // DictionaryValue. Caller takes possession of the returned value. // If |include_nested_pools| is true, the states of any nested diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc index b3417bf..d230ad4 100644 --- a/net/socket/client_socket_pool_base.cc +++ b/net/socket/client_socket_pool_base.cc @@ -207,7 +207,6 @@ ClientSocketPoolBaseHelper::~ClientSocketPoolBaseHelper() { DCHECK(group_map_.empty()); DCHECK(pending_callback_map_.empty()); DCHECK_EQ(0, connecting_socket_count_); - CHECK(higher_layer_pools_.empty()); NetworkChangeNotifier::RemoveIPAddressObserver(this); } @@ -239,18 +238,6 @@ ClientSocketPoolBaseHelper::RemoveRequestFromQueue( return req; } -void ClientSocketPoolBaseHelper::AddLayeredPool(LayeredPool* pool) { - CHECK(pool); - CHECK(!ContainsKey(higher_layer_pools_, pool)); - higher_layer_pools_.insert(pool); -} - -void ClientSocketPoolBaseHelper::RemoveLayeredPool(LayeredPool* pool) { - CHECK(pool); - CHECK(ContainsKey(higher_layer_pools_, pool)); - higher_layer_pools_.erase(pool); -} - int ClientSocketPoolBaseHelper::RequestSocket( const std::string& group_name, const Request* request) { @@ -349,46 +336,26 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal( // Can we make another active socket now? if (!group->HasAvailableSocketSlot(max_sockets_per_group_) && !request->ignore_limits()) { - // TODO(willchan): Consider whether or not we need to close a socket in a - // higher layered group. I don't think this makes sense since we would just - // reuse that socket then if we needed one and wouldn't make it down to this - // layer. request->net_log().AddEvent( NetLog::TYPE_SOCKET_POOL_STALLED_MAX_SOCKETS_PER_GROUP, NULL); return ERR_IO_PENDING; } if (ReachedMaxSocketsLimit() && !request->ignore_limits()) { - // NOTE(mmenke): Wonder if we really need different code for each case - // here. Only reason for them now seems to be preconnects. if (idle_socket_count() > 0) { - // There's an idle socket in this pool. Either that's because there's - // still one in this group, but we got here due to preconnecting bypassing - // idle sockets, or because there's an idle socket in another group. bool closed = CloseOneIdleSocketExceptInGroup(group); if (preconnecting && !closed) return ERR_PRECONNECT_MAX_SOCKET_LIMIT; } else { - do { - if (!CloseOneIdleConnectionInLayeredPool()) { - // We could check if we really have a stalled group here, but it - // requires a scan of all groups, so just flip a flag here, and do - // the check later. - request->net_log().AddEvent( - NetLog::TYPE_SOCKET_POOL_STALLED_MAX_SOCKETS, NULL); - return ERR_IO_PENDING; - } - } while (ReachedMaxSocketsLimit()); - - // It is possible that CloseOneIdleConnectionInLayeredPool() has deleted - // our Group (see http://crbug.com/109876), so look it up again - // to be safe. - group = GetOrCreateGroup(group_name); + // We could check if we really have a stalled group here, but it requires + // a scan of all groups, so just flip a flag here, and do the check later. + request->net_log().AddEvent( + NetLog::TYPE_SOCKET_POOL_STALLED_MAX_SOCKETS, NULL); + return ERR_IO_PENDING; } } - // We couldn't find a socket to reuse, and there's space to allocate one, - // so allocate and connect a new one. + // We couldn't find a socket to reuse, so allocate and connect a new one. scoped_ptr<ConnectJob> connect_job( connect_job_factory_->NewConnectJob(group_name, *request, this)); @@ -650,8 +617,7 @@ DictionaryValue* ClientSocketPoolBaseHelper::GetInfoAsValue( group_dict->Set("connect_jobs", connect_jobs_list); group_dict->SetBoolean("is_stalled", - group->IsStalledOnPoolMaxSockets( - max_sockets_per_group_)); + group->IsStalled(max_sockets_per_group_)); group_dict->SetBoolean("has_backup_job", group->HasBackupJob()); all_groups_dict->SetWithoutPathExpansion(it->first, group_dict); @@ -826,22 +792,18 @@ void ClientSocketPoolBaseHelper::CheckForStalledSocketGroups() { // are not at the |max_sockets_per_group_| limit. Note: for requests with // the same priority, the winner is based on group hash ordering (and not // insertion order). -bool ClientSocketPoolBaseHelper::FindTopStalledGroup( - Group** group, - std::string* group_name) const { - CHECK((group && group_name) || (!group && !group_name)); +bool ClientSocketPoolBaseHelper::FindTopStalledGroup(Group** group, + std::string* group_name) { Group* top_group = NULL; const std::string* top_group_name = NULL; bool has_stalled_group = false; - for (GroupMap::const_iterator i = group_map_.begin(); + for (GroupMap::iterator i = group_map_.begin(); i != group_map_.end(); ++i) { Group* curr_group = i->second; const RequestQueue& queue = curr_group->pending_requests(); if (queue.empty()) continue; - if (curr_group->IsStalledOnPoolMaxSockets(max_sockets_per_group_)) { - if (!group) - return true; + if (curr_group->IsStalled(max_sockets_per_group_)) { has_stalled_group = true; bool has_higher_priority = !top_group || curr_group->TopPendingPriority() < top_group->TopPendingPriority(); @@ -853,11 +815,8 @@ bool ClientSocketPoolBaseHelper::FindTopStalledGroup( } if (top_group) { - CHECK(group); *group = top_group; *group_name = *top_group_name; - } else { - CHECK(!has_stalled_group); } return has_stalled_group; } @@ -930,25 +889,6 @@ void ClientSocketPoolBaseHelper::Flush() { AbortAllRequests(); } -bool ClientSocketPoolBaseHelper::IsStalled() const { - // If we are not using |max_sockets_|, then clearly we are not stalled - if ((handed_out_socket_count_ + connecting_socket_count_) < max_sockets_) - return false; - // So in order to be stalled we need to be using |max_sockets_| AND - // we need to have a request that is actually stalled on the global - // socket limit. To find such a request, we look for a group that - // a has more requests that jobs AND where the number of jobs is less - // than |max_sockets_per_group_|. (If the number of jobs is equal to - // |max_sockets_per_group_|, then the request is stalled on the group, - // which does not count.) - for (GroupMap::const_iterator it = group_map_.begin(); - it != group_map_.end(); it++) { - if (it->second->IsStalledOnPoolMaxSockets(max_sockets_per_group_)) - return true; - } - return false; -} - void ClientSocketPoolBaseHelper::RemoveConnectJob(ConnectJob* job, Group* group) { CHECK_GT(connecting_socket_count_, 0); @@ -1085,10 +1025,8 @@ bool ClientSocketPoolBaseHelper::ReachedMaxSocketsLimit() const { return true; } -bool ClientSocketPoolBaseHelper::CloseOneIdleSocket() { - if (idle_socket_count() == 0) - return false; - return CloseOneIdleSocketExceptInGroup(NULL); +void ClientSocketPoolBaseHelper::CloseOneIdleSocket() { + CloseOneIdleSocketExceptInGroup(NULL); } bool ClientSocketPoolBaseHelper::CloseOneIdleSocketExceptInGroup( @@ -1112,18 +1050,9 @@ bool ClientSocketPoolBaseHelper::CloseOneIdleSocketExceptInGroup( } } - return false; -} + if (!exception_group) + LOG(DFATAL) << "No idle socket found to close!."; -bool ClientSocketPoolBaseHelper::CloseOneIdleConnectionInLayeredPool() { - // This pool doesn't have any idle sockets. It's possible that a pool at a - // higher layer is holding one of this sockets active, but it's actually idle. - // Query the higher layers. - for (std::set<LayeredPool*>::const_iterator it = higher_layer_pools_.begin(); - it != higher_layer_pools_.end(); ++it) { - if ((*it)->CloseOneIdleConnection()) - return true; - } return false; } diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h index 832c166..f550e42 100644 --- a/net/socket/client_socket_pool_base.h +++ b/net/socket/client_socket_pool_base.h @@ -28,7 +28,6 @@ #include <map> #include <set> #include <string> -#include <vector> #include "base/basictypes.h" #include "base/memory/ref_counted.h" @@ -240,11 +239,6 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper virtual ~ClientSocketPoolBaseHelper(); - // Adds/Removes layered pools. It is expected in the destructor that no - // layered pools remain. - void AddLayeredPool(LayeredPool* pool); - void RemoveLayeredPool(LayeredPool* pool); - // See ClientSocketPool::RequestSocket for documentation on this function. // ClientSocketPoolBaseHelper takes ownership of |request|, which must be // heap allocated. @@ -267,9 +261,6 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper // See ClientSocketPool::Flush for documentation on this function. void Flush(); - // See ClientSocketPool::IsStalled for documentation on this function. - bool IsStalled() const; - // See ClientSocketPool::CloseIdleSockets for documentation on this function. void CloseIdleSockets(); @@ -315,16 +306,6 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper // sockets that timed out or can't be reused. Made public for testing. void CleanupIdleSockets(bool force); - // Closes one idle socket. Picks the first one encountered. - // TODO(willchan): Consider a better algorithm for doing this. Perhaps we - // should keep an ordered list of idle sockets, and close them in order. - // Requires maintaining more state. It's not clear if it's worth it since - // I'm not sure if we hit this situation often. - bool CloseOneIdleSocket(); - - // Checks layered pools to see if they can close an idle connection. - bool CloseOneIdleConnectionInLayeredPool(); - // See ClientSocketPool::GetInfoAsValue for documentation on this function. base::DictionaryValue* GetInfoAsValue(const std::string& name, const std::string& type) const; @@ -390,7 +371,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper static_cast<int>(idle_sockets_.size()); } - bool IsStalledOnPoolMaxSockets(int max_sockets_per_group) const { + bool IsStalled(int max_sockets_per_group) const { return HasAvailableSocketSlot(max_sockets_per_group) && pending_requests_.size() > jobs_.size(); } @@ -476,9 +457,9 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper // Scans the group map for groups which have an available socket slot and // at least one pending request. Returns true if any groups are stalled, and - // if so (and if both |group| and |group_name| are not NULL), fills |group| - // and |group_name| with data of the stalled group having highest priority. - bool FindTopStalledGroup(Group** group, std::string* group_name) const; + // if so, fills |group| and |group_name| with data of the stalled group + // having highest priority. + bool FindTopStalledGroup(Group** group, std::string* group_name); // Called when timer_ fires. This method scans the idle sockets removing // sockets that timed out or can't be reused. @@ -530,6 +511,13 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper static void LogBoundConnectJobToRequest( const NetLog::Source& connect_job_source, const Request* request); + // Closes one idle socket. Picks the first one encountered. + // TODO(willchan): Consider a better algorithm for doing this. Perhaps we + // should keep an ordered list of idle sockets, and close them in order. + // Requires maintaining more state. It's not clear if it's worth it since + // I'm not sure if we hit this situation often. + void CloseOneIdleSocket(); + // Same as CloseOneIdleSocket() except it won't close an idle socket in // |group|. If |group| is NULL, it is ignored. Returns true if it closed a // socket. @@ -594,8 +582,6 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper // make sure that they are discarded rather than reused. int pool_generation_number_; - std::set<LayeredPool*> higher_layer_pools_; - base::WeakPtrFactory<ClientSocketPoolBaseHelper> weak_factory_; DISALLOW_COPY_AND_ASSIGN(ClientSocketPoolBaseHelper); @@ -662,13 +648,6 @@ class ClientSocketPoolBase { virtual ~ClientSocketPoolBase() {} // These member functions simply forward to ClientSocketPoolBaseHelper. - void AddLayeredPool(LayeredPool* pool) { - helper_.AddLayeredPool(pool); - } - - void RemoveLayeredPool(LayeredPool* pool) { - helper_.RemoveLayeredPool(pool); - } // RequestSocket bundles up the parameters into a Request and then forwards to // ClientSocketPoolBaseHelper::RequestSocket(). @@ -713,10 +692,6 @@ class ClientSocketPoolBase { return helper_.ReleaseSocket(group_name, socket, id); } - void Flush() { helper_.Flush(); } - - bool IsStalled() const { return helper_.IsStalled(); } - void CloseIdleSockets() { return helper_.CloseIdleSockets(); } int idle_socket_count() const { return helper_.idle_socket_count(); } @@ -765,11 +740,7 @@ class ClientSocketPoolBase { void EnableConnectBackupJobs() { helper_.EnableConnectBackupJobs(); } - bool CloseOneIdleSocket() { return helper_.CloseOneIdleSocket(); } - - bool CloseOneIdleConnectionInLayeredPool() { - return helper_.CloseOneIdleConnectionInLayeredPool(); - } + void Flush() { helper_.Flush(); } private: // This adaptor class exists to bridge the diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc index 6c692fe..b1c1a99 100644 --- a/net/socket/client_socket_pool_base_unittest.cc +++ b/net/socket/client_socket_pool_base_unittest.cc @@ -4,8 +4,6 @@ #include "net/socket/client_socket_pool_base.h" -#include <vector> - #include "base/bind.h" #include "base/bind_helpers.h" #include "base/callback.h" @@ -30,12 +28,8 @@ #include "net/socket/socket_test_util.h" #include "net/socket/ssl_host_info.h" #include "net/socket/stream_socket.h" -#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -using ::testing::Invoke; -using ::testing::Return; - namespace net { namespace { @@ -46,18 +40,10 @@ const net::RequestPriority kDefaultPriority = MEDIUM; class TestSocketParams : public base::RefCounted<TestSocketParams> { public: - TestSocketParams() : ignore_limits_(false) {} - - void set_ignore_limits(bool ignore_limits) { - ignore_limits_ = ignore_limits; - } - bool ignore_limits() { return ignore_limits_; } - + bool ignore_limits() { return false; } private: friend class base::RefCounted<TestSocketParams>; ~TestSocketParams() {} - - bool ignore_limits_; }; typedef ClientSocketPoolBase<TestSocketParams> TestClientSocketPoolBase; @@ -369,18 +355,12 @@ class TestConnectJobFactory public: explicit TestConnectJobFactory(MockClientSocketFactory* client_socket_factory) : job_type_(TestConnectJob::kMockJob), - job_types_(NULL), client_socket_factory_(client_socket_factory) {} virtual ~TestConnectJobFactory() {} void set_job_type(TestConnectJob::JobType job_type) { job_type_ = job_type; } - void set_job_types(std::list<TestConnectJob::JobType>* job_types) { - job_types_ = job_types; - CHECK(!job_types_->empty()); - } - void set_timeout_duration(base::TimeDelta timeout_duration) { timeout_duration_ = timeout_duration; } @@ -391,13 +371,7 @@ class TestConnectJobFactory const std::string& group_name, const TestClientSocketPoolBase::Request& request, ConnectJob::Delegate* delegate) const { - EXPECT_TRUE(!job_types_ || !job_types_->empty()); - TestConnectJob::JobType job_type = job_type_; - if (job_types_ && !job_types_->empty()) { - job_type = job_types_->front(); - job_types_->pop_front(); - } - return new TestConnectJob(job_type, + return new TestConnectJob(job_type_, group_name, request, timeout_duration_, @@ -412,7 +386,6 @@ class TestConnectJobFactory private: TestConnectJob::JobType job_type_; - std::list<TestConnectJob::JobType>* job_types_; base::TimeDelta timeout_duration_; MockClientSocketFactory* const client_socket_factory_; @@ -474,10 +447,6 @@ class TestClientSocketPool : public ClientSocketPool { base_.Flush(); } - virtual bool IsStalled() const OVERRIDE { - return base_.IsStalled(); - } - virtual void CloseIdleSockets() OVERRIDE { base_.CloseIdleSockets(); } @@ -497,14 +466,6 @@ class TestClientSocketPool : public ClientSocketPool { return base_.GetLoadState(group_name, handle); } - virtual void AddLayeredPool(LayeredPool* pool) OVERRIDE { - base_.AddLayeredPool(pool); - } - - virtual void RemoveLayeredPool(LayeredPool* pool) OVERRIDE { - base_.RemoveLayeredPool(pool); - } - virtual DictionaryValue* GetInfoAsValue( const std::string& name, const std::string& type, @@ -538,10 +499,6 @@ class TestClientSocketPool : public ClientSocketPool { void EnableConnectBackupJobs() { base_.EnableConnectBackupJobs(); } - bool CloseOneIdleConnectionInLayeredPool() { - return base_.CloseOneIdleConnectionInLayeredPool(); - } - private: TestClientSocketPoolBase base_; @@ -1207,7 +1164,6 @@ TEST_F(ClientSocketPoolBaseTest, WaitForStalledSocketAtSocketLimit) { ClientSocketHandle stalled_handle; TestCompletionCallback callback; { - EXPECT_FALSE(pool_->IsStalled()); ClientSocketHandle handles[kDefaultMaxSockets]; for (int i = 0; i < kDefaultMaxSockets; ++i) { TestCompletionCallback callback; @@ -1222,7 +1178,6 @@ TEST_F(ClientSocketPoolBaseTest, WaitForStalledSocketAtSocketLimit) { EXPECT_EQ(kDefaultMaxSockets, client_socket_factory_.allocation_count()); EXPECT_EQ(0, pool_->IdleSocketCount()); - EXPECT_FALSE(pool_->IsStalled()); // Now we will hit the socket limit. EXPECT_EQ(ERR_IO_PENDING, stalled_handle.Init("foo", @@ -1231,7 +1186,6 @@ TEST_F(ClientSocketPoolBaseTest, WaitForStalledSocketAtSocketLimit) { callback.callback(), pool_.get(), BoundNetLog())); - EXPECT_TRUE(pool_->IsStalled()); // Dropping out of scope will close all handles and return them to idle. } @@ -2051,9 +2005,9 @@ TEST_F(ClientSocketPoolBaseTest, DisableCleanupTimer) { EXPECT_EQ(1, handle2.socket()->Write(NULL, 1, CompletionCallback())); handle2.Reset(); - // The idle socket timeout value was set to 10 milliseconds. Wait 100 + // The idle socket timeout value was set to 10 milliseconds. Wait 20 // milliseconds so the sockets timeout. - base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100)); + base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(20)); MessageLoop::current()->RunAllPending(); ASSERT_EQ(2, pool_->IdleSocketCount()); @@ -3085,7 +3039,6 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsHitMaxSocketLimit) { ASSERT_TRUE(pool_->HasGroup("a")); EXPECT_EQ(kDefaultMaxSockets - 1, pool_->NumConnectJobsInGroup("a")); - EXPECT_FALSE(pool_->IsStalled()); ASSERT_FALSE(pool_->HasGroup("b")); @@ -3094,7 +3047,6 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsHitMaxSocketLimit) { ASSERT_TRUE(pool_->HasGroup("b")); EXPECT_EQ(1, pool_->NumConnectJobsInGroup("b")); - EXPECT_FALSE(pool_->IsStalled()); } TEST_F(ClientSocketPoolBaseTest, RequestSocketsCountIdleSockets) { @@ -3413,173 +3365,6 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectWithBackupJob) { EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a")); } -class MockLayeredPool : public LayeredPool { - public: - MockLayeredPool(TestClientSocketPool* pool, - const std::string& group_name) - : pool_(pool), - params_(new TestSocketParams), - group_name_(group_name), - can_release_connection_(true) { - pool_->AddLayeredPool(this); - } - - ~MockLayeredPool() { - pool_->RemoveLayeredPool(this); - } - - int RequestSocket(TestClientSocketPool* pool) { - return handle_.Init(group_name_, params_, kDefaultPriority, - callback_.callback(), pool, BoundNetLog()); - } - - int RequestSocketWithoutLimits(TestClientSocketPool* pool) { - params_->set_ignore_limits(true); - return handle_.Init(group_name_, params_, kDefaultPriority, - callback_.callback(), pool, BoundNetLog()); - } - - bool ReleaseOneConnection() { - if (!handle_.is_initialized() || !can_release_connection_) { - return false; - } - handle_.socket()->Disconnect(); - handle_.Reset(); - return true; - } - - void set_can_release_connection(bool can_release_connection) { - can_release_connection_ = can_release_connection; - } - - MOCK_METHOD0(CloseOneIdleConnection, bool()); - - private: - TestClientSocketPool* const pool_; - scoped_refptr<TestSocketParams> params_; - ClientSocketHandle handle_; - TestCompletionCallback callback_; - const std::string group_name_; - bool can_release_connection_; -}; - -TEST_F(ClientSocketPoolBaseTest, FailToCloseIdleSocketsNotHeldByLayeredPool) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - connect_job_factory_->set_job_type(TestConnectJob::kMockJob); - - MockLayeredPool mock_layered_pool(pool_.get(), "foo"); - EXPECT_EQ(OK, mock_layered_pool.RequestSocket(pool_.get())); - EXPECT_CALL(mock_layered_pool, CloseOneIdleConnection()) - .WillOnce(Return(false)); - EXPECT_FALSE(pool_->CloseOneIdleConnectionInLayeredPool()); -} - -TEST_F(ClientSocketPoolBaseTest, ForciblyCloseIdleSocketsHeldByLayeredPool) { - CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup); - connect_job_factory_->set_job_type(TestConnectJob::kMockJob); - - MockLayeredPool mock_layered_pool(pool_.get(), "foo"); - EXPECT_EQ(OK, mock_layered_pool.RequestSocket(pool_.get())); - EXPECT_CALL(mock_layered_pool, CloseOneIdleConnection()) - .WillOnce(Invoke(&mock_layered_pool, - &MockLayeredPool::ReleaseOneConnection)); - EXPECT_TRUE(pool_->CloseOneIdleConnectionInLayeredPool()); -} - -// This test exercises the codepath which caused http://crbug.com/109876 -TEST_F(ClientSocketPoolBaseTest, - CloseIdleSocketsHeldByLayeredPoolInSameGroupWhenNeeded) { - CreatePool(2, 2); - std::list<TestConnectJob::JobType> job_types; - job_types.push_back(TestConnectJob::kMockJob); - job_types.push_back(TestConnectJob::kMockJob); - job_types.push_back(TestConnectJob::kMockFailingJob); - job_types.push_back(TestConnectJob::kMockJob); - connect_job_factory_->set_job_types(&job_types); - - ClientSocketHandle handle1; - TestCompletionCallback callback1; - EXPECT_EQ(OK, handle1.Init("group1", - params_, - kDefaultPriority, - callback1.callback(), - pool_.get(), - BoundNetLog())); - - MockLayeredPool mock_layered_pool(pool_.get(), "group2"); - EXPECT_EQ(OK, mock_layered_pool.RequestSocket(pool_.get())); - EXPECT_CALL(mock_layered_pool, CloseOneIdleConnection()) - .WillRepeatedly(Invoke(&mock_layered_pool, - &MockLayeredPool::ReleaseOneConnection)); - mock_layered_pool.set_can_release_connection(false); - - // This connection attempt will fail when the next request causes the - // MockLayeredPool to delete the socket it's holding. This request is - // needed to trigger the destruction of the "group2" Group. - ClientSocketHandle handle3; - TestCompletionCallback callback3; - EXPECT_EQ(ERR_IO_PENDING, handle3.Init("group2", - params_, - kDefaultPriority, - callback3.callback(), - pool_.get(), - BoundNetLog())); - - mock_layered_pool.set_can_release_connection(true); - ClientSocketHandle handle4; - TestCompletionCallback callback4; - EXPECT_EQ(OK, handle4.Init("group2", - params_, - kDefaultPriority, - callback4.callback(), - pool_.get(), - BoundNetLog())); -} - -TEST_F(ClientSocketPoolBaseTest, CloseIdleSocketsHeldByLayeredPoolWhenNeeded) { - CreatePool(1, 1); - connect_job_factory_->set_job_type(TestConnectJob::kMockJob); - - MockLayeredPool mock_layered_pool(pool_.get(), "foo"); - EXPECT_EQ(OK, mock_layered_pool.RequestSocket(pool_.get())); - EXPECT_CALL(mock_layered_pool, CloseOneIdleConnection()) - .WillOnce(Invoke(&mock_layered_pool, - &MockLayeredPool::ReleaseOneConnection)); - ClientSocketHandle handle; - TestCompletionCallback callback; - EXPECT_EQ(OK, handle.Init("a", - params_, - kDefaultPriority, - callback.callback(), - pool_.get(), - BoundNetLog())); -} - -TEST_F(ClientSocketPoolBaseTest, - CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded) { - CreatePool(1, 1); - connect_job_factory_->set_job_type(TestConnectJob::kMockJob); - - MockLayeredPool mock_layered_pool1(pool_.get(), "foo"); - EXPECT_EQ(OK, mock_layered_pool1.RequestSocket(pool_.get())); - EXPECT_CALL(mock_layered_pool1, CloseOneIdleConnection()) - .WillRepeatedly(Invoke(&mock_layered_pool1, - &MockLayeredPool::ReleaseOneConnection)); - MockLayeredPool mock_layered_pool2(pool_.get(), "bar"); - EXPECT_EQ(OK, mock_layered_pool2.RequestSocketWithoutLimits(pool_.get())); - EXPECT_CALL(mock_layered_pool2, CloseOneIdleConnection()) - .WillRepeatedly(Invoke(&mock_layered_pool2, - &MockLayeredPool::ReleaseOneConnection)); - ClientSocketHandle handle; - TestCompletionCallback callback; - EXPECT_EQ(OK, handle.Init("a", - params_, - kDefaultPriority, - callback.callback(), - pool_.get(), - BoundNetLog())); -} - } // namespace } // namespace net diff --git a/net/socket/socks_client_socket_pool.cc b/net/socket/socks_client_socket_pool.cc index a7c7ecb..ae963bf 100644 --- a/net/socket/socks_client_socket_pool.cc +++ b/net/socket/socks_client_socket_pool.cc @@ -199,16 +199,9 @@ SOCKSClientSocketPool::SOCKSClientSocketPool( new SOCKSConnectJobFactory(transport_pool, host_resolver, net_log)) { - // We should always have a |transport_pool_| except in unit tests. - if (transport_pool_) - transport_pool_->AddLayeredPool(this); } -SOCKSClientSocketPool::~SOCKSClientSocketPool() { - // We should always have a |transport_pool_| except in unit tests. - if (transport_pool_) - transport_pool_->RemoveLayeredPool(this); -} +SOCKSClientSocketPool::~SOCKSClientSocketPool() {} int SOCKSClientSocketPool::RequestSocket( const std::string& group_name, const void* socket_params, @@ -246,10 +239,6 @@ void SOCKSClientSocketPool::Flush() { base_.Flush(); } -bool SOCKSClientSocketPool::IsStalled() const { - return base_.IsStalled() || transport_pool_->IsStalled(); -} - void SOCKSClientSocketPool::CloseIdleSockets() { base_.CloseIdleSockets(); } @@ -268,14 +257,6 @@ LoadState SOCKSClientSocketPool::GetLoadState( return base_.GetLoadState(group_name, handle); } -void SOCKSClientSocketPool::AddLayeredPool(LayeredPool* layered_pool) { - base_.AddLayeredPool(layered_pool); -} - -void SOCKSClientSocketPool::RemoveLayeredPool(LayeredPool* layered_pool) { - base_.RemoveLayeredPool(layered_pool); -} - DictionaryValue* SOCKSClientSocketPool::GetInfoAsValue( const std::string& name, const std::string& type, @@ -299,10 +280,4 @@ ClientSocketPoolHistograms* SOCKSClientSocketPool::histograms() const { return base_.histograms(); }; -bool SOCKSClientSocketPool::CloseOneIdleConnection() { - if (base_.CloseOneIdleSocket()) - return true; - return base_.CloseOneIdleConnectionInLayeredPool(); -} - } // namespace net diff --git a/net/socket/socks_client_socket_pool.h b/net/socket/socks_client_socket_pool.h index 8583d96..3fc7df6 100644 --- a/net/socket/socks_client_socket_pool.h +++ b/net/socket/socks_client_socket_pool.h @@ -105,8 +105,7 @@ class SOCKSConnectJob : public ConnectJob { DISALLOW_COPY_AND_ASSIGN(SOCKSConnectJob); }; -class NET_EXPORT_PRIVATE SOCKSClientSocketPool - : public ClientSocketPool, public LayeredPool { +class NET_EXPORT_PRIVATE SOCKSClientSocketPool : public ClientSocketPool { public: SOCKSClientSocketPool( int max_sockets, @@ -140,8 +139,6 @@ class NET_EXPORT_PRIVATE SOCKSClientSocketPool virtual void Flush() OVERRIDE; - virtual bool IsStalled() const OVERRIDE; - virtual void CloseIdleSockets() OVERRIDE; virtual int IdleSocketCount() const OVERRIDE; @@ -153,10 +150,6 @@ class NET_EXPORT_PRIVATE SOCKSClientSocketPool const std::string& group_name, const ClientSocketHandle* handle) const OVERRIDE; - virtual void AddLayeredPool(LayeredPool* layered_pool) OVERRIDE; - - virtual void RemoveLayeredPool(LayeredPool* layered_pool) OVERRIDE; - virtual base::DictionaryValue* GetInfoAsValue( const std::string& name, const std::string& type, @@ -166,9 +159,6 @@ class NET_EXPORT_PRIVATE SOCKSClientSocketPool virtual ClientSocketPoolHistograms* histograms() const OVERRIDE; - // LayeredPool implementation. - virtual bool CloseOneIdleConnection() OVERRIDE; - private: typedef ClientSocketPoolBase<SOCKSSocketParams> PoolBase; diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc index 9e68a3b..71a5b0d 100644 --- a/net/socket/ssl_client_socket_pool.cc +++ b/net/socket/ssl_client_socket_pool.cc @@ -479,21 +479,9 @@ SSLClientSocketPool::SSLClientSocketPool( ssl_config_service_(ssl_config_service) { if (ssl_config_service_) ssl_config_service_->AddObserver(this); - if (transport_pool_) - transport_pool_->AddLayeredPool(this); - if (socks_pool_) - socks_pool_->AddLayeredPool(this); - if (http_proxy_pool_) - http_proxy_pool_->AddLayeredPool(this); } SSLClientSocketPool::~SSLClientSocketPool() { - if (http_proxy_pool_) - http_proxy_pool_->RemoveLayeredPool(this); - if (socks_pool_) - socks_pool_->RemoveLayeredPool(this); - if (transport_pool_) - transport_pool_->RemoveLayeredPool(this); if (ssl_config_service_) ssl_config_service_->RemoveObserver(this); } @@ -546,13 +534,6 @@ void SSLClientSocketPool::Flush() { base_.Flush(); } -bool SSLClientSocketPool::IsStalled() const { - return base_.IsStalled() || - (transport_pool_ && transport_pool_->IsStalled()) || - (socks_pool_ && socks_pool_->IsStalled()) || - (http_proxy_pool_ && http_proxy_pool_->IsStalled()); -} - void SSLClientSocketPool::CloseIdleSockets() { base_.CloseIdleSockets(); } @@ -571,14 +552,6 @@ LoadState SSLClientSocketPool::GetLoadState( return base_.GetLoadState(group_name, handle); } -void SSLClientSocketPool::AddLayeredPool(LayeredPool* layered_pool) { - base_.AddLayeredPool(layered_pool); -} - -void SSLClientSocketPool::RemoveLayeredPool(LayeredPool* layered_pool) { - base_.RemoveLayeredPool(layered_pool); -} - DictionaryValue* SSLClientSocketPool::GetInfoAsValue( const std::string& name, const std::string& type, @@ -618,10 +591,4 @@ void SSLClientSocketPool::OnSSLConfigChanged() { Flush(); } -bool SSLClientSocketPool::CloseOneIdleConnection() { - if (base_.CloseOneIdleSocket()) - return true; - return base_.CloseOneIdleConnectionInLayeredPool(); -} - } // namespace net diff --git a/net/socket/ssl_client_socket_pool.h b/net/socket/ssl_client_socket_pool.h index 26e5f56..d80ace9 100644 --- a/net/socket/ssl_client_socket_pool.h +++ b/net/socket/ssl_client_socket_pool.h @@ -166,7 +166,6 @@ class SSLConnectJob : public ConnectJob { class NET_EXPORT_PRIVATE SSLClientSocketPool : public ClientSocketPool, - public LayeredPool, public SSLConfigService::Observer { public: // Only the pools that will be used are required. i.e. if you never @@ -212,8 +211,6 @@ class NET_EXPORT_PRIVATE SSLClientSocketPool virtual void Flush() OVERRIDE; - virtual bool IsStalled() const OVERRIDE; - virtual void CloseIdleSockets() OVERRIDE; virtual int IdleSocketCount() const OVERRIDE; @@ -225,10 +222,6 @@ class NET_EXPORT_PRIVATE SSLClientSocketPool const std::string& group_name, const ClientSocketHandle* handle) const OVERRIDE; - virtual void AddLayeredPool(LayeredPool* layered_pool) OVERRIDE; - - virtual void RemoveLayeredPool(LayeredPool* layered_pool) OVERRIDE; - virtual base::DictionaryValue* GetInfoAsValue( const std::string& name, const std::string& type, @@ -238,9 +231,6 @@ class NET_EXPORT_PRIVATE SSLClientSocketPool virtual ClientSocketPoolHistograms* histograms() const OVERRIDE; - // LayeredPool implementation. - virtual bool CloseOneIdleConnection() OVERRIDE; - private: typedef ClientSocketPoolBase<SSLSocketParams> PoolBase; diff --git a/net/socket/transport_client_socket_pool.cc b/net/socket/transport_client_socket_pool.cc index a00e810..d11099f 100644 --- a/net/socket/transport_client_socket_pool.cc +++ b/net/socket/transport_client_socket_pool.cc @@ -449,10 +449,6 @@ void TransportClientSocketPool::Flush() { base_.Flush(); } -bool TransportClientSocketPool::IsStalled() const { - return base_.IsStalled(); -} - void TransportClientSocketPool::CloseIdleSockets() { base_.CloseIdleSockets(); } @@ -471,14 +467,6 @@ LoadState TransportClientSocketPool::GetLoadState( return base_.GetLoadState(group_name, handle); } -void TransportClientSocketPool::AddLayeredPool(LayeredPool* layered_pool) { - base_.AddLayeredPool(layered_pool); -} - -void TransportClientSocketPool::RemoveLayeredPool(LayeredPool* layered_pool) { - base_.RemoveLayeredPool(layered_pool); -} - DictionaryValue* TransportClientSocketPool::GetInfoAsValue( const std::string& name, const std::string& type, diff --git a/net/socket/transport_client_socket_pool.h b/net/socket/transport_client_socket_pool.h index ef3f6fa..a4b4143 100644 --- a/net/socket/transport_client_socket_pool.h +++ b/net/socket/transport_client_socket_pool.h @@ -157,7 +157,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool : public ClientSocketPool { StreamSocket* socket, int id) OVERRIDE; virtual void Flush() OVERRIDE; - virtual bool IsStalled() const OVERRIDE; virtual void CloseIdleSockets() OVERRIDE; virtual int IdleSocketCount() const OVERRIDE; virtual int IdleSocketCountInGroup( @@ -165,8 +164,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool : public ClientSocketPool { virtual LoadState GetLoadState( const std::string& group_name, const ClientSocketHandle* handle) const OVERRIDE; - virtual void AddLayeredPool(LayeredPool* layered_pool) OVERRIDE; - virtual void RemoveLayeredPool(LayeredPool* layered_pool) OVERRIDE; virtual base::DictionaryValue* GetInfoAsValue( const std::string& name, const std::string& type, |