Refactor client cert private key handling.

chrome/browser/ui/views/certificate_selector.h

Index: chrome/browser/ui/views/certificate_selector.h
diff --git a/chrome/browser/ui/views/certificate_selector.h b/chrome/browser/ui/views/certificate_selector.h
index 5eb701a2287e5370db51f3151a33619944f490f3..d0b4467351fe42a7400d9eba11bb7d2006cec303 100644
--- a/chrome/browser/ui/views/certificate_selector.h
+++ b/chrome/browser/ui/views/certificate_selector.h
@@ -9,7 +9,7 @@
 
 #include "base/macros.h"
 #include "base/strings/string16.h"
-#include "net/cert/x509_certificate.h"
+#include "net/ssl/client_cert_identity.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/table/table_view_observer.h"
 #include "ui/views/window/dialog_delegate.h"
@@ -45,13 +45,17 @@ class CertificateSelector : public views::DialogDelegateView,
   static bool CanShow(content::WebContents* web_contents);
 
   // |web_contents| must not be null.
-  CertificateSelector(const net::CertificateList& certificates,
+  CertificateSelector(net::ClientCertIdentityList identities,
                       content::WebContents* web_contents);
   ~CertificateSelector() override;
 
   // Returns the currently selected certificate or null if none is selected.
   // Must be called after |InitWithText()|.
-  net::X509Certificate* GetSelectedCert() const;
+  net::ClientCertIdentity* GetSelectedCert() const;
+
+  // Take ownership of the currently selected certificate or null if none is

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

Nit: Take -> Takes (see note about imperative comments in
http://google.github.io/styleguide/cppguide.html#Function_Comments )

I don't know what it means to "take ownership of...null".  In general, this
comment sounds like this class takes ownership of something, but instead the
implementation says it's doing the opposite.  I'm not sure how best to clarify
this.  I can't think of a better function name, so maybe it's just the comment
that's confusing me?  Maybe "Like GetSelectedCert(), but passes ownership of the
selected cert to the caller."?

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

done
 
Changed up the interface a bit, wdyt?

+  // selected.  Must be called after |InitWithText()|.

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

Nit: No need for || on function names (the () is their signifier)

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

Acknowledged.

+  std::unique_ptr<net::ClientCertIdentity> TakeSelectedCert();
 
   // Shows this dialog as a constrained web modal dialog and focuses the first
   // certificate.
@@ -88,7 +92,7 @@ class CertificateSelector : public views::DialogDelegateView,
  private:
   class CertificateTableModel;
 
-  net::CertificateList certificates_;
+  net::ClientCertIdentityList identities_;
 
   // Whether to show the provider column in the table or not. Certificates
   // provided by the platform show the empty string as provider. That column is