Makes waiting SSLConnectJobs use the message loops to resume their connection.

net/socket/ssl_client_socket_pool.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/ssl_client_socket_pool.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
@@ skipping 117 matching lines @@
 void SSLConnectJobMessenger::OnJobFailed() {
   if (pending_sockets_and_callbacks_.Empty())
     return;
   base::Closure cb = pending_sockets_and_callbacks_.GetFirstCallback();
   connecting_sockets_.erase(connecting_sockets_.begin());
   SSLClientSocket* ssl_socket = pending_sockets_and_callbacks_.GetFirstSocket();
   ssl_socket->SetIsLeader();
   connecting_sockets_.push_back(ssl_socket);
   pending_sockets_and_callbacks_.EraseFirstEntry();
   MonitorConnectionResult(ssl_socket);
-  cb.Run();
+  base::MessageLoopForIO::current()->PostTask(FROM_HERE, cb);

[wtc, 2014/07/08 17:37:03]

IMPORTANT: please make sure the task won't run if |ssl_socket| is destroyed
after the task is posted but before the message loop is ready to run the task.

[Ryan Sleevi, 2014/07/08 19:36:53]

Note: You should not use base::MessageLoopForIO directly.

The "old" path was using base::MessageLoop::current() [which always returns the
MessageLoop of the current thread, and you're always guaranteed that this thread
will be the IO thread by virtue of these classes being single threaded]

The "new" path is to use ThreadTaskRunnerHandle::Get()->PostTask() instead.

However, as Wan-Teh mentioned, we need to ensure that this callback is safe to
run even when the socket is deleted. We should be able to ensure this by
ensuring the callback, when registered (and later returned by
GetFirstCallback()) uses a WeakPtr.

Another thing to consider is to actually schedule a callback to grab the first
callback, rather than grabing it on line 131.

imagine you have sockets 1, 2, and 3 - with 1 being the leader.

If 1 connects, and you decide to start connecting 2, you could PostTask, but
then 2 could be deleted.
1) If 2 is deleted, you need to make sure the unregistration code can handle
when 2 is no longer in pending_sockets_and_callbacks (eg: all the state in line
113-136)
2) If 2 is deleted, what you want is to allow 3 to resume, not 2.

For this reason, I'd suggest you actually do a PostTask that runs a method on
the SSLCJM, which then does what you do on lines 129-138, and use an (internal
only) weakptr to the SSLCJM.

This will ensure that if 2 is deleted, by the time the CB runs, 3 is what is
treated as the leader (line 134) and the CB that is run.

[mshelley, 2014/07/09 21:53:06]

So I think the issue that you're referring to could happen if socket 1 fails to
connect -- if socket 1 connects, then all of the pending socket's callbacks will
be run regardless of whether one of them fails (i.e. there is no leader). 

To make sure that I understand what you're suggesting: I could have OnJobsFailed
post a task with callback to another SSLCJM method, which would essentially do
the same thing as OnJobsFailed, except it would look for the first socket in
pending_sockets_and_callbacks that had not been deleted (not just the first
socket), and then treat that socket as the leader and directly run its callback.

 }
 
 // Timeout for the SSL handshake portion of the connect.
 static const int kSSLHandshakeTimeoutInSeconds = 30;
 
 SSLConnectJob::SSLConnectJob(const std::string& group_name,
                              RequestPriority priority,
                              const scoped_refptr<SSLSocketParams>& params,
                              const base::TimeDelta& timeout_duration,
                              TransportClientSocketPool* transport_pool,
@@ skipping 652 matching lines @@
   if (base_.CloseOneIdleSocket())
     return true;
   return base_.CloseOneIdleConnectionInHigherLayeredPool();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net