Refactoring needed before adding good unit tests for limiting total socket count.

Refactoring needed before adding good unit tests for limiting total socket count.

TEST=Covered by net_unittests.
http://crbug.com/15093

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=20623

[eroman, 2009-07-14 00:11:03]

LGTM.

I like this refactor! I find it easier to read than the earlier data-driven
approach.

http://codereview.chromium.org/149545/diff/1/3
File net/socket/client_socket_pool_base_unittest.cc (right):

http://codereview.chromium.org/149545/diff/1/3#newcode296
Line 296: int CreateRequest(const std::string& group_name, int priority) {
Perhaps this could be named "StartRequest".

http://codereview.chromium.org/149545/diff/1/3#newcode303
Line 303: if (rv == OK)
Seems this should be:

if (rv != ERR_IO_PENDING)

So request_order_ includes the synchronous *failures* as well.

http://codereview.chromium.org/149545/diff/1/3#newcode311
Line 311: return -2;
Maybe these two magic values can be wrapped in an enum.

[Paweł Hajdan Jr., 2009-07-14 00:48:59]

On 2009/07/14 00:11:03, eroman wrote:
> http://codereview.chromium.org/149545/diff/1/3#newcode296
> Line 296: int CreateRequest(const std::string& group_name, int priority) {
> Perhaps this could be named "StartRequest".

Done.

> http://codereview.chromium.org/149545/diff/1/3#newcode303
> Line 303: if (rv == OK)
> Seems this should be:
> 
> if (rv != ERR_IO_PENDING)
> 
> So request_order_ includes the synchronous *failures* as well.

Done.

> http://codereview.chromium.org/149545/diff/1/3#newcode311
> Line 311: return -2;
> Maybe these two magic values can be wrapped in an enum.

Named constants seemed more natural to me here (because we also return "normal"
ints out of the range of such enum). What do you think?

[eroman, 2009-07-14 01:22:33]

LGTM

http://codereview.chromium.org/149545/diff/1/3
File net/socket/client_socket_pool_base_unittest.cc (right):

http://codereview.chromium.org/149545/diff/1/3#newcode311
Line 311: return -2;
On 2009/07/14 00:11:03, eroman wrote:
> Maybe these two magic values can be wrapped in an enum.

sounds good to me.