diff options
author | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-04 05:28:40 +0000 |
---|---|---|
committer | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-04 05:28:40 +0000 |
commit | d207a5f9ff5af30e4d1dabdeb61c2164a926fd2b (patch) | |
tree | 0d4619b4a982a79f4af8e29ccfcc623e51d9aca5 /net | |
parent | 079fc0db5e3faeda06f20dcf36bbaad49381069c (diff) | |
download | chromium_src-d207a5f9ff5af30e4d1dabdeb61c2164a926fd2b.zip chromium_src-d207a5f9ff5af30e4d1dabdeb61c2164a926fd2b.tar.gz chromium_src-d207a5f9ff5af30e4d1dabdeb61c2164a926fd2b.tar.bz2 |
Reland my ClientSocketPool refactor again...
The bug was that the handle was getting reused, so the ConnectingSocket doesn't know that it got canceled. It just keeps chugging away.
I added a map to keep track of the ConnectingSockets so they can be canceled if we detect a reuse.
TBR=wtc
Review URL: http://codereview.chromium.org/118219
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17606 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/base/client_socket_handle.cc | 51 | ||||
-rw-r--r-- | net/base/client_socket_handle.h | 54 | ||||
-rw-r--r-- | net/base/client_socket_pool.cc | 295 | ||||
-rw-r--r-- | net/base/client_socket_pool.h | 133 | ||||
-rw-r--r-- | net/base/client_socket_pool_unittest.cc | 364 | ||||
-rw-r--r-- | net/base/test_completion_callback.h | 2 | ||||
-rw-r--r-- | net/http/http_network_layer.cc | 3 | ||||
-rw-r--r-- | net/http/http_network_session.h | 7 | ||||
-rw-r--r-- | net/http/http_network_transaction.cc | 134 | ||||
-rw-r--r-- | net/http/http_network_transaction.h | 51 | ||||
-rw-r--r-- | net/http/http_network_transaction_unittest.cc | 177 |
11 files changed, 894 insertions, 377 deletions
diff --git a/net/base/client_socket_handle.cc b/net/base/client_socket_handle.cc index f5ab056..49de998 100644 --- a/net/base/client_socket_handle.cc +++ b/net/base/client_socket_handle.cc @@ -4,37 +4,70 @@ #include "net/base/client_socket_handle.h" +#include "base/compiler_specific.h" #include "net/base/client_socket.h" #include "net/base/client_socket_pool.h" +#include "net/base/net_errors.h" namespace net { ClientSocketHandle::ClientSocketHandle(ClientSocketPool* pool) - : pool_(pool), socket_(NULL) { -} + : pool_(pool), + socket_(NULL), + is_reused_(false), + ALLOW_THIS_IN_INITIALIZER_LIST( + callback_(this, &ClientSocketHandle::OnIOComplete)) {} ClientSocketHandle::~ClientSocketHandle() { Reset(); } int ClientSocketHandle::Init(const std::string& group_name, + const std::string& host, + int port, int priority, CompletionCallback* callback) { - Reset(); + ResetInternal(true); group_name_ = group_name; - return pool_->RequestSocket(this, priority, callback); + user_callback_ = callback; + return pool_->RequestSocket( + group_name, host, port, priority, this, &callback_); } void ClientSocketHandle::Reset() { + ResetInternal(true); +} + +void ClientSocketHandle::ResetInternal(bool cancel) { if (group_name_.empty()) // Was Init called? return; - if (socket_) { - pool_->ReleaseSocket(this); - socket_ = NULL; - } else { - pool_->CancelRequest(this); + if (socket_.get()) { + // If we've still got a socket, release it back to the ClientSocketPool so + // it can be deleted or reused. + pool_->ReleaseSocket(group_name_, release_socket()); + } else if (cancel) { + // If we did not get initialized yet, so we've got a socket request pending. + // Cancel it. + pool_->CancelRequest(group_name_, this); } group_name_.clear(); + is_reused_ = false; + user_callback_ = NULL; +} + +LoadState ClientSocketHandle::GetLoadState() const { + DCHECK(!is_initialized()); + DCHECK(!group_name_.empty()); + return pool_->GetLoadState(group_name_, this); +} + +void ClientSocketHandle::OnIOComplete(int result) { + DCHECK_NE(ERR_IO_PENDING, result); + CompletionCallback* callback = user_callback_; + user_callback_ = NULL; + if (result != OK) + ResetInternal(false); // The request failed, so there's nothing to cancel. + callback->Run(result); } } // namespace net diff --git a/net/base/client_socket_handle.h b/net/base/client_socket_handle.h index 1328369..5bac2c8 100644 --- a/net/base/client_socket_handle.h +++ b/net/base/client_socket_handle.h @@ -11,40 +11,43 @@ #include "base/scoped_ptr.h" #include "net/base/client_socket.h" #include "net/base/completion_callback.h" +#include "net/base/load_states.h" namespace net { class ClientSocketPool; -// A container for a connected ClientSocket. +// A container for a ClientSocket. // // The handle's |group_name| uniquely identifies the origin and type of the // connection. It is used by the ClientSocketPool to group similar connected // client socket objects. // -// A handle is initialized with a null socket. It is the consumer's job to -// initialize a ClientSocket object and set it on the handle. -// class ClientSocketHandle { public: explicit ClientSocketHandle(ClientSocketPool* pool); ~ClientSocketHandle(); // Initializes a ClientSocketHandle object, which involves talking to the - // ClientSocketPool to locate a socket to possibly reuse. This method - // returns either OK or ERR_IO_PENDING. On ERR_IO_PENDING, |priority| is - // used to determine the placement in ClientSocketPool's wait list. + // ClientSocketPool to obtain a connected socket, possibly reusing one. This + // method returns either OK or ERR_IO_PENDING. On ERR_IO_PENDING, |priority| + // is used to determine the placement in ClientSocketPool's wait list. // // If this method succeeds, then the socket member will be set to an existing - // socket if an existing socket was available to reuse. Otherwise, the - // consumer should set the socket member of this handle. + // connected socket if an existing connected socket was available to reuse, + // otherwise it will be set to a new connected socket. Consumers can then + // call is_reused() to see if the socket was reused. If not reusing an + // existing socket, ClientSocketPool may need to establish a new + // connection to the |host| |port| pair. // // This method returns ERR_IO_PENDING if it cannot complete synchronously, in - // which case the consumer should wait for the completion callback to run. + // which case the consumer will be notified of completion via |callback|. // // Init may be called multiple times. // int Init(const std::string& group_name, + const std::string& host, + int port, int priority, CompletionCallback* callback); @@ -54,25 +57,40 @@ class ClientSocketHandle { // the socket may be kept alive for use by a subsequent ClientSocketHandle. // // NOTE: To prevent the socket from being kept alive, be sure to call its - // Disconnect method. - // + // Disconnect method. This will result in the ClientSocketPool deleting the + // ClientSocket. void Reset(); - // Returns true when Init has completed successfully. + // Used after Init() is called, but before the ClientSocketPool has + // initialized the ClientSocketHandle. + LoadState GetLoadState() const; + + // Returns true when Init() has completed successfully. bool is_initialized() const { return socket_ != NULL; } + // Used by ClientSocketPool to initialize the ClientSocketHandle. + void set_is_reused(bool is_reused) { is_reused_ = is_reused; } + void set_socket(ClientSocket* s) { socket_.reset(s); } + // These may only be used if is_initialized() is true. const std::string& group_name() const { return group_name_; } - ClientSocket* socket() { return socket_->get(); } - ClientSocket* release_socket() { return socket_->release(); } - void set_socket(ClientSocket* s) { socket_->reset(s); } + ClientSocket* socket() { return socket_.get(); } + ClientSocket* release_socket() { return socket_.release(); } + bool is_reused() const { return is_reused_; } private: - friend class ClientSocketPool; + void OnIOComplete(int result); + + // Resets the state of the ClientSocketHandle. |cancel| indicates whether or + // not to try to cancel the request with the ClientSocketPool. + void ResetInternal(bool cancel); scoped_refptr<ClientSocketPool> pool_; - scoped_ptr<ClientSocket>* socket_; + scoped_ptr<ClientSocket> socket_; std::string group_name_; + bool is_reused_; + CompletionCallbackImpl<ClientSocketHandle> callback_; + CompletionCallback* user_callback_; DISALLOW_COPY_AND_ASSIGN(ClientSocketHandle); }; diff --git a/net/base/client_socket_pool.cc b/net/base/client_socket_pool.cc index 4171b0c..c0f281b 100644 --- a/net/base/client_socket_pool.cc +++ b/net/base/client_socket_pool.cc @@ -4,10 +4,16 @@ #include "net/base/client_socket_pool.h" +#include "base/compiler_specific.h" +#include "base/field_trial.h" #include "base/message_loop.h" -#include "net/base/client_socket.h" +#include "base/time.h" +#include "base/stl_util-inl.h" +#include "net/base/client_socket_factory.h" #include "net/base/client_socket_handle.h" +#include "net/base/dns_resolution_observer.h" #include "net/base/net_errors.h" +#include "net/base/tcp_client_socket.h" using base::TimeDelta; @@ -28,8 +34,137 @@ const int kIdleTimeout = 300; // 5 minutes. namespace net { -ClientSocketPool::ClientSocketPool(int max_sockets_per_group) - : idle_socket_count_(0), +ClientSocketPool::ConnectingSocket::ConnectingSocket( + const std::string& group_name, + const ClientSocketHandle* handle, + ClientSocketFactory* client_socket_factory, + ClientSocketPool* pool) + : group_name_(group_name), + handle_(handle), + client_socket_factory_(client_socket_factory), + ALLOW_THIS_IN_INITIALIZER_LIST( + callback_(this, + &ClientSocketPool::ConnectingSocket::OnIOComplete)), + pool_(pool), + canceled_(false) { + DCHECK(!ContainsKey(pool_->connecting_socket_map_, handle)); + pool_->connecting_socket_map_[handle] = this; +} + +ClientSocketPool::ConnectingSocket::~ConnectingSocket() { + if (!canceled_) + pool_->connecting_socket_map_.erase(handle_); +} + +int ClientSocketPool::ConnectingSocket::Connect( + const std::string& host, + int port, + CompletionCallback* callback) { + DCHECK(!canceled_); + DidStartDnsResolution(host, this); + int rv = resolver_.Resolve(host, port, &addresses_, &callback_); + if (rv == OK) { + // TODO(willchan): This code is broken. It should be fixed, but the code + // path is impossible in the current implementation since the host resolver + // always dumps the request to a worker pool, so it cannot complete + // synchronously. + NOTREACHED(); + connect_start_time_ = base::Time::Now(); + rv = socket_->Connect(&callback_); + } + return rv; +} + +ClientSocket* ClientSocketPool::ConnectingSocket::ReleaseSocket() { + return socket_.release(); +} + +void ClientSocketPool::ConnectingSocket::OnIOComplete(int result) { + DCHECK_NE(result, ERR_IO_PENDING); + + if (canceled_) { + // We got canceled, so bail out. + delete this; + return; + } + + 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; + } + + 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; + } + + if (result == OK) { + if (it->second.load_state == LOAD_STATE_RESOLVING_HOST) { + it->second.load_state = LOAD_STATE_CONNECTING; + socket_.reset(client_socket_factory_->CreateTCPClientSocket(addresses_)); + connect_start_time_ = base::Time::Now(); + result = socket_->Connect(&callback_); + if (result == ERR_IO_PENDING) + return; + } else { + DCHECK(connect_start_time_ != base::Time()); + base::TimeDelta connect_duration = + base::Time::Now() - connect_start_time_; + + UMA_HISTOGRAM_CLIPPED_TIMES( + FieldTrial::MakeName( + "Net.TCP_Connection_Latency", "DnsImpact").data(), + connect_duration, + base::TimeDelta::FromMilliseconds(1), + base::TimeDelta::FromMinutes(10), + 100); + } + } + + // Now, we either succeeded at Connect()'ing, or we failed at host resolution + // or Connect()'ing. Either way, we'll run the callback to alert the client. + + Request request = it->second; + request_map->erase(it); + + if (result == OK) { + request.handle->set_socket(socket_.release()); + request.handle->set_is_reused(false); + } else { + group.active_socket_count--; + + // Delete group if no longer needed. + if (group.active_socket_count == 0 && group.idle_sockets.empty()) { + DCHECK(group.pending_requests.empty()); + DCHECK(group.connecting_requests.empty()); + pool_->group_map_.erase(group_it); + } + } + + request.callback->Run(result); + delete this; +} + +void ClientSocketPool::ConnectingSocket::Cancel() { + DCHECK(!canceled_); + DCHECK(ContainsKey(pool_->connecting_socket_map_, handle_)); + pool_->connecting_socket_map_.erase(handle_); + canceled_ = true; +} + +ClientSocketPool::ClientSocketPool(int max_sockets_per_group, + ClientSocketFactory* client_socket_factory) + : client_socket_factory_(client_socket_factory), + idle_socket_count_(0), max_sockets_per_group_(max_sockets_per_group) { } @@ -54,10 +189,15 @@ void ClientSocketPool::InsertRequestIntoQueue(const Request& r, pending_requests->insert(it, r); } -int ClientSocketPool::RequestSocket(ClientSocketHandle* handle, +int ClientSocketPool::RequestSocket(const std::string& group_name, + const std::string& host, + int port, int priority, + ClientSocketHandle* handle, CompletionCallback* callback) { - Group& group = group_map_[handle->group_name_]; + DCHECK(!host.empty()); + DCHECK_GE(priority, 0); + Group& group = group_map_[group_name]; // Can we make another active socket now? if (group.active_socket_count == max_sockets_per_group_) { @@ -66,6 +206,9 @@ int ClientSocketPool::RequestSocket(ClientSocketHandle* handle, DCHECK(callback); r.callback = callback; r.priority = priority; + r.host = host; + r.port = port; + r.load_state = LOAD_STATE_IDLE; InsertRequestIntoQueue(r, &group.pending_requests); return ERR_IO_PENDING; } @@ -73,49 +216,99 @@ int ClientSocketPool::RequestSocket(ClientSocketHandle* handle, // OK, we are going to activate one. group.active_socket_count++; - // Use idle sockets in LIFO order because they're more likely to be - // still reusable. while (!group.idle_sockets.empty()) { IdleSocket idle_socket = group.idle_sockets.back(); group.idle_sockets.pop_back(); DecrementIdleCount(); - if ((*idle_socket.ptr)->IsConnectedAndIdle()) { + if (idle_socket.socket->IsConnectedAndIdle()) { // We found one we can reuse! - handle->socket_ = idle_socket.ptr; + handle->set_socket(idle_socket.socket); + handle->set_is_reused(true); return OK; } - delete idle_socket.ptr; + delete idle_socket.socket; + } + + // 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(); + + scoped_ptr<ConnectingSocket> connecting_socket( + new ConnectingSocket(group_name, handle, client_socket_factory_, this)); + int rv = connecting_socket->Connect(host, port, callback); + if (rv == OK) { + NOTREACHED(); + handle->set_socket(connecting_socket->ReleaseSocket()); + handle->set_is_reused(false); + } else if (rv == ERR_IO_PENDING) { + // The ConnectingSocket will delete itself. + connecting_socket.release(); + Request r; + r.handle = handle; + DCHECK(callback); + r.callback = callback; + r.priority = priority; + r.host = host; + r.port = port; + r.load_state = LOAD_STATE_RESOLVING_HOST; + group_map_[group_name].connecting_requests[handle] = r; + } else { + group.active_socket_count--; + + // Delete group if no longer needed. + if (group.active_socket_count == 0 && group.idle_sockets.empty()) { + DCHECK(group.pending_requests.empty()); + DCHECK(group.connecting_requests.empty()); + group_map_.erase(group_name); + } } - handle->socket_ = new ClientSocketPtr(); - return OK; + return rv; } -void ClientSocketPool::CancelRequest(ClientSocketHandle* handle) { - Group& group = group_map_[handle->group_name_]; +void ClientSocketPool::CancelRequest(const std::string& group_name, + const ClientSocketHandle* handle) { + DCHECK(ContainsKey(group_map_, group_name)); - // In order for us to be canceling a pending request, we must have active - // sockets equaling the limit. NOTE: The correctness of the code doesn't - // require this assertion. - DCHECK(group.active_socket_count == max_sockets_per_group_); + Group& group = group_map_[group_name]; // Search pending_requests for matching handle. - std::deque<Request>::iterator it = group.pending_requests.begin(); + RequestQueue::iterator it = group.pending_requests.begin(); for (; it != group.pending_requests.end(); ++it) { if (it->handle == handle) { group.pending_requests.erase(it); - break; + return; + } + } + + // It's invalid to cancel a non-existent request. + DCHECK(ContainsKey(group.connecting_requests, handle)); + + RequestMap::iterator map_it = group.connecting_requests.find(handle); + if (map_it != group.connecting_requests.end()) { + group.connecting_requests.erase(map_it); + group.active_socket_count--; + + // Delete group if no longer needed. + if (group.active_socket_count == 0 && group.idle_sockets.empty()) { + DCHECK(group.pending_requests.empty()); + DCHECK(group.connecting_requests.empty()); + group_map_.erase(group_name); } } } -void ClientSocketPool::ReleaseSocket(ClientSocketHandle* handle) { +void ClientSocketPool::ReleaseSocket(const std::string& group_name, + ClientSocket* socket) { // Run this asynchronously to allow the caller to finish before we let // another to begin doing work. This also avoids nasty recursion issues. // NOTE: We cannot refer to the handle argument after this method returns. MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod( - this, &ClientSocketPool::DoReleaseSocket, handle->group_name_, - handle->socket_)); + this, &ClientSocketPool::DoReleaseSocket, group_name, socket)); } void ClientSocketPool::CloseIdleSockets() { @@ -130,10 +323,42 @@ int ClientSocketPool::IdleSocketCountInGroup( return i->second.idle_sockets.size(); } +LoadState ClientSocketPool::GetLoadState( + const std::string& group_name, + const ClientSocketHandle* handle) const { + DCHECK(ContainsKey(group_map_, group_name)) << group_name; + + // 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()) { + const LoadState load_state = map_it->second.load_state; + DCHECK(load_state == LOAD_STATE_RESOLVING_HOST || + load_state == LOAD_STATE_CONNECTING); + return load_state; + } + + // Search pending_requests for matching handle. + RequestQueue::const_iterator it = group.pending_requests.begin(); + for (; it != group.pending_requests.end(); ++it) { + if (it->handle == handle) { + DCHECK_EQ(LOAD_STATE_IDLE, it->load_state); + // TODO(wtc): Add a state for being on the wait list. + // See http://www.crbug.com/5077. + return LOAD_STATE_IDLE; + } + } + + NOTREACHED(); + return LOAD_STATE_IDLE; +} + bool ClientSocketPool::IdleSocket::ShouldCleanup(base::TimeTicks now) const { bool timed_out = (now - start_time) >= base::TimeDelta::FromSeconds(kIdleTimeout); - return timed_out || !(*ptr)->IsConnectedAndIdle(); + return timed_out || !socket->IsConnectedAndIdle(); } void ClientSocketPool::CleanupIdleSockets(bool force) { @@ -151,7 +376,7 @@ void ClientSocketPool::CleanupIdleSockets(bool force) { std::deque<IdleSocket>::iterator j = group.idle_sockets.begin(); while (j != group.idle_sockets.end()) { if (force || j->ShouldCleanup(now)) { - delete j->ptr; + delete j->socket; j = group.idle_sockets.erase(j); DecrementIdleCount(); } else { @@ -162,6 +387,7 @@ void ClientSocketPool::CleanupIdleSockets(bool force) { // Delete group if no longer needed. if (group.active_socket_count == 0 && group.idle_sockets.empty()) { DCHECK(group.pending_requests.empty()); + DCHECK(group.connecting_requests.empty()); group_map_.erase(i++); } else { ++i; @@ -181,40 +407,43 @@ void ClientSocketPool::DecrementIdleCount() { } void ClientSocketPool::DoReleaseSocket(const std::string& group_name, - ClientSocketPtr* ptr) { + ClientSocket* socket) { GroupMap::iterator i = group_map_.find(group_name); DCHECK(i != group_map_.end()); Group& group = i->second; - DCHECK(group.active_socket_count > 0); + DCHECK_GT(group.active_socket_count, 0); group.active_socket_count--; - bool can_reuse = ptr->get() && (*ptr)->IsConnectedAndIdle(); + const bool can_reuse = socket->IsConnectedAndIdle(); if (can_reuse) { IdleSocket idle_socket; - idle_socket.ptr = ptr; + idle_socket.socket = socket; idle_socket.start_time = base::TimeTicks::Now(); group.idle_sockets.push_back(idle_socket); IncrementIdleCount(); } else { - delete ptr; + delete socket; } // Process one pending request. if (!group.pending_requests.empty()) { Request r = group.pending_requests.front(); group.pending_requests.pop_front(); - int rv = RequestSocket(r.handle, r.priority, NULL); - DCHECK(rv == OK); - r.callback->Run(rv); + + int rv = RequestSocket( + group_name, r.host, r.port, r.priority, r.handle, r.callback); + if (rv != ERR_IO_PENDING) + r.callback->Run(rv); return; } // Delete group if no longer needed. if (group.active_socket_count == 0 && group.idle_sockets.empty()) { DCHECK(group.pending_requests.empty()); + DCHECK(group.connecting_requests.empty()); group_map_.erase(i); } } diff --git a/net/base/client_socket_pool.h b/net/base/client_socket_pool.h index 6c92f37..0ee3ffe 100644 --- a/net/base/client_socket_pool.h +++ b/net/base/client_socket_pool.h @@ -12,58 +12,66 @@ #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/timer.h" +#include "net/base/address_list.h" #include "net/base/completion_callback.h" +#include "net/base/host_resolver.h" +#include "net/base/load_states.h" namespace net { class ClientSocket; +class ClientSocketFactory; class ClientSocketHandle; // A ClientSocketPool is used to restrict the number of sockets open at a time. // It also maintains a list of idle persistent sockets. // -// The ClientSocketPool allocates scoped_ptr<ClientSocket> objects, but it is -// not responsible for allocating the associated ClientSocket objects. The -// consumer must do so if it gets a scoped_ptr<ClientSocket> with a null value. -// class ClientSocketPool : public base::RefCounted<ClientSocketPool> { public: - explicit ClientSocketPool(int max_sockets_per_group); - - // Called to request a socket for the given handle. There are three possible - // results: 1) the handle will be initialized with a socket to reuse, 2) the - // handle will be initialized without a socket such that the consumer needs - // to supply a socket, or 3) the handle will be added to a wait list until a - // socket is available to reuse or the opportunity to create a new socket - // arises. The completion callback is notified in the 3rd case. |priority| - // will determine the placement into the wait list. + ClientSocketPool(int max_sockets_per_group, + ClientSocketFactory* client_socket_factory); + + // Requests a connected socket for a group_name. + // + // There are four possible results from calling this function: + // 1) RequestSocket returns OK and initializes |handle| with a reused socket. + // 2) RequestSocket returns OK with a newly connected socket. + // 3) RequestSocket returns ERR_IO_PENDING. The handle will be added to a + // wait list until a socket is available to reuse or a new socket finishes + // connecting. |priority| will determine the placement into the wait list. + // 4) An error occurred early on, so RequestSocket returns an error code. // // If this function returns OK, then |handle| is initialized upon return. // The |handle|'s is_initialized method will return true in this case. If a - // ClientSocket was reused, then |handle|'s socket member will be non-NULL. - // Otherwise, the consumer will need to supply |handle| with a socket by - // allocating a new ClientSocket object and calling the |handle|'s set_socket - // method. + // ClientSocket was reused, then ClientSocketPool will call + // |handle|->set_reused(true). In either case, the socket will have been + // allocated and will be connected. A client might want to know whether or + // not the socket is reused in order to know whether or not he needs to + // perform SSL connection or tunnel setup or to request a new socket if he + // encounters an error with the reused socket. // - // If ERR_IO_PENDING is returned, then the completion callback will be called - // when |handle| has been initialized. + // If ERR_IO_PENDING is returned, then the callback will be used to notify the + // client of completion. // - int RequestSocket(ClientSocketHandle* handle, + int RequestSocket(const std::string& group_name, + const std::string& host, + int port, int priority, + ClientSocketHandle* handle, CompletionCallback* callback); // Called to cancel a RequestSocket call that returned ERR_IO_PENDING. The // same handle parameter must be passed to this method as was passed to the // RequestSocket call being cancelled. The associated CompletionCallback is // not run. - void CancelRequest(ClientSocketHandle* handle); + void CancelRequest(const std::string& group_name, + const ClientSocketHandle* handle); - // Called to release the socket member of an initialized ClientSocketHandle - // once the socket is no longer needed. If the socket member is non-null and - // still has an established connection, then it will be added to the idle set - // of sockets to be used to satisfy future RequestSocket calls. Otherwise, - // the ClientSocket is destroyed. - void ReleaseSocket(ClientSocketHandle* handle); + // Called to release a socket once the socket is no longer needed. If the + // socket still has an established connection, then it will be added to the + // set of idle sockets to be used to satisfy future RequestSocket calls. + // Otherwise, the ClientSocket is destroyed. + void ReleaseSocket(const std::string& group_name, ClientSocket* socket); // Called to close any idle connections held by the connection manager. void CloseIdleSockets(); @@ -76,22 +84,27 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> { // The total number of idle sockets in a connection group. int IdleSocketCountInGroup(const std::string& group_name) const; + // Determine the LoadState of a connecting ClientSocketHandle. + LoadState GetLoadState(const std::string& group_name, + const ClientSocketHandle* handle) const; + private: friend class base::RefCounted<ClientSocketPool>; - typedef scoped_ptr<ClientSocket> ClientSocketPtr; - // A Request is allocated per call to RequestSocket that results in // ERR_IO_PENDING. struct Request { ClientSocketHandle* handle; CompletionCallback* callback; int priority; + std::string host; + int port; + LoadState load_state; }; // Entry for a persistent socket which became idle at time |start_time|. struct IdleSocket { - ClientSocketPtr* ptr; + ClientSocket* socket; base::TimeTicks start_time; // An idle socket should be removed if it can't be reused, or has been idle @@ -105,6 +118,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> { }; typedef std::deque<Request> RequestQueue; + typedef std::map<const ClientSocketHandle*, Request> RequestMap; // A Group is allocated per group_name when there are idle sockets or pending // requests. Otherwise, the Group object is removed from the map. @@ -112,11 +126,62 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> { Group() : active_socket_count(0) {} std::deque<IdleSocket> idle_sockets; RequestQueue pending_requests; + RequestMap connecting_requests; int active_socket_count; }; typedef std::map<std::string, Group> GroupMap; + // ConnectingSocket handles the host resolution necessary for socket creation + // and the Connect(). + class ConnectingSocket { + public: + enum State { + STATE_RESOLVE_HOST, + STATE_CONNECT + }; + + ConnectingSocket(const std::string& group_name, + const ClientSocketHandle* handle, + ClientSocketFactory* client_socket_factory, + ClientSocketPool* pool); + ~ConnectingSocket(); + + // Begins the host resolution and the TCP connect. Returns OK on success, + // in which case |callback| is not called. On pending IO, Connect returns + // ERR_IO_PENDING and runs |callback| on completion. + int Connect(const std::string& host, + int port, + CompletionCallback* callback); + + // If Connect() returns OK, ClientSocketPool may invoke this method to get + // the ConnectingSocket to release |socket_| to be set into the + // ClientSocketHandle immediately. + ClientSocket* ReleaseSocket(); + + // Called by the ClientSocketPool to cancel this ConnectingSocket. Only + // necessary if a ClientSocketHandle is reused. + void Cancel(); + + private: + void OnIOComplete(int result); + + const std::string group_name_; + const ClientSocketHandle* const handle_; + ClientSocketFactory* const client_socket_factory_; + CompletionCallbackImpl<ConnectingSocket> callback_; + scoped_ptr<ClientSocket> socket_; + scoped_refptr<ClientSocketPool> pool_; + HostResolver resolver_; + AddressList addresses_; + bool canceled_; + + // The time the Connect() method was called (if it got called). + base::Time connect_start_time_; + + DISALLOW_COPY_AND_ASSIGN(ConnectingSocket); + }; + ~ClientSocketPool(); static void InsertRequestIntoQueue(const Request& r, @@ -131,7 +196,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> { void DecrementIdleCount(); // Called via PostTask by ReleaseSocket. - void DoReleaseSocket(const std::string& group_name, ClientSocketPtr* ptr); + void DoReleaseSocket(const std::string& group_name, ClientSocket* socket); // Called when timer_ fires. This method scans the idle sockets removing // sockets that timed out or can't be reused. @@ -139,8 +204,12 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> { CleanupIdleSockets(false); } + ClientSocketFactory* const client_socket_factory_; + GroupMap group_map_; + std::map<const ClientSocketHandle*, ConnectingSocket*> connecting_socket_map_; + // Timer used to periodically prune idle sockets that timed out or can't be // reused. base::RepeatingTimer<ClientSocketPool> timer_; @@ -149,7 +218,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> { int idle_socket_count_; // The maximum number of sockets kept per group. - int max_sockets_per_group_; + const int max_sockets_per_group_; DISALLOW_COPY_AND_ASSIGN(ClientSocketPool); }; diff --git a/net/base/client_socket_pool_unittest.cc b/net/base/client_socket_pool_unittest.cc index a889d9a..7272709 100644 --- a/net/base/client_socket_pool_unittest.cc +++ b/net/base/client_socket_pool_unittest.cc @@ -4,11 +4,16 @@ #include "base/message_loop.h" #include "net/base/client_socket.h" +#include "net/base/client_socket_factory.h" #include "net/base/client_socket_handle.h" #include "net/base/client_socket_pool.h" +#include "net/base/host_resolver_unittest.h" #include "net/base/net_errors.h" +#include "net/base/test_completion_callback.h" #include "testing/gtest/include/gtest/gtest.h" +namespace net { + namespace { const int kMaxSocketsPerGroup = 6; @@ -21,16 +26,16 @@ const int kPriorities[10] = { 1, 7, 9, 5, 6, 2, 8, 3, 4, 1 }; // available sockets in the socket group. const int kNumPendingRequests = arraysize(kPriorities); -class MockClientSocket : public net::ClientSocket { +const int kNumRequests = kMaxSocketsPerGroup + kNumPendingRequests; + +class MockClientSocket : public ClientSocket { public: - MockClientSocket() : connected_(false) { - allocation_count++; - } + MockClientSocket() : connected_(false) {} // ClientSocket methods: - virtual int Connect(net::CompletionCallback* callback) { + virtual int Connect(CompletionCallback* callback) { connected_ = true; - return net::OK; + return OK; } virtual void Disconnect() { connected_ = false; @@ -43,51 +48,148 @@ class MockClientSocket : public net::ClientSocket { } // Socket methods: - virtual int Read(net::IOBuffer* buf, int buf_len, - net::CompletionCallback* callback) { - return net::ERR_FAILED; + virtual int Read(IOBuffer* buf, int buf_len, + CompletionCallback* callback) { + return ERR_FAILED; } - virtual int Write(net::IOBuffer* buf, int buf_len, - net::CompletionCallback* callback) { - return net::ERR_FAILED; + virtual int Write(IOBuffer* buf, int buf_len, + CompletionCallback* callback) { + return ERR_FAILED; } - static int allocation_count; - private: bool connected_; }; -int MockClientSocket::allocation_count = 0; +class MockFailingClientSocket : public ClientSocket { + public: + MockFailingClientSocket() {} + + // ClientSocket methods: + virtual int Connect(CompletionCallback* callback) { + return ERR_CONNECTION_FAILED; + } + + virtual void Disconnect() {} + + virtual bool IsConnected() const { + return false; + } + virtual bool IsConnectedAndIdle() const { + return false; + } + + // Socket methods: + virtual int Read(IOBuffer* buf, int buf_len, + CompletionCallback* callback) { + return ERR_FAILED; + } + + virtual int Write(IOBuffer* buf, int buf_len, + CompletionCallback* callback) { + return ERR_FAILED; + } +}; + +class MockPendingClientSocket : public ClientSocket { + public: + MockPendingClientSocket() {} + + // ClientSocket methods: + virtual int Connect(CompletionCallback* callback) { + return ERR_IO_PENDING; + } + + virtual void Disconnect() {} + + virtual bool IsConnected() const { + return false; + } + virtual bool IsConnectedAndIdle() const { + return false; + } + + // Socket methods: + virtual int Read(IOBuffer* buf, int buf_len, + CompletionCallback* callback) { + return ERR_FAILED; + } + + virtual int Write(IOBuffer* buf, int buf_len, + CompletionCallback* callback) { + return ERR_FAILED; + } +}; + +class MockClientSocketFactory : public ClientSocketFactory { + public: + enum ClientSocketType { + MOCK_CLIENT_SOCKET, + MOCK_FAILING_CLIENT_SOCKET, + MOCK_PENDING_CLIENT_SOCKET, + }; + + MockClientSocketFactory() + : allocation_count_(0), client_socket_type_(MOCK_CLIENT_SOCKET) {} + + virtual ClientSocket* CreateTCPClientSocket(const AddressList& addresses) { + allocation_count_++; + switch (client_socket_type_) { + case MOCK_CLIENT_SOCKET: + return new MockClientSocket(); + case MOCK_FAILING_CLIENT_SOCKET: + return new MockFailingClientSocket(); + case MOCK_PENDING_CLIENT_SOCKET: + return new MockPendingClientSocket(); + default: + NOTREACHED(); + return new MockClientSocket(); + } + } + + virtual SSLClientSocket* CreateSSLClientSocket( + ClientSocket* transport_socket, + const std::string& hostname, + const SSLConfig& ssl_config) { + NOTIMPLEMENTED(); + return NULL; + } + + int allocation_count() const { return allocation_count_; } + + void set_client_socket_type(ClientSocketType type) { + client_socket_type_ = type; + } + + private: + int allocation_count_; + ClientSocketType client_socket_type_; +}; class TestSocketRequest : public CallbackRunner< Tuple1<int> > { public: TestSocketRequest( - net::ClientSocketPool* pool, + ClientSocketPool* pool, std::vector<TestSocketRequest*>* request_order) : handle(pool), request_order_(request_order) {} - net::ClientSocketHandle handle; + ClientSocketHandle handle; - void EnsureSocket() { - DCHECK(handle.is_initialized()); - request_order_->push_back(this); - if (!handle.socket()) { - handle.set_socket(new MockClientSocket()); - handle.socket()->Connect(NULL); - } + int WaitForResult() { + return callback_.WaitForResult(); } virtual void RunWithParams(const Tuple1<int>& params) { - DCHECK(params.a == net::OK); + callback_.RunWithParams(params); completion_count++; - EnsureSocket(); + request_order_->push_back(this); } static int completion_count; private: std::vector<TestSocketRequest*>* request_order_; + TestCompletionCallback callback_; }; int TestSocketRequest::completion_count = 0; @@ -95,68 +197,75 @@ int TestSocketRequest::completion_count = 0; class ClientSocketPoolTest : public testing::Test { protected: ClientSocketPoolTest() - : pool_(new net::ClientSocketPool(kMaxSocketsPerGroup)) {} + : pool_(new ClientSocketPool(kMaxSocketsPerGroup, + &client_socket_factory_)) {} virtual void SetUp() { - MockClientSocket::allocation_count = 0; TestSocketRequest::completion_count = 0; } - scoped_refptr<net::ClientSocketPool> pool_; + MockClientSocketFactory client_socket_factory_; + scoped_refptr<ClientSocketPool> pool_; std::vector<TestSocketRequest*> request_order_; }; TEST_F(ClientSocketPoolTest, Basic) { - TestSocketRequest r(pool_.get(), &request_order_); - int rv; + TestCompletionCallback callback; + ClientSocketHandle handle(pool_.get()); + int rv = handle.Init("a", "www.google.com", 80, 0, &callback); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_FALSE(handle.is_initialized()); + EXPECT_FALSE(handle.socket()); - rv = r.handle.Init("a", 0, &r); - EXPECT_EQ(net::OK, rv); - EXPECT_TRUE(r.handle.is_initialized()); + EXPECT_EQ(OK, callback.WaitForResult()); + EXPECT_TRUE(handle.is_initialized()); + EXPECT_TRUE(handle.socket()); - r.handle.Reset(); + handle.Reset(); // The handle's Reset method may have posted a task. MessageLoop::current()->RunAllPending(); } -TEST_F(ClientSocketPoolTest, WithIdleConnection) { - TestSocketRequest r(pool_.get(), &request_order_); - int rv; - - rv = r.handle.Init("a", 0, &r); - EXPECT_EQ(net::OK, rv); - EXPECT_TRUE(r.handle.is_initialized()); - - // Create a socket. - r.EnsureSocket(); - - // Release the socket. It should find its way into the idle list. We're - // testing that this does not trigger a crash. - r.handle.Reset(); +TEST_F(ClientSocketPoolTest, InitHostResolutionFailure) { + RuleBasedHostMapper* host_mapper = new RuleBasedHostMapper; + host_mapper->AddSimulatedFailure("unresolvable.host.name"); + ScopedHostMapper scoped_host_mapper(host_mapper); + TestSocketRequest req(pool_.get(), &request_order_); + EXPECT_EQ(ERR_IO_PENDING, + req.handle.Init("a", "unresolvable.host.name", 80, 5, &req)); + EXPECT_EQ(ERR_NAME_NOT_RESOLVED, req.WaitForResult()); +} - // The handle's Reset method may have posted a task. - MessageLoop::current()->RunAllPending(); +TEST_F(ClientSocketPoolTest, InitConnectionFailure) { + client_socket_factory_.set_client_socket_type( + MockClientSocketFactory::MOCK_FAILING_CLIENT_SOCKET); + TestSocketRequest req(pool_.get(), &request_order_); + EXPECT_EQ(ERR_IO_PENDING, + req.handle.Init("a", "unresolvable.host.name", 80, 5, &req)); + EXPECT_EQ(ERR_CONNECTION_FAILED, req.WaitForResult()); } TEST_F(ClientSocketPoolTest, PendingRequests) { int rv; - scoped_ptr<TestSocketRequest> reqs[kMaxSocketsPerGroup + kNumPendingRequests]; + scoped_ptr<TestSocketRequest> reqs[kNumRequests]; for (size_t i = 0; i < arraysize(reqs); ++i) reqs[i].reset(new TestSocketRequest(pool_.get(), &request_order_)); // Create connections or queue up requests. for (int i = 0; i < kMaxSocketsPerGroup; ++i) { - rv = reqs[i]->handle.Init("a", 5, reqs[i].get()); - EXPECT_EQ(net::OK, rv); - reqs[i]->EnsureSocket(); + rv = reqs[i]->handle.Init("a", "www.google.com", 80, 5, reqs[i].get()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, reqs[i]->WaitForResult()); } + for (int i = 0; i < kNumPendingRequests; ++i) { rv = reqs[kMaxSocketsPerGroup + i]->handle.Init( - "a", kPriorities[i], reqs[kMaxSocketsPerGroup + i].get()); - EXPECT_EQ(net::ERR_IO_PENDING, rv); + "a", "www.google.com", 80, kPriorities[i], + reqs[kMaxSocketsPerGroup + i].get()); + EXPECT_EQ(ERR_IO_PENDING, rv); } // Release any connections until we have no connections. @@ -172,8 +281,8 @@ TEST_F(ClientSocketPoolTest, PendingRequests) { } } while (released_one); - EXPECT_EQ(kMaxSocketsPerGroup, MockClientSocket::allocation_count); - EXPECT_EQ(kNumPendingRequests, TestSocketRequest::completion_count); + EXPECT_EQ(kMaxSocketsPerGroup, client_socket_factory_.allocation_count()); + EXPECT_EQ(kNumRequests, TestSocketRequest::completion_count); for (int i = 0; i < kMaxSocketsPerGroup; ++i) { EXPECT_EQ(request_order_[i], reqs[i].get()) << @@ -194,58 +303,135 @@ TEST_F(ClientSocketPoolTest, PendingRequests) { } TEST_F(ClientSocketPoolTest, PendingRequests_NoKeepAlive) { - int rv; - - scoped_ptr<TestSocketRequest> reqs[kMaxSocketsPerGroup + kNumPendingRequests]; + scoped_ptr<TestSocketRequest> reqs[kNumRequests]; for (size_t i = 0; i < arraysize(reqs); ++i) reqs[i].reset(new TestSocketRequest(pool_.get(), &request_order_)); // Create connections or queue up requests. - for (size_t i = 0; i < arraysize(reqs); ++i) { - rv = reqs[i]->handle.Init("a", 0, reqs[i].get()); - if (rv != net::ERR_IO_PENDING) { - EXPECT_EQ(net::OK, rv); - reqs[i]->EnsureSocket(); - } + for (int i = 0; i < kMaxSocketsPerGroup; ++i) { + int rv = reqs[i]->handle.Init("a", "www.google.com", 80, 0, reqs[i].get()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, reqs[i]->WaitForResult()); + } + + for (int i = 0; i < kNumPendingRequests; ++i) { + EXPECT_EQ(ERR_IO_PENDING, reqs[kMaxSocketsPerGroup + i]->handle.Init( + "a", "www.google.com", 80, 0, reqs[kMaxSocketsPerGroup + i].get())); } // Release any connections until we have no connections. - bool released_one; - do { - released_one = false; + + while (TestSocketRequest::completion_count < kNumRequests) { + int num_released = 0; for (size_t i = 0; i < arraysize(reqs); ++i) { if (reqs[i]->handle.is_initialized()) { reqs[i]->handle.socket()->Disconnect(); reqs[i]->handle.Reset(); - MessageLoop::current()->RunAllPending(); - released_one = true; + num_released++; } } - } while (released_one); + int curr_num_completed = TestSocketRequest::completion_count; + for (int i = 0; + (i < num_released) && (i + curr_num_completed < kNumRequests); ++i) { + EXPECT_EQ(OK, reqs[i + curr_num_completed]->WaitForResult()); + } + } - EXPECT_EQ(kMaxSocketsPerGroup + kNumPendingRequests, - MockClientSocket::allocation_count); - EXPECT_EQ(kNumPendingRequests, TestSocketRequest::completion_count); + EXPECT_EQ(kNumRequests, client_socket_factory_.allocation_count()); + EXPECT_EQ(kNumRequests, TestSocketRequest::completion_count); } -TEST_F(ClientSocketPoolTest, CancelRequest) { - int rv; +// This test will start up a RequestSocket() and then immediately Cancel() it. +// The pending host resolution will eventually complete, and destroy the +// ClientSocketPool which will crash if the group was not cleared properly. +TEST_F(ClientSocketPoolTest, CancelRequestClearGroup) { + TestSocketRequest req(pool_.get(), &request_order_); + EXPECT_EQ(ERR_IO_PENDING, + req.handle.Init("a", "www.google.com", 80, 5, &req)); + req.handle.Reset(); + + PlatformThread::Sleep(100); + + // There is a race condition here. If the worker pool doesn't post the task + // before we get here, then this might not run ConnectingSocket::OnIOComplete + // and therefore leak the canceled ConnectingSocket. However, other tests + // after this will call MessageLoop::RunAllPending() which should prevent a + // leak, unless the worker thread takes longer than all of them. + MessageLoop::current()->RunAllPending(); +} - scoped_ptr<TestSocketRequest> reqs[kMaxSocketsPerGroup + kNumPendingRequests]; +TEST_F(ClientSocketPoolTest, TwoRequestsCancelOne) { + TestSocketRequest req(pool_.get(), &request_order_); + TestSocketRequest req2(pool_.get(), &request_order_); + + EXPECT_EQ(ERR_IO_PENDING, + req.handle.Init("a", "www.google.com", 80, 5, &req)); + EXPECT_EQ(ERR_IO_PENDING, + req2.handle.Init("a", "www.google.com", 80, 5, &req)); + + req.handle.Reset(); + PlatformThread::Sleep(100); + + // There is a benign race condition here. The worker pool may or may not post + // the tasks before we get here. It won't test the case properly if it + // doesn't, but 100ms should be enough most of the time. + MessageLoop::current()->RunAllPending(); + + req2.handle.Reset(); + // The handle's Reset method may have posted a task. + MessageLoop::current()->RunAllPending(); +} + +TEST_F(ClientSocketPoolTest, ConnectCancelConnect) { + client_socket_factory_.set_client_socket_type( + MockClientSocketFactory::MOCK_PENDING_CLIENT_SOCKET); + TestSocketRequest req(pool_.get(), &request_order_); + + EXPECT_EQ(ERR_IO_PENDING, + req.handle.Init("a", "www.google.com", 80, 5, &req)); + + req.handle.Reset(); + + EXPECT_EQ(ERR_IO_PENDING, + req.handle.Init("a", "www.google.com", 80, 5, &req)); + + // There is a benign race condition here. The worker pool may or may not post + // the tasks before we get here. It won't test the case properly if it + // doesn't, but 100ms should be enough most of the time. + + // Let the first ConnectingSocket for the handle run. This should have been + // canceled, so it shouldn't update the state of any Request. + PlatformThread::Sleep(100); + MessageLoop::current()->RunAllPending(); + + // Let the second ConnectingSocket for the handle run. If the first + // ConnectingSocket updated the state of any request, this will crash. + PlatformThread::Sleep(100); + MessageLoop::current()->RunAllPending(); + + req.handle.Reset(); + // The handle's Reset method may have posted a task. + MessageLoop::current()->RunAllPending(); +} + +TEST_F(ClientSocketPoolTest, CancelRequest) { + scoped_ptr<TestSocketRequest> reqs[kNumRequests]; for (size_t i = 0; i < arraysize(reqs); ++i) reqs[i].reset(new TestSocketRequest(pool_.get(), &request_order_)); // Create connections or queue up requests. for (int i = 0; i < kMaxSocketsPerGroup; ++i) { - rv = reqs[i]->handle.Init("a", 5, reqs[i].get()); - EXPECT_EQ(net::OK, rv); - reqs[i]->EnsureSocket(); + int rv = reqs[i]->handle.Init("a", "www.google.com", 80, 5, reqs[i].get()); + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, reqs[i]->WaitForResult()); } + for (int i = 0; i < kNumPendingRequests; ++i) { - rv = reqs[kMaxSocketsPerGroup + i]->handle.Init( - "a", kPriorities[i], reqs[kMaxSocketsPerGroup + i].get()); - EXPECT_EQ(net::ERR_IO_PENDING, rv); + int rv = reqs[kMaxSocketsPerGroup + i]->handle.Init( + "a", "www.google.com", 80, kPriorities[i], + reqs[kMaxSocketsPerGroup + i].get()); + EXPECT_EQ(ERR_IO_PENDING, rv); } // Cancel a request. @@ -266,8 +452,8 @@ TEST_F(ClientSocketPoolTest, CancelRequest) { } } while (released_one); - EXPECT_EQ(kMaxSocketsPerGroup, MockClientSocket::allocation_count); - EXPECT_EQ(kNumPendingRequests - 1, TestSocketRequest::completion_count); + EXPECT_EQ(kMaxSocketsPerGroup, client_socket_factory_.allocation_count()); + EXPECT_EQ(kNumRequests - 1, TestSocketRequest::completion_count); for (int i = 0; i < kMaxSocketsPerGroup; ++i) { EXPECT_EQ(request_order_[i], reqs[i].get()) << "Request " << i << " was not in order."; @@ -290,3 +476,5 @@ TEST_F(ClientSocketPoolTest, CancelRequest) { } } // namespace + +} // namespace net diff --git a/net/base/test_completion_callback.h b/net/base/test_completion_callback.h index 65fec62..82680aa 100644 --- a/net/base/test_completion_callback.h +++ b/net/base/test_completion_callback.h @@ -40,7 +40,6 @@ class TestCompletionCallback : public CallbackRunner< Tuple1<int> > { bool have_result() const { return have_result_; } - private: virtual void RunWithParams(const Tuple1<int>& params) { result_ = params.a; have_result_ = true; @@ -48,6 +47,7 @@ class TestCompletionCallback : public CallbackRunner< Tuple1<int> > { MessageLoop::current()->Quit(); } + private: int result_; bool have_result_; bool waiting_for_result_; diff --git a/net/http/http_network_layer.cc b/net/http/http_network_layer.cc index 6c905e7..b46012f 100644 --- a/net/http/http_network_layer.cc +++ b/net/http/http_network_layer.cc @@ -66,7 +66,8 @@ void HttpNetworkLayer::Suspend(bool suspend) { HttpNetworkSession* HttpNetworkLayer::GetSession() { if (!session_) { DCHECK(proxy_service_); - session_ = new HttpNetworkSession(proxy_service_); + session_ = new HttpNetworkSession(proxy_service_, + ClientSocketFactory::GetDefaultFactory()); } return session_; } diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h index 7c4ac68..d96255a 100644 --- a/net/http/http_network_session.h +++ b/net/http/http_network_session.h @@ -12,13 +12,16 @@ namespace net { +class ClientSocketFactory; class ProxyService; // This class holds session objects used by HttpNetworkTransaction objects. class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> { public: - explicit HttpNetworkSession(ProxyService* proxy_service) - : connection_pool_(new ClientSocketPool(max_sockets_per_group_)), + HttpNetworkSession(ProxyService* proxy_service, + ClientSocketFactory* client_socket_factory) + : connection_pool_(new ClientSocketPool( + max_sockets_per_group_, client_socket_factory)), proxy_service_(proxy_service) { DCHECK(proxy_service); } diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 63eacc1..38ed249 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -177,7 +177,7 @@ int HttpNetworkTransaction::RestartIgnoringLastError( if (connection_.socket()->IsConnected()) { next_state_ = STATE_WRITE_HEADERS; } else { - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); connection_.Reset(); next_state_ = STATE_INIT_CONNECTION; } @@ -301,7 +301,7 @@ void HttpNetworkTransaction::DidDrainBodyForAuthRestart(bool keep_alive) { reused_socket_ = true; } else { next_state_ = STATE_INIT_CONNECTION; - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); connection_.Reset(); } @@ -351,10 +351,8 @@ LoadState HttpNetworkTransaction::GetLoadState() const { switch (next_state_) { case STATE_RESOLVE_PROXY_COMPLETE: return LOAD_STATE_RESOLVING_PROXY_FOR_URL; - case STATE_RESOLVE_HOST_COMPLETE: - return LOAD_STATE_RESOLVING_HOST; - case STATE_TCP_CONNECT_COMPLETE: - return LOAD_STATE_CONNECTING; + case STATE_INIT_CONNECTION_COMPLETE: + return connection_.GetLoadState(); case STATE_WRITE_HEADERS_COMPLETE: case STATE_WRITE_BODY_COMPLETE: return LOAD_STATE_SENDING_REQUEST; @@ -375,10 +373,10 @@ uint64 HttpNetworkTransaction::GetUploadProgress() const { } HttpNetworkTransaction::~HttpNetworkTransaction() { - // If we still have an open socket, then make sure to close it so we don't - // try to reuse it later on. + // If we still have an open socket, then make sure to disconnect it so we + // don't try to reuse it later on. if (connection_.is_initialized()) - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); if (pac_request_) session_->proxy_service()->CancelPacRequest(pac_request_); @@ -426,24 +424,6 @@ int HttpNetworkTransaction::DoLoop(int result) { rv = DoInitConnectionComplete(rv); TRACE_EVENT_END("http.init_conn", request_, request_->url.spec()); break; - case STATE_RESOLVE_HOST: - DCHECK_EQ(OK, rv); - TRACE_EVENT_BEGIN("http.resolve_host", request_, request_->url.spec()); - rv = DoResolveHost(); - break; - case STATE_RESOLVE_HOST_COMPLETE: - rv = DoResolveHostComplete(rv); - TRACE_EVENT_END("http.resolve_host", request_, request_->url.spec()); - break; - case STATE_TCP_CONNECT: - DCHECK_EQ(OK, rv); - TRACE_EVENT_BEGIN("http.connect", request_, request_->url.spec()); - rv = DoTCPConnect(); - break; - case STATE_TCP_CONNECT_COMPLETE: - rv = DoTCPConnectComplete(rv); - TRACE_EVENT_END("http.connect", request_, request_->url.spec()); - break; case STATE_SSL_CONNECT: DCHECK_EQ(OK, rv); TRACE_EVENT_BEGIN("http.ssl_connect", request_, request_->url.spec()); @@ -552,84 +532,43 @@ int HttpNetworkTransaction::DoInitConnection() { using_tunnel_ = !proxy_info_.is_direct() && using_ssl_; // Build the string used to uniquely identify connections of this type. + // Determine the host and port to connect to. std::string connection_group; - if (using_proxy_ || using_tunnel_) - connection_group = "proxy/" + proxy_info_.proxy_server().ToURI() + "/"; + std::string host; + int port; + if (using_proxy_ || using_tunnel_) { + ProxyServer proxy_server = proxy_info_.proxy_server(); + connection_group = "proxy/" + proxy_server.ToURI() + "/"; + host = proxy_server.HostNoBrackets(); + port = proxy_server.port(); + } else { + host = request_->url.HostNoBrackets(); + port = request_->url.EffectiveIntPort(); + } if (!using_proxy_) connection_group.append(request_->url.GetOrigin().spec()); DCHECK(!connection_group.empty()); - return connection_.Init(connection_group, request_->priority, &io_callback_); + int rv = connection_.Init(connection_group, host, port, request_->priority, + &io_callback_); + return rv; } int HttpNetworkTransaction::DoInitConnectionComplete(int result) { if (result < 0) - return result; + return ReconsiderProxyAfterError(result); DCHECK(connection_.is_initialized()); // Set the reused_socket_ flag to indicate that we are using a keep-alive // connection. This flag is used to handle errors that occur while we are // trying to reuse a keep-alive connection. - reused_socket_ = (connection_.socket() != NULL); + reused_socket_ = connection_.is_reused(); if (reused_socket_) { next_state_ = STATE_WRITE_HEADERS; } else { - next_state_ = STATE_RESOLVE_HOST; - } - return OK; -} - -int HttpNetworkTransaction::DoResolveHost() { - next_state_ = STATE_RESOLVE_HOST_COMPLETE; - - std::string host; - int port; - - // Determine the host and port to connect to. - if (using_proxy_ || using_tunnel_) { - ProxyServer proxy_server = proxy_info_.proxy_server(); - host = proxy_server.HostNoBrackets(); - port = proxy_server.port(); - } else { - // Direct connection - host = request_->url.HostNoBrackets(); - port = request_->url.EffectiveIntPort(); - } - - host_resolution_start_time_ = base::Time::Now(); - - DidStartDnsResolution(host, this); - return resolver_.Resolve(host, port, &addresses_, &io_callback_); -} - -int HttpNetworkTransaction::DoResolveHostComplete(int result) { - bool ok = (result == OK); - DidFinishDnsResolutionWithStatus(ok, request_->referrer, this); - if (ok) { - next_state_ = STATE_TCP_CONNECT; - } else { - result = ReconsiderProxyAfterError(result); - } - return result; -} - -int HttpNetworkTransaction::DoTCPConnect() { - next_state_ = STATE_TCP_CONNECT_COMPLETE; - - DCHECK(!connection_.socket()); - - connect_start_time_ = base::Time::Now(); - - ClientSocket* s = socket_factory_->CreateTCPClientSocket(addresses_); - connection_.set_socket(s); - return connection_.socket()->Connect(&io_callback_); -} - -int HttpNetworkTransaction::DoTCPConnectComplete(int result) { - // If we are using a direct SSL connection, then go ahead and establish the - // SSL connection, now. Otherwise, we need to first issue a CONNECT request. - if (result == OK) { + // Now we have a TCP connected socket. Perform other connection setup as + // needed. LogTCPConnectedMetrics(); if (using_ssl_ && !using_tunnel_) { next_state_ = STATE_SSL_CONNECT; @@ -638,10 +577,8 @@ int HttpNetworkTransaction::DoTCPConnectComplete(int result) { if (using_tunnel_) establishing_tunnel_ = true; } - } else { - result = ReconsiderProxyAfterError(result); } - return result; + return OK; } int HttpNetworkTransaction::DoSSLConnect() { @@ -942,7 +879,7 @@ int HttpNetworkTransaction::DoReadBodyComplete(int result) { if (done) { LogTransactionMetrics(); if (!keep_alive) - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); connection_.Reset(); // The next Read call will return 0 (EOF). } @@ -1014,15 +951,6 @@ int HttpNetworkTransaction::DoDrainBodyForAuthRestartComplete(int result) { } void HttpNetworkTransaction::LogTCPConnectedMetrics() const { - DCHECK(connect_start_time_ != base::Time()); - base::TimeDelta connect_duration = - base::Time::Now() - connect_start_time_; - - UMA_HISTOGRAM_CLIPPED_TIMES(FieldTrial::MakeName( - "Net.TCP_Connection_Latency", "DnsImpact").data(), connect_duration, - base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromMinutes(10), - 100); - base::TimeDelta host_resolution_and_tcp_connection_latency = base::Time::Now() - host_resolution_start_time_; @@ -1295,7 +1223,7 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) { // This could be a TLS-intolerant server or an SSL 3.0 server that // chose a TLS-only cipher suite. Turn off TLS 1.0 and retry. ssl_config_.tls1_enabled = false; - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); connection_.Reset(); next_state_ = STATE_INIT_CONNECTION; error = OK; @@ -1358,7 +1286,7 @@ bool HttpNetworkTransaction::ShouldResendRequest() const { } void HttpNetworkTransaction::ResetConnectionAndRequestForResend() { - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); connection_.Reset(); // There are two reasons we need to clear request_headers_. 1) It contains // the real request headers, but we may need to resend the CONNECT request @@ -1403,7 +1331,7 @@ int HttpNetworkTransaction::ReconsiderProxyAfterError(int error) { int rv = session_->proxy_service()->ReconsiderProxyAfterError( request_->url, &proxy_info_, &io_callback_, &pac_request_); if (rv == OK || rv == ERR_IO_PENDING) { - connection_.set_socket(NULL); + connection_.socket()->Disconnect(); connection_.Reset(); DCHECK(!request_headers_bytes_sent_); next_state_ = STATE_RESOLVE_PROXY_COMPLETE; diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index 57bd6cc..3c3811d 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -12,8 +12,11 @@ #include "base/time.h" #include "net/base/address_list.h" #include "net/base/client_socket_handle.h" +#include "net/base/client_socket_pool.h" #include "net/base/host_resolver.h" #include "net/base/io_buffer.h" +#include "net/base/load_flags.h" +#include "net/base/load_states.h" #include "net/base/ssl_config_service.h" #include "net/http/http_auth.h" #include "net/http/http_auth_handler.h" @@ -88,6 +91,26 @@ class HttpNetworkTransaction : public HttpTransaction { scoped_ptr_malloc<char> headers_; }; + enum State { + STATE_RESOLVE_PROXY, + STATE_RESOLVE_PROXY_COMPLETE, + STATE_INIT_CONNECTION, + STATE_INIT_CONNECTION_COMPLETE, + STATE_SSL_CONNECT, + STATE_SSL_CONNECT_COMPLETE, + STATE_WRITE_HEADERS, + STATE_WRITE_HEADERS_COMPLETE, + STATE_WRITE_BODY, + STATE_WRITE_BODY_COMPLETE, + STATE_READ_HEADERS, + STATE_READ_HEADERS_COMPLETE, + STATE_READ_BODY, + STATE_READ_BODY_COMPLETE, + STATE_DRAIN_BODY_FOR_AUTH_RESTART, + STATE_DRAIN_BODY_FOR_AUTH_RESTART_COMPLETE, + STATE_NONE + }; + void DoCallback(int result); void OnIOComplete(int result); @@ -102,10 +125,6 @@ class HttpNetworkTransaction : public HttpTransaction { int DoResolveProxyComplete(int result); int DoInitConnection(); int DoInitConnectionComplete(int result); - int DoResolveHost(); - int DoResolveHostComplete(int result); - int DoTCPConnect(); - int DoTCPConnectComplete(int result); int DoSSLConnect(); int DoSSLConnectComplete(int result); int DoWriteHeaders(); @@ -363,29 +382,7 @@ class HttpNetworkTransaction : public HttpTransaction { // The time the host resolution started (if it indeed got started). base::Time host_resolution_start_time_; - enum State { - STATE_RESOLVE_PROXY, - STATE_RESOLVE_PROXY_COMPLETE, - STATE_INIT_CONNECTION, - STATE_INIT_CONNECTION_COMPLETE, - STATE_RESOLVE_HOST, - STATE_RESOLVE_HOST_COMPLETE, - STATE_TCP_CONNECT, - STATE_TCP_CONNECT_COMPLETE, - STATE_SSL_CONNECT, - STATE_SSL_CONNECT_COMPLETE, - STATE_WRITE_HEADERS, - STATE_WRITE_HEADERS_COMPLETE, - STATE_WRITE_BODY, - STATE_WRITE_BODY_COMPLETE, - STATE_READ_HEADERS, - STATE_READ_HEADERS_COMPLETE, - STATE_READ_BODY, - STATE_READ_BODY_COMPLETE, - STATE_DRAIN_BODY_FOR_AUTH_RESTART, - STATE_DRAIN_BODY_FOR_AUTH_RESTART_COMPLETE, - STATE_NONE - }; + // The next state in the state machine. State next_state_; }; diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index c8435d2f..da89422 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -36,8 +36,9 @@ ProxyService* CreateFixedProxyService(const std::string& proxy) { } -HttpNetworkSession* CreateSession(ProxyService* proxy_service) { - return new HttpNetworkSession(proxy_service); +HttpNetworkSession* CreateSession(ProxyService* proxy_service, + ClientSocketFactory* client_socket_factory) { + return new HttpNetworkSession(proxy_service, client_socket_factory); } class HttpNetworkTransactionTest : public PlatformTest { @@ -61,8 +62,10 @@ class HttpNetworkTransactionTest : public PlatformTest { SimpleGetHelperResult out; scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -150,8 +153,10 @@ std::string MockGetHostName() { TEST_F(HttpNetworkTransactionTest, Basic) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); } TEST_F(HttpNetworkTransactionTest, SimpleGET) { @@ -261,8 +266,10 @@ TEST_F(HttpNetworkTransactionTest, StopsReading204) { // message body (since HEAD has none). TEST_F(HttpNetworkTransactionTest, Head) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "HEAD"; @@ -323,7 +330,7 @@ TEST_F(HttpNetworkTransactionTest, Head) { TEST_F(HttpNetworkTransactionTest, ReuseConnection) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); scoped_refptr<HttpNetworkSession> session = - CreateSession(proxy_service.get()); + CreateSession(proxy_service.get(), &mock_socket_factory_); MockRead data_reads[] = { MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"), @@ -372,8 +379,10 @@ TEST_F(HttpNetworkTransactionTest, ReuseConnection) { TEST_F(HttpNetworkTransactionTest, Ignores100) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "POST"; @@ -417,8 +426,10 @@ TEST_F(HttpNetworkTransactionTest, Ignores100) { // HTTP/1.1. TEST_F(HttpNetworkTransactionTest, Ignores1xx) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -461,7 +472,7 @@ void HttpNetworkTransactionTest::KeepAliveConnectionResendRequestTest( const MockRead& read_failure) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); scoped_refptr<HttpNetworkSession> session = - CreateSession(proxy_service.get()); + CreateSession(proxy_service.get(), &mock_socket_factory_); HttpRequestInfo request; request.method = "GET"; @@ -527,8 +538,10 @@ TEST_F(HttpNetworkTransactionTest, KeepAliveConnectionEOF) { TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionReset) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -581,8 +594,10 @@ TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionEOF) { // (basic auth is the easiest to mock, because it has no randomness). TEST_F(HttpNetworkTransactionTest, BasicAuth) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -670,8 +685,10 @@ TEST_F(HttpNetworkTransactionTest, BasicAuth) { // connection. TEST_F(HttpNetworkTransactionTest, BasicAuthKeepAlive) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -746,8 +763,10 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthKeepAlive) { // connection and with no response body to drain. TEST_F(HttpNetworkTransactionTest, BasicAuthKeepAliveNoBody) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -825,8 +844,10 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthKeepAliveNoBody) { // connection and with a large response body to drain. TEST_F(HttpNetworkTransactionTest, BasicAuthKeepAliveLargeBody) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -911,7 +932,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyKeepAlive) { CreateFixedProxyService("myproxy:70")); scoped_refptr<HttpNetworkSession> session( - CreateSession(proxy_service.get())); + CreateSession(proxy_service.get(), &mock_socket_factory_)); scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( session.get(), &mock_socket_factory_)); @@ -1011,7 +1032,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyCancelTunnel) { CreateFixedProxyService("myproxy:70")); scoped_refptr<HttpNetworkSession> session( - CreateSession(proxy_service.get())); + CreateSession(proxy_service.get(), &mock_socket_factory_)); scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( session.get(), &mock_socket_factory_)); @@ -1068,7 +1089,7 @@ void HttpNetworkTransactionTest::ConnectStatusHelperWithExpectedStatus( CreateFixedProxyService("myproxy:70")); scoped_refptr<HttpNetworkSession> session( - CreateSession(proxy_service.get())); + CreateSession(proxy_service.get(), &mock_socket_factory_)); scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( session.get(), &mock_socket_factory_)); @@ -1283,7 +1304,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) { // Configure against proxy server "myproxy:70". scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), + CreateSession(proxy_service.get(), &mock_socket_factory_), &mock_socket_factory_)); HttpRequestInfo request; @@ -1420,8 +1441,10 @@ TEST_F(HttpNetworkTransactionTest, NTLMAuth1) { MockGetHostName); scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -1546,8 +1569,10 @@ TEST_F(HttpNetworkTransactionTest, NTLMAuth2) { MockGetHostName); scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -1752,8 +1777,10 @@ TEST_F(HttpNetworkTransactionTest, NTLMAuth2) { // fail with ERR_RESPONSE_HEADERS_TOO_BIG. TEST_F(HttpNetworkTransactionTest, LargeHeadersNoBody) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -1795,7 +1822,7 @@ TEST_F(HttpNetworkTransactionTest, DontRecycleTCPSocketForSSLTunnel) { CreateFixedProxyService("myproxy:70")); scoped_refptr<HttpNetworkSession> session( - CreateSession(proxy_service.get())); + CreateSession(proxy_service.get(), &mock_socket_factory_)); scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( session.get(), &mock_socket_factory_)); @@ -1853,7 +1880,7 @@ TEST_F(HttpNetworkTransactionTest, DontRecycleTCPSocketForSSLTunnel) { TEST_F(HttpNetworkTransactionTest, RecycleSocket) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); scoped_refptr<HttpNetworkSession> session( - CreateSession(proxy_service.get())); + CreateSession(proxy_service.get(), &mock_socket_factory_)); scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( session.get(), &mock_socket_factory_)); @@ -1912,7 +1939,7 @@ TEST_F(HttpNetworkTransactionTest, RecycleSocket) { TEST_F(HttpNetworkTransactionTest, RecycleSocketAfterZeroContentLength) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); scoped_refptr<HttpNetworkSession> session( - CreateSession(proxy_service.get())); + CreateSession(proxy_service.get(), &mock_socket_factory_)); scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( session.get(), &mock_socket_factory_)); @@ -1986,7 +2013,7 @@ TEST_F(HttpNetworkTransactionTest, ResendRequestOnWriteBodyError) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); scoped_refptr<HttpNetworkSession> session = - CreateSession(proxy_service.get()); + CreateSession(proxy_service.get(), &mock_socket_factory_); // The first socket is used for transaction 1 and the first attempt of // transaction 2. @@ -2062,8 +2089,10 @@ TEST_F(HttpNetworkTransactionTest, ResendRequestOnWriteBodyError) { // it fails the identity from the URL is used to answer the challenge. TEST_F(HttpNetworkTransactionTest, AuthIdentityInUrl) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -2140,7 +2169,7 @@ TEST_F(HttpNetworkTransactionTest, AuthIdentityInUrl) { TEST_F(HttpNetworkTransactionTest, BasicAuthCacheAndPreauth) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); scoped_refptr<HttpNetworkSession> session = - CreateSession(proxy_service.get()); + CreateSession(proxy_service.get(), &mock_socket_factory_); // Transaction 1: authenticate (foo, bar) on MyRealm1 { @@ -2549,8 +2578,10 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthCacheAndPreauth) { TEST_F(HttpNetworkTransactionTest, ResetStateForRestart) { // Create a transaction (the dependencies aren't important). scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpNetworkTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpNetworkTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); // Setup some state (which we expect ResetStateForRestart() will clear). trans->header_buf_->Realloc(10); @@ -2605,8 +2636,10 @@ TEST_F(HttpNetworkTransactionTest, ResetStateForRestart) { // Test HTTPS connections to a site with a bad certificate TEST_F(HttpNetworkTransactionTest, HTTPSBadCertificate) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -2708,8 +2741,10 @@ TEST_F(HttpNetworkTransactionTest, HTTPSBadCertificateViaProxy) { for (int i = 0; i < 2; i++) { mock_socket_factory_.ResetNextMockIndexes(); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); int rv = trans->Start(&request, &callback); EXPECT_EQ(ERR_IO_PENDING, rv); @@ -2732,8 +2767,10 @@ TEST_F(HttpNetworkTransactionTest, HTTPSBadCertificateViaProxy) { TEST_F(HttpNetworkTransactionTest, BuildRequest_UserAgent) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -2771,8 +2808,10 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_UserAgent) { TEST_F(HttpNetworkTransactionTest, BuildRequest_Referer) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -2811,8 +2850,10 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_Referer) { TEST_F(HttpNetworkTransactionTest, BuildRequest_PostContentLengthZero) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "POST"; @@ -2849,8 +2890,10 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_PostContentLengthZero) { TEST_F(HttpNetworkTransactionTest, BuildRequest_PutContentLengthZero) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "PUT"; @@ -2887,8 +2930,10 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_PutContentLengthZero) { TEST_F(HttpNetworkTransactionTest, BuildRequest_HeadContentLengthZero) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "HEAD"; @@ -2925,8 +2970,10 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_HeadContentLengthZero) { TEST_F(HttpNetworkTransactionTest, BuildRequest_CacheControlNoCache) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -2966,8 +3013,10 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_CacheControlNoCache) { TEST_F(HttpNetworkTransactionTest, BuildRequest_CacheControlValidateCache) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; @@ -3005,8 +3054,10 @@ TEST_F(HttpNetworkTransactionTest, TEST_F(HttpNetworkTransactionTest, BuildRequest_ExtraHeaders) { scoped_ptr<ProxyService> proxy_service(CreateNullProxyService()); - scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction( - CreateSession(proxy_service.get()), &mock_socket_factory_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction( + CreateSession(proxy_service.get(), &mock_socket_factory_), + &mock_socket_factory_)); HttpRequestInfo request; request.method = "GET"; |