Only create the backup ConnectJob when it is needed.

Only create the backup ConnectJob when it is needed.
This reduces the unnecessary NetLog spam, since we would log to the NetLog everytime we created the backup ConnectJob, even though we usually wouldn't actually call Connect() on it.  Now, we only create the backup ConnectJob when we intend to call Connect() on it.
Includes some various cleanup necessary for this:
  * struct Group => class Group
  * method_factory moves from CSP to Group

[Mike Belshe, 2010-08-23 04:41:00]

LGTM

http://codereview.chromium.org/3171017/diff/2001/3001
File net/socket/client_socket_pool_base.cc (right):

http://codereview.chromium.org/3171017/diff/2001/3001#newcode250
net/socket/client_socket_pool_base.cc:250:
group->StartBackupSocketTimer(group_name, this);
I wonder if you couldn't put this into Group::AddJob?  The Group class should be
smart enough to handle it automatically without help from the SocketPool now?

http://codereview.chromium.org/3171017/diff/2001/3001#newcode717
net/socket/client_socket_pool_base.cc:717: RemoveGroup(i++);
nit:  I find this late i++ stuff a little weird.  Why not put it into the
for(;;++i) clause?

[willchan no longer on Chromium, 2010-08-23 16:21:01]

http://codereview.chromium.org/3171017/diff/2001/3001
File net/socket/client_socket_pool_base.cc (right):

http://codereview.chromium.org/3171017/diff/2001/3001#newcode250
net/socket/client_socket_pool_base.cc:250:
group->StartBackupSocketTimer(group_name, this);
On 2010/08/23 04:41:00, Mike Belshe wrote:
> I wonder if you couldn't put this into Group::AddJob?  The Group class should
be
> smart enough to handle it automatically without help from the SocketPool now?

I looked into doing this, but AddJob() would have to pass in the |group_name|
and the |pool|.  It looks ugly.  I need to fix this by passing in those values
to Group's constructor or something.  I'm going to defer this to a later
refactor (I think I can move all the backup job stuff to the Group and then kick
it out to TCPClientSocketPool).

http://codereview.chromium.org/3171017/diff/2001/3001#newcode717
net/socket/client_socket_pool_base.cc:717: RemoveGroup(i++);
On 2010/08/23 04:41:00, Mike Belshe wrote:
> nit:  I find this late i++ stuff a little weird.  Why not put it into the
> for(;;++i) clause?

Legacy code.  I've fixed it now.