Do not attempt to reuse active sockets after a socket pool flush (usually a network change).

Do not attempt to reuse active sockets after a socket pool flush (usually a network change).
Implements this functionality by adding an |id_| field to ClientSocketPoolBaseHelper that is incremented on each Flush().
BUG=45872
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49076

[eroman, 2010-06-04 22:43:44]

http://codereview.chromium.org/2647003/diff/1/11
File net/socket/socks_client_socket_pool.cc (right):

http://codereview.chromium.org/2647003/diff/1/11#newcode199
net/socket/socks_client_socket_pool.cc:199: int id) {
Is there a more specific name than "id" that can be used? Some of the ideas I
came up with to convey whose identifier this really is:

|pool_generation_number|
|pool_generation|
|parent_pool_generation|

[willchan no longer on Chromium, 2010-06-04 22:55:13]

http://codereview.chromium.org/2647003/diff/1/11
File net/socket/socks_client_socket_pool.cc (right):

http://codereview.chromium.org/2647003/diff/1/11#newcode199
net/socket/socks_client_socket_pool.cc:199: int id) {
On 2010/06/04 22:43:44, eroman wrote:
> Is there a more specific name than "id" that can be used? Some of the ideas I
> came up with to convey whose identifier this really is:
> 
> |pool_generation_number|
> |pool_generation|
> |parent_pool_generation|

I chose |id_| for consistency with ProxyConfig, which does a similar thing.  I'm
happy to change it to something else, although I don't really like anything :P 
So I guess if you want it changed, I'll just do |pool_generation_number|.  How
does that sound?

[eroman, 2010-06-04 23:09:18]

lgtm

http://codereview.chromium.org/2647003/diff/1/6
File net/socket/client_socket_handle.h (right):

http://codereview.chromium.org/2647003/diff/1/6#newcode146
net/socket/client_socket_handle.h:146: int pool_id_;
Can you document this? (or point to the other file where it is explained).

http://codereview.chromium.org/2647003/diff/1/7
File net/socket/client_socket_pool.h (right):

http://codereview.chromium.org/2647003/diff/1/7#newcode79
net/socket/client_socket_pool.h:79: // idle, active, and connecting sockets are
discarded.  Active sockets being
nit: I wouldn't mention "active" sockets as being discarded in the second
phrase. But rather let the following one explain how any active sockets will be
discarded once they are returned to the pool.

http://codereview.chromium.org/2647003/diff/1/11
File net/socket/socks_client_socket_pool.cc (right):

http://codereview.chromium.org/2647003/diff/1/11#newcode199
net/socket/socks_client_socket_pool.cc:199: int id) {
On 2010/06/04 22:55:13, willchan wrote:
> On 2010/06/04 22:43:44, eroman wrote:
> > Is there a more specific name than "id" that can be used? Some of the ideas
I
> > came up with to convey whose identifier this really is:
> > 
> > |pool_generation_number|
> > |pool_generation|
> > |parent_pool_generation|
> 
> I chose |id_| for consistency with ProxyConfig, which does a similar thing. 
I'm
> happy to change it to something else, although I don't really like anything :P

> So I guess if you want it changed, I'll just do |pool_generation_number|.  How
> does that sound?

For what its worth, I prefer pool_generation_number over ID, but i'll leave it
up to you.

(I don't like ProxyConfig's use of |id| either.)

http://codereview.chromium.org/2647003/diff/1/13
File net/socket/socks_client_socket_pool_unittest.cc (right):

http://codereview.chromium.org/2647003/diff/1/13#newcode127
net/socket/socks_client_socket_pool_unittest.cc:127: release_count_++;
Can you assert that id == 1?

[willchan no longer on Chromium, 2010-06-04 23:27:53]

http://codereview.chromium.org/2647003/diff/1/6
File net/socket/client_socket_handle.h (right):

http://codereview.chromium.org/2647003/diff/1/6#newcode146
net/socket/client_socket_handle.h:146: int pool_id_;
On 2010/06/04 23:09:18, eroman wrote:
> Can you document this? (or point to the other file where it is explained).

Done.

http://codereview.chromium.org/2647003/diff/1/7
File net/socket/client_socket_pool.h (right):

http://codereview.chromium.org/2647003/diff/1/7#newcode79
net/socket/client_socket_pool.h:79: // idle, active, and connecting sockets are
discarded.  Active sockets being
On 2010/06/04 23:09:18, eroman wrote:
> nit: I wouldn't mention "active" sockets as being discarded in the second
> phrase. But rather let the following one explain how any active sockets will
be
> discarded once they are returned to the pool.

Done.

http://codereview.chromium.org/2647003/diff/1/11
File net/socket/socks_client_socket_pool.cc (right):

http://codereview.chromium.org/2647003/diff/1/11#newcode199
net/socket/socks_client_socket_pool.cc:199: int id) {
On 2010/06/04 23:09:18, eroman wrote:
> On 2010/06/04 22:55:13, willchan wrote:
> > On 2010/06/04 22:43:44, eroman wrote:
> > > Is there a more specific name than "id" that can be used? Some of the
ideas
> I
> > > came up with to convey whose identifier this really is:
> > > 
> > > |pool_generation_number|
> > > |pool_generation|
> > > |parent_pool_generation|
> > 
> > I chose |id_| for consistency with ProxyConfig, which does a similar thing. 
> I'm
> > happy to change it to something else, although I don't really like anything
:P
> 
> > So I guess if you want it changed, I'll just do |pool_generation_number|. 
How
> > does that sound?
> 
> For what its worth, I prefer pool_generation_number over ID, but i'll leave it
> up to you.
> 
> (I don't like ProxyConfig's use of |id| either.)

Fair enough.  I think they all suck :)  Changed.

http://codereview.chromium.org/2647003/diff/1/13
File net/socket/socks_client_socket_pool_unittest.cc (right):

http://codereview.chromium.org/2647003/diff/1/13#newcode127
net/socket/socks_client_socket_pool_unittest.cc:127: release_count_++;
On 2010/06/04 23:09:18, eroman wrote:
> Can you assert that id == 1?

Done.