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 76 matching lines @@
   DCHECK_EQ(GetConnectionType(), SOCKS_PROXY);
   return socks_proxy_params_;
 }
 
 const scoped_refptr<HttpProxySocketParams>&
 SSLSocketParams::GetHttpProxyConnectionParams() const {
   DCHECK_EQ(GetConnectionType(), HTTP_PROXY);
   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.
   if (ssl_socket->InSessionCache() || connecting_sockets_.empty()) {
     return true;
   }
   return false;
 }
 
 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,
                                               const base::Closure& callback) {
   pending_sockets_and_callbacks_.push_back(SocketAndCallback(socket, callback));
 }
 
 void SSLConnectJobMessenger::OnJobSucceeded() {
   SSLPendingSocketsAndCallbacks temp_list = pending_sockets_and_callbacks_;
   pending_sockets_and_callbacks_.clear();
   connecting_sockets_.clear();
   RunAllJobs(temp_list);
 }
 
 void SSLConnectJobMessenger::OnJobFailed() {
-  if (pending_sockets_and_callbacks_.empty())
+  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;
+  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;
-  base::Closure callback = pending_sockets_and_callbacks_[0].callback;
-  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());
+
+  // 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();
 }
 
 void SSLConnectJobMessenger::RunAllJobs(
     std::vector<SocketAndCallback>& pending_sockets_and_callbacks) {
   for (std::vector<SocketAndCallback>::const_iterator it =
            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.
 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 648 matching lines @@
   if (base_.CloseOneIdleSocket())
     return true;
   return base_.CloseOneIdleConnectionInHigherLayeredPool();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net