Add a bound on the total number of active sockets in ClientSocketPool....

Add a bound on the total number of active sockets in ClientSocketPool.

BUG=9598

[eroman, 2009-04-09 05:07:09]

I am still figuring out the unit-test, and will ping when that is uploaded.

[wtc, 2009-04-10 22:08:22]

LGTM.

One change you made is that you're deleting empty groups
more eagerly than before.  Is this deliberate?  In one
place (http://codereview.chromium.org/62181/diff/2001/2003#newcode220)
you made this change by removing a return statement
near a refactoring site, so I am not sure if the return
statement was removed by mistake or intentionally.

http://codereview.chromium.org/62181/diff/2001/2003
File net/base/client_socket_pool.cc (right):

http://codereview.chromium.org/62181/diff/2001/2003#newcode115
Line 115: // Delete group if no longer needed.
Are you sure you want to delete an empty group here in
CancelRequest?  I think we should delete an empty group
only if it has been empty for a while, so that we don't
delete it and immediately recreate it.

http://codereview.chromium.org/62181/diff/2001/2003#newcode214
Line 214: // Check if there are any stalled groups left.
You can just let FindTopStalledGroup() do this.  After
FindTopStalledGroup() iterates over all the groups, if
there is <= 1 stalled group, it can reset
may_have_stalled_group_ to false before returning.
This will eliminate the second FindTopStalledGroup call.

http://codereview.chromium.org/62181/diff/2001/2003#newcode220
Line 220: StartPendingRequestForGroup(&group);
The original code has a return statement here.  Why is
it removed?

http://codereview.chromium.org/62181/diff/2001/2003#newcode243
Line 243: Group* group = &i->second;
Nit: define |group| as Group& (a reference) to be consistent
with existing code.

http://codereview.chromium.org/62181/diff/2001/2003#newcode247
Line 247: top_group->pending_requests.front().priority)) {
Nit: does this line need to be indented more than the
previous line?

http://codereview.chromium.org/62181/diff/2001/2004
File net/base/client_socket_pool.h (right):

http://codereview.chromium.org/62181/diff/2001/2004#newcode41
Line 41: // TODO(eroman): would be nice to define these in terms of "open"
sockets
Why can't we define these max socket limits in terms of
"open" sockets now?

http://codereview.chromium.org/62181/diff/2001/2004#newcode170
Line 170: // of when it happens with the following variable, and thus avoid
doing
Nit: be more direct, instead of "the following variable",
just say "may_have_stalled_group_".

http://codereview.chromium.org/62181/diff/2001/2004#newcode172
Line 172: bool may_have_stalled_group_;
Nit: move this data member down to be with the other
data members.

http://codereview.chromium.org/62181/diff/2001/2004#newcode196
Line 196: int max_sockets_;
Should we rename this max_total_sockets_ or
max_sockets_all_groups_?