Remove client_certs from SSLCertRequestInfo.

The client_certs aren't actually part of the certificate request, rather the client_certs member was used to store the list of matching certificates on the client side and pass them through to the certificate selector.
Pass the list of certs through the relevant callbacks instead.

BUG=166642,394131

Review-Url: https://codereview.chromium.org/2838243002
Cr-Commit-Position: refs/heads/master@{#467901}

android_webview/browser/aw_content_browser_client.cc

@@ -396,6 +396,7 @@ void AwContentBrowserClient::AllowCertificateError(
 void AwContentBrowserClient::SelectClientCertificate(
     content::WebContents* web_contents,
     net::SSLCertRequestInfo* cert_request_info,
+    net::CertificateList client_certs,
     std::unique_ptr<content::ClientCertificateDelegate> delegate) {
   AwContentsClientBridgeBase* client =
       AwContentsClientBridgeBase::FromWebContents(web_contents);

android_webview/browser/aw_content_browser_client.h

@@ -90,6 +90,7 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
   void SelectClientCertificate(
       content::WebContents* web_contents,
       net::SSLCertRequestInfo* cert_request_info,
+      net::CertificateList client_certs,
       std::unique_ptr<content::ClientCertificateDelegate> delegate) override;
   bool CanCreateWindow(content::RenderFrameHost* opener,
                        const GURL& opener_url,

chrome/browser/chrome_content_browser_client.cc

@@ -2385,6 +2385,7 @@ void ChromeContentBrowserClient::AllowCertificateError(
 void ChromeContentBrowserClient::SelectClientCertificate(
     content::WebContents* web_contents,
     net::SSLCertRequestInfo* cert_request_info,
+    net::CertificateList client_certs,
     std::unique_ptr<content::ClientCertificateDelegate> delegate) {
   prerender::PrerenderContents* prerender_contents =
       prerender::PrerenderContents::FromWebContents(web_contents);
@@ -2412,12 +2413,10 @@ void ChromeContentBrowserClient::SelectClientCertificate(
       base::DictionaryValue* filter_dict =
           static_cast<base::DictionaryValue*>(filter.get());
 
-      const std::vector<scoped_refptr<net::X509Certificate> >&
-          all_client_certs = cert_request_info->client_certs;
-      for (size_t i = 0; i < all_client_certs.size(); ++i) {
-        if (CertMatchesFilter(*all_client_certs[i].get(), *filter_dict)) {
+      for (size_t i = 0; i < client_certs.size(); ++i) {
+        if (CertMatchesFilter(*client_certs[i].get(), *filter_dict)) {
           // Use the first certificate that is matched by the filter.
-          delegate->ContinueWithCertificate(all_client_certs[i].get());
+          delegate->ContinueWithCertificate(client_certs[i].get());
           return;
         }
       }
@@ -2427,6 +2426,7 @@ void ChromeContentBrowserClient::SelectClientCertificate(
   }
 
   chrome::ShowSSLClientCertificateSelector(web_contents, cert_request_info,
+                                           std::move(client_certs),
                                            std::move(delegate));
 }
 

chrome/browser/chrome_content_browser_client.h

@@ -197,6 +197,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   void SelectClientCertificate(
       content::WebContents* web_contents,
       net::SSLCertRequestInfo* cert_request_info,
+      net::CertificateList client_certs,
       std::unique_ptr<content::ClientCertificateDelegate> delegate) override;
   content::MediaObserver* GetMediaObserver() override;
   content::PlatformNotificationService* GetPlatformNotificationService()

chrome/browser/chromeos/net/client_cert_store_chromeos.cc

@@ -12,6 +12,7 @@
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/location.h"
+#include "base/task_runner_util.h"
 #include "base/threading/worker_pool.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider.h"
 #include "crypto/nss_crypto_module_delegate.h"
@@ -47,14 +48,13 @@ ClientCertStoreChromeOS::~ClientCertStoreChromeOS() {}
 
 void ClientCertStoreChromeOS::GetClientCerts(
     const net::SSLCertRequestInfo& cert_request_info,
-    net::CertificateList* selected_certs,
-    const base::Closure& callback) {
+    const ClientCertListCallback& callback) {
   // Caller is responsible for keeping the ClientCertStore alive until the
   // callback is run.
   base::Callback<void(const net::CertificateList&)>
-      get_platform_certs_and_filter = base::Bind(
-          &ClientCertStoreChromeOS::GotAdditionalCerts, base::Unretained(this),
-          &cert_request_info, selected_certs, callback);
+      get_platform_certs_and_filter =
+          base::Bind(&ClientCertStoreChromeOS::GotAdditionalCerts,
+                     base::Unretained(this), &cert_request_info, callback);
 
   base::Closure get_additional_certs_and_continue;
   if (cert_provider_) {
@@ -72,35 +72,32 @@ void ClientCertStoreChromeOS::GetClientCerts(
 
 void ClientCertStoreChromeOS::GotAdditionalCerts(
     const net::SSLCertRequestInfo* request,
-    net::CertificateList* selected_certs,
-    const base::Closure& callback,
+    const ClientCertListCallback& callback,
     const net::CertificateList& additional_certs) {
   std::unique_ptr<crypto::CryptoModuleBlockingPasswordDelegate>
       password_delegate;
   if (!password_delegate_factory_.is_null()) {
     password_delegate.reset(
         password_delegate_factory_.Run(request->host_and_port));
   }
-  if (base::WorkerPool::PostTaskAndReply(
+  if (base::PostTaskAndReplyWithResult(
+          base::WorkerPool::GetTaskRunner(true /* task_is_slow */).get(),
           FROM_HERE,
           base::Bind(&ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread,
                      base::Unretained(this), base::Passed(&password_delegate),
-                     request, additional_certs, selected_certs),
-          callback, true)) {
+                     request, additional_certs),
+          callback)) {
     return;
   }
-  // If the task could not be posted, behave as if there were no certificates
-  // which requires to clear |selected_certs|.
-  selected_certs->clear();
-  callback.Run();
+  // If the task could not be posted, behave as if there were no certificates.
+  callback.Run(net::CertificateList());
 }
 
-void ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread(
+net::CertificateList ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread(
     std::unique_ptr<crypto::CryptoModuleBlockingPasswordDelegate>
         password_delegate,
     const net::SSLCertRequestInfo* request,
-    const net::CertificateList& additional_certs,
-    net::CertificateList* selected_certs) {
+    const net::CertificateList& additional_certs) {
   net::CertificateList unfiltered_certs;
   net::ClientCertStoreNSS::GetPlatformCertsOnWorkerThread(
       std::move(password_delegate), &unfiltered_certs);
@@ -113,8 +110,10 @@ void ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread(
   unfiltered_certs.insert(unfiltered_certs.end(), additional_certs.begin(),
                           additional_certs.end());
 
+  net::CertificateList selected_certs;
   net::ClientCertStoreNSS::FilterCertsOnWorkerThread(unfiltered_certs, *request,
-                                                     selected_certs);
+                                                     &selected_certs);
+  return selected_certs;
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/net/client_cert_store_chromeos.h

@@ -51,21 +51,18 @@ class ClientCertStoreChromeOS : public net::ClientCertStore {
 
   // net::ClientCertStore:
   void GetClientCerts(const net::SSLCertRequestInfo& cert_request_info,
-                      net::CertificateList* selected_certs,
-                      const base::Closure& callback) override;
+                      const ClientCertListCallback& callback) override;
 
  private:
   void GotAdditionalCerts(const net::SSLCertRequestInfo* request,
-                          net::CertificateList* selected_certs,
-                          const base::Closure& callback,
+                          const ClientCertListCallback& callback,
                           const net::CertificateList& additional_certs);
 
-  void GetAndFilterCertsOnWorkerThread(
+  net::CertificateList GetAndFilterCertsOnWorkerThread(
       std::unique_ptr<crypto::CryptoModuleBlockingPasswordDelegate>
           password_delegate,
       const net::SSLCertRequestInfo* request,
-      const net::CertificateList& additional_certs,
-      net::CertificateList* selected_certs);
+      const net::CertificateList& additional_certs);
 
   std::unique_ptr<CertificateProvider> cert_provider_;
   std::unique_ptr<CertFilter> cert_filter_;

chrome/browser/chromeos/net/client_cert_store_chromeos_unittest.cc

@@ -73,6 +73,13 @@ class TestCertFilter : public ClientCertStoreChromeOS::CertFilter {
   scoped_refptr<net::X509Certificate> not_allowed_cert_;
 };
 
+void SaveCertsAndQuitCallback(net::CertificateList* out_certs,
+                              base::Closure quit_closure,
+                              net::CertificateList in_certs) {
+  *out_certs = std::move(in_certs);
+  quit_closure.Run();
+}
+
 }  // namespace
 
 class ClientCertStoreChromeOSTest : public ::testing::Test {
@@ -111,22 +118,24 @@ TEST_F(ClientCertStoreChromeOSTest, RequestWaitsForNSSInitAndSucceeds) {
   scoped_refptr<net::SSLCertRequestInfo> request_all(
       new net::SSLCertRequestInfo());
 
+  net::CertificateList selected_certs;
   base::RunLoop run_loop;
-  store.GetClientCerts(*request_all, &request_all->client_certs,
-                       run_loop.QuitClosure());
+  store.GetClientCerts(*request_all,
+                       base::Bind(SaveCertsAndQuitCallback, &selected_certs,
+                                  run_loop.QuitClosure()));
 
   {
     base::RunLoop run_loop_inner;
     run_loop_inner.RunUntilIdle();
     // GetClientCerts should wait for the initialization of the filter to
     // finish.
-    ASSERT_EQ(0u, request_all->client_certs.size());
+    ASSERT_EQ(0u, selected_certs.size());
     EXPECT_TRUE(cert_filter->init_called());
   }
   cert_filter->FinishInit();
   run_loop.Run();
 
-  ASSERT_EQ(1u, request_all->client_certs.size());
+  ASSERT_EQ(1u, selected_certs.size());
 }
 
 // Ensure that cert requests, that are started after the filter was initialized,
@@ -148,11 +157,13 @@ TEST_F(ClientCertStoreChromeOSTest, RequestsAfterNSSInitSucceed) {
       new net::SSLCertRequestInfo());
 
   base::RunLoop run_loop;
-  store.GetClientCerts(*request_all, &request_all->client_certs,
-                       run_loop.QuitClosure());
+  net::CertificateList selected_certs;
+  store.GetClientCerts(*request_all,
+                       base::Bind(SaveCertsAndQuitCallback, &selected_certs,
+                                  run_loop.QuitClosure()));
   run_loop.Run();
 
-  ASSERT_EQ(1u, request_all->client_certs.size());
+  ASSERT_EQ(1u, selected_certs.size());
 }
 
 TEST_F(ClientCertStoreChromeOSTest, Filter) {
@@ -179,7 +190,9 @@ TEST_F(ClientCertStoreChromeOSTest, Filter) {
     base::RunLoop run_loop;
     cert_filter->SetNotAllowedCert(cert_2);
     net::CertificateList selected_certs;
-    store.GetClientCerts(*request_all, &selected_certs, run_loop.QuitClosure());
+    store.GetClientCerts(*request_all,
+                         base::Bind(SaveCertsAndQuitCallback, &selected_certs,
+                                    run_loop.QuitClosure()));
     run_loop.Run();
 
     ASSERT_EQ(1u, selected_certs.size());
@@ -190,7 +203,9 @@ TEST_F(ClientCertStoreChromeOSTest, Filter) {
     base::RunLoop run_loop;
     cert_filter->SetNotAllowedCert(cert_1);
     net::CertificateList selected_certs;
-    store.GetClientCerts(*request_all, &selected_certs, run_loop.QuitClosure());
+    store.GetClientCerts(*request_all,
+                         base::Bind(SaveCertsAndQuitCallback, &selected_certs,
+                                    run_loop.QuitClosure()));
     run_loop.Run();
 
     ASSERT_EQ(1u, selected_certs.size());
@@ -226,7 +241,9 @@ TEST_F(ClientCertStoreChromeOSTest, CertRequestMatching) {
 
   base::RunLoop run_loop;
   net::CertificateList selected_certs;
-  store.GetClientCerts(*request, &selected_certs, run_loop.QuitClosure());
+  store.GetClientCerts(*request,
+                       base::Bind(SaveCertsAndQuitCallback, &selected_certs,
+                                  run_loop.QuitClosure()));
   run_loop.Run();
 
   ASSERT_EQ(1u, selected_certs.size());

chrome/browser/chromeos/platform_keys/platform_keys_nss.cc

@@ -243,7 +243,6 @@ class SelectCertificatesState : public NSSOperationState {
   const bool use_system_key_slot_;
   scoped_refptr<net::SSLCertRequestInfo> cert_request_info_;
   std::unique_ptr<net::ClientCertStore> cert_store_;
-  std::unique_ptr<net::CertificateList> certs_;
 
  private:
   // Must be called on origin thread, therefore use CallBack().
@@ -550,9 +549,11 @@ void SignRSAWithDB(std::unique_ptr<SignRSAState> state,
 // of net::CertificateList and calls back. Used by
 // SelectCertificatesOnIOThread().
 void DidSelectCertificatesOnIOThread(
-    std::unique_ptr<SelectCertificatesState> state) {
+    std::unique_ptr<SelectCertificatesState> state,
+    net::CertificateList certs) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  state->CallBack(FROM_HERE, std::move(state->certs_),
+  state->CallBack(FROM_HERE,
+                  base::MakeUnique<net::CertificateList>(std::move(certs)),
                   std::string() /* no error */);
 }
 
@@ -567,11 +568,9 @@ void SelectCertificatesOnIOThread(
                                                  state->username_hash_),
       ClientCertStoreChromeOS::PasswordDelegateFactory()));
 
-  state->certs_.reset(new net::CertificateList);
-
   SelectCertificatesState* state_ptr = state.get();
   state_ptr->cert_store_->GetClientCerts(
-      *state_ptr->cert_request_info_, state_ptr->certs_.get(),
+      *state_ptr->cert_request_info_,
       base::Bind(&DidSelectCertificatesOnIOThread, base::Passed(&state)));
 }
 

chrome/browser/prerender/prerender_browsertest.cc

@@ -2253,10 +2253,8 @@ class TestClientCertStore : public net::ClientCertStore {
 
   // net::ClientCertStore:
   void GetClientCerts(const net::SSLCertRequestInfo& cert_request_info,
-                      net::CertificateList* selected_certs,
-                      const base::Closure& callback) override {
-    *selected_certs = certs_;
-    callback.Run();
+                      const ClientCertListCallback& callback) override {
+    callback.Run(certs_);
   }
 
  private:

chrome/browser/ssl/ssl_client_certificate_selector.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/callback_forward.h"
+#include "net/cert/x509_certificate.h"
 
 namespace content {
 class ClientCertificateDelegate;
@@ -21,12 +22,14 @@ class SSLCertRequestInfo;
 namespace chrome {
 
 // Opens a constrained SSL client certificate selection dialog under |parent|,
-// offering certificates from |cert_request_info|. When the user has made a
-// selection, the dialog will report back to |delegate|. If the dialog is
-// closed with no selection, |delegate| will simply be destroyed.
+// offering certificates in |client_certs| for the host specified by
+// |cert_request_info|. When the user has made a selection, the dialog will
+// report back to |delegate|. If the dialog is closed with no selection,
+// |delegate| will simply be destroyed.
 void ShowSSLClientCertificateSelector(
     content::WebContents* contents,
     net::SSLCertRequestInfo* cert_request_info,
+    net::CertificateList client_certs,
     std::unique_ptr<content::ClientCertificateDelegate> delegate);
 
 }  // namespace chrome

chrome/browser/ssl/ssl_client_certificate_selector_test.cc

@@ -13,11 +13,8 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "net/base/request_priority.h"
-#include "net/cert/x509_certificate.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/ssl/ssl_cert_request_info.h"
-#include "net/test/cert_test_util.h"
-#include "net/test/test_data_directory.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
@@ -36,19 +33,8 @@ SSLClientCertificateSelectorTestBase::~SSLClientCertificateSelectorTestBase() {
 }
 
 void SSLClientCertificateSelectorTestBase::SetUpInProcessBrowserTestFixture() {
-  base::FilePath certs_dir = net::GetTestCertsDirectory();
-
-  mit_davidben_cert_ = net::ImportCertFromFile(certs_dir, "mit.davidben.der");
-  ASSERT_TRUE(mit_davidben_cert_.get());
-
-  foaf_me_chromium_test_cert_ = net::ImportCertFromFile(
-      certs_dir, "foaf.me.chromium-test-cert.der");
-  ASSERT_TRUE(foaf_me_chromium_test_cert_.get());
-
   cert_request_info_ = new net::SSLCertRequestInfo;
   cert_request_info_->host_and_port = net::HostPortPair("foo", 123);
-  cert_request_info_->client_certs.push_back(mit_davidben_cert_);
-  cert_request_info_->client_certs.push_back(foaf_me_chromium_test_cert_);
 }
 
 void SSLClientCertificateSelectorTestBase::SetUpOnMainThread() {

chrome/browser/ssl/ssl_client_certificate_selector_test.h

@@ -38,8 +38,6 @@ class SSLClientCertificateSelectorTestBase : public InProcessBrowserTest {
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   net::URLRequest* url_request_;
 
-  scoped_refptr<net::X509Certificate> mit_davidben_cert_;
-  scoped_refptr<net::X509Certificate> foaf_me_chromium_test_cert_;
   scoped_refptr<net::SSLCertRequestInfo> cert_request_info_;
   scoped_refptr<testing::StrictMock<SSLClientAuthRequestorMock> >
       auth_requestor_;

chrome/browser/ui/android/ssl_client_certificate_request.cc

@@ -204,6 +204,7 @@ bool RegisterSSLClientCertificateRequestAndroid(JNIEnv* env) {
 void ShowSSLClientCertificateSelector(
     content::WebContents* contents,
     net::SSLCertRequestInfo* cert_request_info,
+    net::CertificateList unused_client_certs,
     std::unique_ptr<content::ClientCertificateDelegate> delegate) {
   ui::WindowAndroid* window = ViewAndroidHelper::FromWebContents(contents)
       ->GetViewAndroid()->GetWindowAndroid();