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

net/socket/ssl_client_socket_pool.cc

Index: net/socket/ssl_client_socket_pool.cc
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index 27fb87f85549feb374ba066d3361114ca29808dd..5480060d44e76024098a3b95c86d962178a465f8 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -94,6 +94,9 @@ SSLSocketParams::GetHttpProxyConnectionParams() const {
   return http_proxy_params_;
 }
 
+SSLConnectJobMessenger::SSLConnectJobMessenger() : weak_factory_(this) {
+}
+
 bool SSLConnectJobMessenger::CanProceed(SSLClientSocket* ssl_socket) {
   // If the session is in the session cache, or there are no connecting
   // sockets allow the connection to proceed.
@@ -105,11 +108,12 @@ bool SSLConnectJobMessenger::CanProceed(SSLClientSocket* ssl_socket) {
 
 void SSLConnectJobMessenger::MonitorConnectionResult(
     SSLClientSocket* ssl_socket) {
+  connecting_sockets_.push_back(ssl_socket);
   ssl_socket->SetIsLeader();
-  ssl_socket->SetSocketFailureCallback(
-      base::Bind(&SSLConnectJobMessenger::OnJobFailed, base::Unretained(this)));
+  ssl_socket->SetSocketFailureCallback(base::Bind(
+      &SSLConnectJobMessenger::OnJobFailed, weak_factory_.GetWeakPtr()));
   ssl_socket->WatchSessionForCompletion(base::Bind(
-      &SSLConnectJobMessenger::OnJobSucceeded, base::Unretained(this)));
+      &SSLConnectJobMessenger::OnJobSucceeded, weak_factory_.GetWeakPtr()));

[Ryan Sleevi, 2014/07/10 00:03:22]

It would be good to document why you're using WeakPtr here, in that it's
non-obvious (though it's what we discussed)

[mshelley, 2014/07/10 00:44:20]

Done.

 }
 
 void SSLConnectJobMessenger::AddPendingSocket(SSLClientSocket* socket,
@@ -125,12 +129,38 @@ void SSLConnectJobMessenger::OnJobSucceeded() {
 }
 
 void SSLConnectJobMessenger::OnJobFailed() {
-  if (pending_sockets_and_callbacks_.empty())
-    return;
-  base::Closure callback = pending_sockets_and_callbacks_[0].callback;
+  base::Closure callback = base::Bind(&SSLConnectJobMessenger::ConnectNewLeader,
+                                      weak_factory_.GetWeakPtr());
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
+}
+
+void SSLConnectJobMessenger::ConnectNewLeader() {
   connecting_sockets_.erase(connecting_sockets_.begin());
-  SSLClientSocket* ssl_socket = pending_sockets_and_callbacks_[0].socket;
-  pending_sockets_and_callbacks_.erase(pending_sockets_and_callbacks_.begin());
+  SSLClientSocket* ssl_socket;
+  base::Closure callback;
+  std::vector<SocketAndCallback>::iterator it;

[Ryan Sleevi, 2014/07/10 00:03:22]

anal retentive: Would nice to have declaration order match assignment order (it
-> socket -> callback)

[mshelley, 2014/07/10 00:44:20]

Done.

+
+  // Connect the first pending socket that has not been deleted.
+  for (it = pending_sockets_and_callbacks_.begin();
+       it != pending_sockets_and_callbacks_.end();
+       ++it) {
+    if (it->socket != NULL) {
+      ssl_socket = it->socket;
+      callback = it->callback;
+      break;
+    }
+  }
+
+  // If there were no valid pending sockets, return.
+  if (it == pending_sockets_and_callbacks_.end())
+    return;
+
+  // Erase all deleted sockets and their callbacks, as well as the socket that
+  // will
+  // be connected next.
+  pending_sockets_and_callbacks_.erase(pending_sockets_and_callbacks_.begin(),
+                                       ++it);
+
   MonitorConnectionResult(ssl_socket);
   callback.Run();
 }
@@ -141,7 +171,7 @@ void SSLConnectJobMessenger::RunAllJobs(
            pending_sockets_and_callbacks.begin();
        it != pending_sockets_and_callbacks.end();
        ++it)
-    it->callback.Run();
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, it->callback);

[Ryan Sleevi, 2014/07/10 00:03:22]

Lifetime question: This seems another situation where we might have a callback
de-register itself in between when the PostTask happened and it's actually run.

Is that possible? Expected? While the callback-binder can always use WeakPtr to
handle an unexpected callback, it's "nicer" to provide clear lifetime semantics
about when/how things will be called (even if it is much harder)

[mshelley, 2014/07/10 00:44:20]

I believe the only way that one of these callbacks could normally be
de-registered is if its SSLConnectJob was deleted. That shouldn't happen until
the SSLConnectJob has finished its connection. Since by definition these connect
jobs cannot finish their connections until their callback has been run, the
callbacks should not be de-registered. 

However, if for some reason the SSLConnectJobFactory was deleted before the jobs
finished connecting I guess that would be possible?

So I think my answer is it's possible but not expected. 

 }
 
 // Timeout for the SSL handshake portion of the connect.