Refactor client cert private key handling.

chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc

Index: chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc
diff --git a/chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc b/chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc
index 5549a3d257b5c0438656d6747b2065357054bd12..161ab2213e7e2f501b0687c42a9e463a173ca1d6 100644
--- a/chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc
+++ b/chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc
@@ -8,23 +8,59 @@
 #include <utility>
 
 #include "base/callback_helpers.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/grit/generated_resources.h"
+#include "net/ssl/client_cert_identity.h"
+#include "net/ssl/ssl_private_key.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/font.h"
 #include "ui/views/controls/styled_label.h"
 
 namespace chromeos {
 
+namespace {
+
+// Fake ClientCertIdentity that does not support retrieving the private key.
+// platformKeys API currently only deals in certificates, not identities.

[Peter Kasting, 2017/06/15 22:13:41]

Nit: Leading "The"

[mattm, 2017/06/16 03:30:42]

Done.

+// Looking up the private key by the certificate is done as a separate step.
+class ClientCertIdentityPlatformKeys : public net::ClientCertIdentity {
+ public:
+  explicit ClientCertIdentityPlatformKeys(
+      scoped_refptr<net::X509Certificate> cert)
+      : net::ClientCertIdentity(std::move(cert)) {}
+  ~ClientCertIdentityPlatformKeys() override = default;
+
+  void AcquirePrivateKey(
+      const base::Callback<void(scoped_refptr<net::SSLPrivateKey>)>&
+          private_key_callback) override {
+    NOTREACHED();
+    private_key_callback.Run(nullptr);

[Peter Kasting, 2017/06/15 22:13:41]

It doesn't make sense to me to execute any code after a NOTREACHED().  This is
basically like handling DCHECK failure, which the styleguide bans.  If it's
NOTREACHED(), it's truly never reached.

[mattm, 2017/06/16 03:30:42]

Done.

+  }
+};
+
+net::ClientCertIdentityList CertificateListToIdentityList(
+    const net::CertificateList& certs) {
+  net::ClientCertIdentityList identities;
+  for (const auto& cert : certs) {
+    identities.push_back(
+        base::MakeUnique<ClientCertIdentityPlatformKeys>(cert));
+  }
+  return identities;
+}
+
+}  // namespace
+
 PlatformKeysCertificateSelector::PlatformKeysCertificateSelector(
     const net::CertificateList& certificates,
     const std::string& extension_name,
     const CertificateSelectedCallback& callback,
     content::WebContents* web_contents)
-    : CertificateSelector(certificates, web_contents),
+    : CertificateSelector(CertificateListToIdentityList(certificates),
+                          web_contents),
       extension_name_(extension_name),
       callback_(callback) {
   DCHECK(!callback_.is_null());
@@ -63,10 +99,11 @@ bool PlatformKeysCertificateSelector::Cancel() {
 
 bool PlatformKeysCertificateSelector::Accept() {
   DCHECK(!callback_.is_null());
-  scoped_refptr<net::X509Certificate> cert = GetSelectedCert();
-  if (!cert)
+  net::ClientCertIdentity* identity = GetSelectedCert();
+  if (!identity)
     return false;
-  base::ResetAndReturn(&callback_).Run(cert);
+  base::ResetAndReturn(&callback_)
+      .Run(make_scoped_refptr(identity->certificate()));
   return true;
 }