Commit 105a6a1b authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Socket Pools Refactor 29: Rework HTTP proxy auth (part 3).

This CL renames "pending" requests in ClientSocketPoolBase::Request to
"unbound" requests, after the introduction of the notion of "bound" in
https://chromium-review.googlesource.com/c/chromium/src/+/1481900

This is a separate CL to keep the CL it's on top of more focused.

This is part of an effort to flatten the socket pools.
https://docs.google.com/document/d/1g0EA4iDqaDhNXA_mq-YK3SlSX-xRkoKvZetAQqdRrxM/edit

Bug: 472729
Change-Id: I9ed0e4e4cd5e87be2d2b032fdd98313f420a621b
Reviewed-on: https://chromium-review.googlesource.com/c/1487791
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: default avatarAsanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636887}
parent 15f3437d
This diff is collapsed.
...@@ -237,6 +237,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -237,6 +237,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
ClientSocketPool::kMaxConnectRetryIntervalMs); ClientSocketPool::kMaxConnectRetryIntervalMs);
} }
// TODO(mmenke): de-inline these.
size_t NumNeverAssignedConnectJobsInGroup( size_t NumNeverAssignedConnectJobsInGroup(
const std::string& group_name) const { const std::string& group_name) const {
return group_map_.find(group_name)->second->never_assigned_job_count(); return group_map_.find(group_name)->second->never_assigned_job_count();
...@@ -323,8 +324,9 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -323,8 +324,9 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
using RequestQueue = PriorityQueue<std::unique_ptr<Request>>; using RequestQueue = PriorityQueue<std::unique_ptr<Request>>;
// A Group is allocated per group_name when there are idle sockets or pending // A Group is allocated per group_name when there are idle sockets, unbound
// requests. Otherwise, the Group object is removed from the map. // request, or bound requests. Otherwise, the Group object is removed from the
// map.
// //
// A request is "bound" to a ConnectJob when an unbound ConnectJob encounters // A request is "bound" to a ConnectJob when an unbound ConnectJob encounters
// a proxy HTTP auth challenge, and the auth challenge is presented to that // a proxy HTTP auth challenge, and the auth challenge is presented to that
...@@ -355,7 +357,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -355,7 +357,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
bool IsEmpty() const { bool IsEmpty() const {
return active_socket_count_ == 0 && idle_sockets_.empty() && return active_socket_count_ == 0 && idle_sockets_.empty() &&
jobs_.empty() && pending_requests_.empty() && jobs_.empty() && unbound_requests_.empty() &&
bound_requests_.empty(); bound_requests_.empty();
} }
...@@ -373,17 +375,17 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -373,17 +375,17 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
// it were given one. // it were given one.
bool CanUseAdditionalSocketSlot(int max_sockets_per_group) const { bool CanUseAdditionalSocketSlot(int max_sockets_per_group) const {
return HasAvailableSocketSlot(max_sockets_per_group) && return HasAvailableSocketSlot(max_sockets_per_group) &&
pending_requests_.size() > jobs_.size(); unbound_requests_.size() > jobs_.size();
} }
// Returns the priority of the top of the pending request queue // Returns the priority of the top of the unbound request queue
// (which may be less than the maximum priority over the entire // (which may be less than the maximum priority over the entire
// queue, due to how we prioritize requests with |respect_limits| // queue, due to how we prioritize requests with |respect_limits|
// DISABLED over others). // DISABLED over others).
RequestPriority TopPendingPriority() const { RequestPriority TopPendingPriority() const {
// NOTE: FirstMax().value()->priority() is not the same as // NOTE: FirstMax().value()->priority() is not the same as
// FirstMax().priority()! // FirstMax().priority()!
return pending_requests_.FirstMax().value()->priority(); return unbound_requests_.FirstMax().value()->priority();
} }
// Set a timer to create a backup job if it takes too long to // Set a timer to create a backup job if it takes too long to
...@@ -401,16 +403,12 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -401,16 +403,12 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
void AddJob(std::unique_ptr<ConnectJob> job, bool is_preconnect); void AddJob(std::unique_ptr<ConnectJob> job, bool is_preconnect);
// Remove |job| from this group, which must already own |job|. Returns the // Remove |job| from this group, which must already own |job|. Returns the
// removed ConnectJob. // removed ConnectJob.
std::unique_ptr<ConnectJob> RemoveJob(ConnectJob* job); std::unique_ptr<ConnectJob> RemoveUnboundJob(ConnectJob* job);
void RemoveAllJobs(); void RemoveAllUnboundJobs();
bool has_pending_requests() const { bool has_unbound_requests() const { return !unbound_requests_.empty(); }
return !pending_requests_.empty();
}
size_t pending_request_count() const { size_t unbound_request_count() const { return unbound_requests_.size(); }
return pending_requests_.size();
}
size_t ConnectJobCount() const; size_t ConnectJobCount() const;
...@@ -420,19 +418,19 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -420,19 +418,19 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
// Inserts the request into the queue based on priority // Inserts the request into the queue based on priority
// order. Older requests are prioritized over requests of equal // order. Older requests are prioritized over requests of equal
// priority. // priority.
void InsertPendingRequest(std::unique_ptr<Request> request); void InsertUnboundRequest(std::unique_ptr<Request> request);
// Gets (but does not remove) the next pending request. Returns // Gets (but does not remove) the next unbound request. Returns
// NULL if there are no pending requests. // NULL if there are no unbound requests.
const Request* GetNextPendingRequest() const; const Request* GetNextUnboundRequest() const;
// Gets and removes the next pending request. Returns NULL if // Gets and removes the next unbound request. Returns NULL if
// there are no pending requests. // there are no unbound requests.
std::unique_ptr<Request> PopNextPendingRequest(); std::unique_ptr<Request> PopNextUnboundRequest();
// Finds the pending request for |handle| and removes it. Returns // Finds the unbound request for |handle| and removes it. Returns
// the removed pending request, or NULL if there was none. // the removed unbound request, or NULL if there was none.
std::unique_ptr<Request> FindAndRemovePendingRequest( std::unique_ptr<Request> FindAndRemoveUnboundRequest(
ClientSocketHandle* handle); ClientSocketHandle* handle);
// Sets a pending error for all bound requests. Bound requests may be in the // Sets a pending error for all bound requests. Bound requests may be in the
...@@ -468,7 +466,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -468,7 +466,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
void IncrementActiveSocketCount() { active_socket_count_++; } void IncrementActiveSocketCount() { active_socket_count_++; }
void DecrementActiveSocketCount() { active_socket_count_--; } void DecrementActiveSocketCount() { active_socket_count_--; }
// Whether the request in |pending_requests_| with a given handle has a job. // Whether the request in |unbound_requests_| with a given handle has a job.
bool RequestWithHandleHasJobForTesting( bool RequestWithHandleHasJobForTesting(
const ClientSocketHandle* handle) const; const ClientSocketHandle* handle) const;
...@@ -501,17 +499,18 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -501,17 +499,18 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
int pending_error; int pending_error;
}; };
// Returns the iterator's pending request after removing it from // Returns the iterator's unbound request after removing it from
// the queue. Expects the Group to pass SanityCheck() when called. // the queue. Expects the Group to pass SanityCheck() when called.
std::unique_ptr<Request> RemovePendingRequest( std::unique_ptr<Request> RemoveUnboundRequest(
const RequestQueue::Pointer& pointer); const RequestQueue::Pointer& pointer);
// Finds the Request which is associated with the given ConnectJob. // Finds the Request which is associated with the given ConnectJob.
// Returns nullptr if none is found. Expects the Group to pass SanityCheck() // Returns nullptr if none is found. Expects the Group to pass SanityCheck()
// when called. // when called.
RequestQueue::Pointer FindRequestWithJob(const ConnectJob* job) const; RequestQueue::Pointer FindUnboundRequestWithJob(
const ConnectJob* job) const;
// Finds the Request in |pending_requests_| which is the first request // Finds the Request in |unbound_requests_| which is the first request
// without a job. Returns a null pointer if all requests have jobs. Does not // without a job. Returns a null pointer if all requests have jobs. Does not
// expect the Group to pass SanityCheck() when called, but does expect all // expect the Group to pass SanityCheck() when called, but does expect all
// jobs to either be assigned to a request or in |unassigned_jobs_|. Expects // jobs to either be assigned to a request or in |unassigned_jobs_|. Expects
...@@ -533,7 +532,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -533,7 +532,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
// expect that: // expect that:
// - the request associated with |request_pointer| must not have // - the request associated with |request_pointer| must not have
// an assigned ConnectJob, // an assigned ConnectJob,
// - the first min( jobs_.size(), pending_requests_.size() - 1 ) Requests // - the first min( jobs_.size(), unbound_requests_.size() - 1 ) Requests
// other than the given request must have ConnectJobs, i.e. the group // other than the given request must have ConnectJobs, i.e. the group
// must have passed SanityCheck() before the passed in Request was either // must have passed SanityCheck() before the passed in Request was either
// added or had its job unassigned. // added or had its job unassigned.
...@@ -573,16 +572,16 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper ...@@ -573,16 +572,16 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
JobList jobs_; // For bookkeeping purposes, there is a copy of the raw JobList jobs_; // For bookkeeping purposes, there is a copy of the raw
// pointer of each element of |jobs_| stored either in // pointer of each element of |jobs_| stored either in
// |unassigned_jobs_|, or as the associated |job_| of an // |unassigned_jobs_|, or as the associated |job_| of an
// element of |pending_requests_|. // element of |unbound_requests_|.
std::list<ConnectJob*> unassigned_jobs_; std::list<ConnectJob*> unassigned_jobs_;
RequestQueue pending_requests_; RequestQueue unbound_requests_;
int active_socket_count_; // number of active sockets used by clients int active_socket_count_; // number of active sockets used by clients
// A timer for when to start the backup job. // A timer for when to start the backup job.
base::OneShotTimer backup_job_timer_; base::OneShotTimer backup_job_timer_;
// List of Requests bound to ConnectJobs currently undergoing proxy auth. // List of Requests bound to ConnectJobs currently undergoing proxy auth.
// The Requests and ConnectJobs in this list do not appear in // The Requests and ConnectJobs in this list do not appear in
// |pending_requests_| or |jobs_|. // |unbound_requests_| or |jobs_|.
std::vector<BoundRequest> bound_requests_; std::vector<BoundRequest> bound_requests_;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment