Refactor SSLClientCertificateSelector for reuse with platformKeys API.

chrome/browser/ui/views/certificate_selector.h

Index: chrome/browser/ui/views/certificate_selector.h
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector.h b/chrome/browser/ui/views/certificate_selector.h
similarity index 44%
copy from chrome/browser/ui/views/ssl_client_certificate_selector.h
copy to chrome/browser/ui/views/certificate_selector.h
index de4fbed4bffbf4aa25163267776bff05652bbe0c..7098b80a1f1885a2b9f2a190d2249abf868fa67e 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector.h
+++ b/chrome/browser/ui/views/certificate_selector.h
@@ -1,55 +1,49 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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_SSL_CLIENT_CERTIFICATE_SELECTOR_H_
-#define CHROME_BROWSER_UI_VIEWS_SSL_CLIENT_CERTIFICATE_SELECTOR_H_
+#ifndef CHROME_BROWSER_UI_VIEWS_CERTIFICATE_SELECTOR_H_
+#define CHROME_BROWSER_UI_VIEWS_CERTIFICATE_SELECTOR_H_
 
-#include <string>
 #include <vector>
 
-#include "base/basictypes.h"
+#include "base/macros.h"
 #include "base/strings/string16.h"
-#include "chrome/browser/ssl/ssl_client_auth_observer.h"
-#include "chrome/browser/ssl/ssl_client_certificate_selector.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/table/table_view_observer.h"
 #include "ui/views/window/dialog_delegate.h"
 
-// This header file exists only for testing.  Chrome should access the
-// certificate selector only through the cross-platform interface
-// chrome/browser/ssl_client_certificate_selector.h.
+namespace content {
+class WebContents;
+}
 
 namespace net {
-class SSLCertRequestInfo;
 class X509Certificate;
+typedef std::vector<scoped_refptr<X509Certificate>> CertificateList;

[davidben, 2015/02/18 20:32:01]

:-(

It looks like everything else duplicates the typedef though, so okay.

[pneubeck (no reviews), 2015/02/19 15:12:24]

Acknowledged.

 }
 
 namespace views {
 class LabelButton;
 class TableView;
-class Widget;
 }
 
-class CertificateSelectorTableModel;
+namespace chrome {
 
-class SSLClientCertificateSelector : public SSLClientAuthObserver,
-                                     public views::DialogDelegateView,
-                                     public views::ButtonListener,
-                                     public views::TableViewObserver {
+class CertificateSelector : public views::DialogDelegateView,
+                            public views::ButtonListener,
+                            public views::TableViewObserver {
  public:
-  SSLClientCertificateSelector(
-      content::WebContents* web_contents,
-      const scoped_refptr<net::SSLCertRequestInfo>& cert_request_info,
-      const chrome::SelectCertificateCallback& callback);
-  ~SSLClientCertificateSelector() override;
+  class CertificateTableModel;
 
-  void Init();
+  using SelectCertificateCallback = base::Callback<void(
+      const scoped_refptr<net::X509Certificate>& selected_cert)>;

[davidben, 2015/02/18 20:32:01]

Note: this'll conflict with this CL. I expect yours will land first, so I can
rebase mine. Probably we'd want to swap out the callback with that delegate
interface or something similar. Would that work for platformKeys?

https://codereview.chromium.org/859213006/

[pneubeck (no reviews), 2015/02/19 15:12:24]

Thinking more about it, I have the impression that for platformKeys it doesn't
really matter.
For HTTPS, IIUC, the reasoning is that we persist the choice of 'selecting no
certificate'. (We should actually make that explicit in the dialog instead of
hiding that in a Cancel button)

platformKeys itself doesn't persist the selection. It's the decision of the
extension using the API whether and what it persists. So this differentiation
doesn't really have any meaning for the API.

[davidben, 2015/02/19 20:54:20]

(Agreed. That UI is terrible. Though we'll still need the separate plumbing
outside of there.)
Alright. Well, I'll route them into the same one if it ends up applying. But it
looks this new separation between the base class makes it no longer matter.

 
-  net::X509Certificate* GetSelectedCert() const;
+  CertificateSelector(content::WebContents* web_contents,
+                      const net::CertificateList& certificates,
+                      const SelectCertificateCallback& callback);
+  ~CertificateSelector() override;
 
-  // SSLClientAuthObserver implementation:
-  void OnCertSelectedByNotification() override;
+  void Init(const base::string16& text);
 
   // DialogDelegateView:
   bool CanResize() const override;
@@ -69,20 +63,24 @@ class SSLClientCertificateSelector : public SSLClientAuthObserver,
   void OnSelectionChanged() override;
   void OnDoubleClick() override;
 
+ protected:
+  net::X509Certificate* GetSelectedCert() const;
+
  private:
   void CreateCertTable();
 
-  // Callback after unlocking certificate slot.
-  void Unlocked(net::X509Certificate* cert);
-
-  scoped_ptr<CertificateSelectorTableModel> model_;
+  const net::CertificateList certificates_;
+  const SelectCertificateCallback callback_;
+  scoped_ptr<CertificateTableModel> model_;
 
   content::WebContents* web_contents_;
 
   views::TableView* table_;
   views::LabelButton* view_cert_button_;
 
-  DISALLOW_COPY_AND_ASSIGN(SSLClientCertificateSelector);
+  DISALLOW_COPY_AND_ASSIGN(CertificateSelector);
 };
 
-#endif  // CHROME_BROWSER_UI_VIEWS_SSL_CLIENT_CERTIFICATE_SELECTOR_H_
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_UI_VIEWS_CERTIFICATE_SELECTOR_H_