Revert of Remove SSLClientAuthHandler's RDH dependency.

Revert of Remove SSLClientAuthHandler's RDH dependency. (patchset #8 id:140001 of https://codereview.chromium.org/596873002/)

Reason for revert:
Leak in SSLUITestWithClientCert.TestWSSClientCert

http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/4654/steps/browser_tests/logs/TestWSSClientCert

First reported leak:
Indirect leak of 104 byte(s) in 1 object(s) allocated from:
    #0 0x57659b in operator new(unsigned long) (/tmp/run_tha_test0QXqvt/out/Release/browser_tests+0x57659b)
    #1 0x2c7762d in ProfileIOData::ResourceContext::CreateClientCertStore() chrome/browser/profiles/profile_io_data.cc:920:7
    #2 0x6c908bd in content::ResourceLoader::OnCertificateRequested(net::URLRequest*, net::SSLCertRequestInfo*) content/browser/loader/resource_loader.cc:298:7
    #3 0x3e3e15f in net::URLRequest::NotifyCertificateRequested(net::SSLCertRequestInfo*) net/url_request/url_request.cc:1109:5
    #4 0x405ea0d in net::URLRequestHttpJob::OnStartCompleted(int) net/url_request/url_request_http_job.cc:925:5
    #5 0x3ccd86c in Run base/callback.h:396:12
    #6 0x3ccd86c in net::HttpCache::Transaction::DoCallback(int) net/http/http_cache_transaction.cc:710
    #7 0x3cca160 in HandleResult net/http/http_cache_transaction.cc:716:5
    #8 0x3cca160 in net::HttpCache::Transaction::DoLoop(int) net/http/http_cache_transaction.cc:977
    #9 0x3cc7a68 in net::HttpCache::Transaction::OnIOComplete(int) net/http/http_cache_transaction.cc:2934:3
    #10 0xc78765a in Run base/callback.h:396:12
    #11 0xc78765a in DevToolsNetworkTransaction::OnCallback(int) chrome/browser/devtools/devtools_network_transaction.cc:73
    #12 0x3ceab0c in Run base/callback.h:396:12
    #13 0x3ceab0c in DoCallback net/http/http_network_transaction.cc:577
    #14 0x3ceab0c in net::HttpNetworkTransaction::OnIOComplete(int) net/http/http_network_transaction.cc:594
    #15 0x394b58f in Run base/callback.h:396:12
    #16 0x394b58f in base::debug::TaskAnnotator::RunTask(char const*, char const*, base::PendingTask const&) base/debug/task_annotator.cc:63
    #17 0x388dd3c in base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:448:3
    #18 0x388e49b in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) base/message_loop/message_loop.cc:458:5
    #19 0x388ef57 in base::MessageLoop::DoWork() base/message_loop/message_loop.cc:567:13
    #20 0x3932db7 in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_libevent.cc:232:21
    #21 0x38c0c1c in base::RunLoop::Run() base/run_loop.cc:55:3
    #22 0x388c843 in base::MessageLoop::Run() base/message_loop/message_loop.cc:310:3
    #23 0x6af33c1 in content::BrowserThreadImpl::IOThreadRun(base::MessageLoop*) content/browser/browser_thread_impl.cc:218:3
    #24 0x6af375e in content::BrowserThreadImpl::Run(base::MessageLoop*) content/browser/browser_thread_impl.cc:253:14
    #25 0x390cec1 in base::Thread::ThreadMain() base/threading/thread.cc:228:5
    #26 0x3901e17 in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:80:3
    #27 0x7f99b0016e99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308

