Refactor client cert private key handling.

chrome/browser/ui/views/certificate_selector.h

 // Copyright 2015 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.
 
 #ifndef CHROME_BROWSER_UI_VIEWS_CERTIFICATE_SELECTOR_H_
 #define CHROME_BROWSER_UI_VIEWS_CERTIFICATE_SELECTOR_H_
 
 #include <memory>
 
 #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"
 
 namespace content {
 class WebContents;
 }
 
 namespace views {
 class LabelButton;
@@ skipping 15 matching lines @@
 class CertificateSelector : public views::DialogDelegateView,
                             public views::ButtonListener,
                             public views::TableViewObserver {
  public:
   // Indicates if the dialog can be successfully shown.
   // TODO(davidben): Remove this when the certificate selector prompt is moved
   // to the WebContentsDelegate. https://crbug.com/456255.
   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.
   // Must be called after |InitWithText()|.
   void Show();
 
   // DialogDelegateView:
   bool CanResize() const override;
   base::string16 GetWindowTitle() const override;
   bool IsDialogButtonEnabled(ui::DialogButton button) const override;
@@ skipping 16 matching lines @@
   // Initializes the dialog. |text| is shown above the list of certificates
   // and is supposed to explain to the user what the implication of the
   // certificate selection is.
   void InitWithText(std::unique_ptr<views::View> text_label);
 
   ui::TableModel* table_model_for_testing() const;
 
  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
   // shown only if there is at least one non-empty provider, i.e. non-platform
   // certificate.
   bool show_provider_column_ = false;
   std::unique_ptr<CertificateTableModel> model_;
 
   content::WebContents* const web_contents_;
 
   views::TableView* table_;
   views::LabelButton* view_cert_button_;
 
   DISALLOW_COPY_AND_ASSIGN(CertificateSelector);
 };
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_UI_VIEWS_CERTIFICATE_SELECTOR_H_