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"
 #include "base/metrics/sparse_histogram.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/transport_client_socket_pool.h"
@@ skipping 86 matching lines @@
            pending_sockets_and_callbacks_.begin();
        it != pending_sockets_and_callbacks_.end();
        ++it) {
     if (it->socket == ssl_socket) {
       pending_sockets_and_callbacks_.erase(it);
       break;
     }
   }
 }
 
+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.
   return ssl_socket->InSessionCache() || connecting_sockets_.empty();
 }
 
 void SSLConnectJobMessenger::MonitorConnectionResult(
     SSLClientSocket* ssl_socket) {
   connecting_sockets_.push_back(ssl_socket);
-  // TODO(mshelley): Both of these callbacks will use weak_ptr in future CL.
-  ssl_socket->SetHandshakeFailureCallback(
-      base::Bind(&SSLConnectJobMessenger::OnJobFailed, base::Unretained(this)));
+
+  ssl_socket->SetHandshakeFailureCallback(base::Bind(
+      &SSLConnectJobMessenger::OnJobFailed, weak_factory_.GetWeakPtr()));
   ssl_socket->SetHandshakeSuccessCallback(base::Bind(
-      &SSLConnectJobMessenger::OnJobSucceeded, base::Unretained(this)));
+      &SSLConnectJobMessenger::OnJobSucceeded, weak_factory_.GetWeakPtr()));
 }
 
 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();
   RunAllCallbacks(temp_list);
 }
 
 void SSLConnectJobMessenger::OnJobFailed() {
+  connecting_sockets_.erase(connecting_sockets_.begin());
+
+  // If there were no valid pending sockets, return.
   if (pending_sockets_and_callbacks_.empty())
     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;
+
+  SocketAndCallback socket_and_callback =
+      pending_sockets_and_callbacks_.front();
   pending_sockets_and_callbacks_.erase(pending_sockets_and_callbacks_.begin());
-  MonitorConnectionResult(ssl_socket);
-  callback.Run();
+
+  MonitorConnectionResult(socket_and_callback.socket);
+
+  // PostTask is used to avoid the recursive call sequence caused
+  // by SSLConnectJobMessengers running the callback directly.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                socket_and_callback.callback);

[mmenke, 2014/07/22 18:06:32]

Hrm...  Looks like we're fine if the second socket is destroyed either before or
after this function is called, but before the callback is executed.  Wonder if
it's worth testing the second case.  Suggested testing the first on the other
CL.

[mshelley, 2014/07/23 22:09:07]

Just to confirm: by "second case" you mean the case in which the socket is
destroyed after the function is called but before the callback is executed?  

Also, given that after my last patch to my big CL OnJobFailed will be handled in
the same manner as OnJobSucceeded (OnJobComplete), do you think this test is
still necessary? The connections of other pending sockets will no longer be
dependent upon the connection of the second socket, so I'm not sure if it is.

 }
 
 void SSLConnectJobMessenger::RunAllCallbacks(
     const SSLPendingSocketsAndCallbacks& pending_sockets_and_callbacks) {
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      base::ThreadTaskRunnerHandle::Get();
   for (std::vector<SocketAndCallback>::const_iterator it =
            pending_sockets_and_callbacks.begin();
        it != pending_sockets_and_callbacks.end();
-       ++it)
-    it->callback.Run();
+       ++it) {
+    task_runner->PostTask(FROM_HERE, it->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 660 matching lines @@
   if (base_.CloseOneIdleSocket())
     return true;
   return base_.CloseOneIdleConnectionInHigherLayeredPool();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net