Original issue's description:
> Remove SSLClientAuthHandler's RDH dependency.
> 
> Instead, give it a callback to select the client certificate. This removes a
> dependency on GlobalRequestID. Also stop reference-counting it and avoid
> passing an HttpNetworkSession to the UI thread. (Use BrowserContext instead.)
> 
> BUG=376003
> 
> Committed: https://crrev.com/99aee5eb0f6733566c60dfe356a1b7169477808f
> Cr-Commit-Position: refs/heads/master@{#298782}

TBR=mmenke@chromium.org,abarth@chromium.org,jam@chromium.org,benm@chromium.org,davidben@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=376003

[robliao, 2014-11-26 17:51:39]

Created Revert of Remove SSLClientAuthHandler's RDH dependency.

[robliao, 2014-11-26 17:51:59]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2014-11-26 17:52:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/766463002/1

[commit-bot: I haz the power, 2014-11-26 17:53:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-26 17:53:31]

Failed to apply patch for content/browser/ssl/ssl_client_auth_handler.cc:
While running git apply --index -3 -p1;
  error: patch failed: content/browser/ssl/ssl_client_auth_handler.cc:5
  Falling back to three-way merge...
  Applied patch to 'content/browser/ssl/ssl_client_auth_handler.cc' with
conflicts.
  U content/browser/ssl/ssl_client_auth_handler.cc

Patch:       content/browser/ssl/ssl_client_auth_handler.cc
Index: content/browser/ssl/ssl_client_auth_handler.cc
diff --git a/content/browser/ssl/ssl_client_auth_handler.cc
b/content/browser/ssl/ssl_client_auth_handler.cc
index
8cab2c75229abdf23bd560e5e32a14dd5340fd6c..b7fe9f0a7fa4857c90074bcd7a04b07f69da8261
100644
--- a/content/browser/ssl/ssl_client_auth_handler.cc
+++ b/content/browser/ssl/ssl_client_auth_handler.cc
@@ -5,84 +5,77 @@
 #include "content/browser/ssl/ssl_client_auth_handler.h"
 
 #include "base/bind.h"
-#include "base/logging.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "net/cert/x509_certificate.h"
+#include "net/http/http_transaction_factory.h"
 #include "net/ssl/client_cert_store.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_context.h"
 
 namespace content {
-
-namespace {
-
-typedef base::Callback<void(net::X509Certificate*)> CertificateCallback;
-
-void CertificateSelectedOnUIThread(
-    const CertificateCallback& io_thread_callback,
-    net::X509Certificate* cert) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(io_thread_callback, make_scoped_refptr(cert)));
-}
-
-void SelectCertificateOnUIThread(
-    int render_process_host_id,
-    int render_frame_host_id,
-    net::SSLCertRequestInfo* cert_request_info,
-    const CertificateCallback& io_thread_callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  GetContentClient()->browser()->SelectClientCertificate(
-      render_process_host_id, render_frame_host_id, cert_request_info,
-      base::Bind(&CertificateSelectedOnUIThread, io_thread_callback));
-}
-
-}  // namespace
 
 SSLClientAuthHandler::SSLClientAuthHandler(
     scoped_ptr<net::ClientCertStore> client_cert_store,
     net::URLRequest* request,
-    net::SSLCertRequestInfo* cert_request_info,
-    const SSLClientAuthHandler::CertificateCallback& callback)
+    net::SSLCertRequestInfo* cert_request_info)
     : request_(request),
+      http_network_session_(
+          request_->context()->http_transaction_factory()->GetSession()),
       cert_request_info_(cert_request_info),
-      client_cert_store_(client_cert_store.Pass()),
-      callback_(callback),
-      weak_factory_(this) {
+      client_cert_store_(client_cert_store.Pass()) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 }
 
 SSLClientAuthHandler::~SSLClientAuthHandler() {
+  // If we were simply dropped, then act as if we selected no certificate.
+  DoCertificateSelected(NULL);
+}
+
+void SSLClientAuthHandler::OnRequestCancelled() {
+  request_ = NULL;
 }
 
 void SSLClientAuthHandler::SelectCertificate() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(request_);
 
   if (client_cert_store_) {
     client_cert_store_->GetClientCerts(
         *cert_request_info_,
         &cert_request_info_->client_certs,
-        base::Bind(&SSLClientAuthHandler::DidGetClientCerts,
-                   weak_factory_.GetWeakPtr()));
+        base::Bind(&SSLClientAuthHandler::DidGetClientCerts, this));
   } else {
     DidGetClientCerts();
   }
 }
 
+void SSLClientAuthHandler::CertificateSelected(net::X509Certificate* cert) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  VLOG(1) << this << " CertificateSelected " << cert;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &SSLClientAuthHandler::DoCertificateSelected, this,
+          make_scoped_refptr(cert)));
+}
+
 void SSLClientAuthHandler::DidGetClientCerts() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // Request may have cancelled while we were getting client certs.
+  if (!request_)
+    return;
 
   // Note that if |client_cert_store_| is NULL, we intentionally fall through
to
   // DoCertificateSelected. This is for platforms where the client cert
matching
-  // is not performed by Chrome. Those platforms handle the cert matching
before
-  // showing the dialog.
+  // is not performed by Chrome, the platform can handle the cert matching
+  // before showing the dialog.
   if (client_cert_store_ && cert_request_info_->client_certs.empty()) {
     // No need to query the user if there are no certs to choose from.
-    CertificateSelected(NULL);
+    DoCertificateSelected(NULL);
     return;
   }
 
@@ -90,27 +83,43 @@
   int render_frame_host_id;
   if (!ResourceRequestInfo::ForRequest(request_)->GetAssociatedRenderFrame(
           &render_process_host_id,
-          &render_frame_host_id)) {
+          &render_frame_host_id))
     NOTREACHED();
-    CertificateSelected(NULL);
-    return;
-  }
 
+  // If the RVH does not exist by the time this task gets run, then the task
+  // will be dropped and the scoped_refptr to SSLClientAuthHandler will go
+  // away, so we do not leak anything. The destructor takes care of ensuring
+  // the net::URLRequest always gets a response.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&SelectCertificateOnUIThread,
-                 render_process_host_id, render_frame_host_id,
-                 cert_request_info_,
-                 base::Bind(&SSLClientAuthHandler::CertificateSelected,
-                            weak_factory_.GetWeakPtr())));
+      base::Bind(
+          &SSLClientAuthHandler::DoSelectCertificate, this,
+          render_process_host_id, render_frame_host_id));
 }
 
-void SSLClientAuthHandler::CertificateSelected(net::X509Certificate* cert) {
+void SSLClientAuthHandler::DoCertificateSelected(net::X509Certificate* cert) {
   VLOG(1) << this << " DoCertificateSelected " << cert;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // request_ could have been NULLed if the request was cancelled while the
+  // user was choosing a cert, or because we have already responded to the
+  // certificate.
+  if (request_) {
+    request_->ContinueWithCertificate(cert);
 
-  callback_.Run(cert);
-  // |this| may be deleted at this point.
+    ResourceDispatcherHostImpl::Get()->
+        ClearSSLClientAuthHandlerForRequest(request_);
+    request_ = NULL;
+  }
+}
+
+void SSLClientAuthHandler::DoSelectCertificate(
+    int render_process_host_id, int render_frame_host_id) {
+  GetContentClient()->browser()->SelectClientCertificate(
+      render_process_host_id,
+      render_frame_host_id,
+      http_network_session_,
+      cert_request_info_.get(),
+      base::Bind(&SSLClientAuthHandler::CertificateSelected, this));
 }
 
 }  // namespace content

[commit-bot: I haz the power, 2014-11-26 17:53:47]

The CQ bit was unchecked by commit-bot@chromium.org

[mmenke, 2014-11-26 17:55:01]

Are you sure about this revert?  That CL was landed over a month ago.

[robliao, 2014-11-26 17:57:34]

On 2014/11/26 17:55:01, mmenke wrote:
> Are you sure about this revert?  That CL was landed over a month ago.

My mistake, I needed to apply a revert of this revert.
https://codereview.chromium.org/755933002

[robliao, 2014-11-26 17:58:33]

On 2014/11/26 17:57:34, robliao wrote:
> On 2014/11/26 17:55:01, mmenke wrote:
> > Are you sure about this revert?  That CL was landed over a month ago.
> 
> My mistake, I needed to apply a revert of this revert.
> https://codereview.chromium.org/755933002

Closing.