Refactor SSLClientCertificateSelector for reuse with platformKeys API.

chrome/browser/ui/views/ssl_client_certificate_selector.h

Index: chrome/browser/ui/views/ssl_client_certificate_selector.h
diff --git a/chrome/browser/ui/views/ssl_client_certificate_selector.h b/chrome/browser/ui/views/ssl_client_certificate_selector.h
index de4fbed4bffbf4aa25163267776bff05652bbe0c..f06b805f932b0a054c120bcc2d63605ff93f7367 100644
--- a/chrome/browser/ui/views/ssl_client_certificate_selector.h
+++ b/chrome/browser/ui/views/ssl_client_certificate_selector.h
@@ -5,38 +5,26 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_SSL_CLIENT_CERTIFICATE_SELECTOR_H_
 #define CHROME_BROWSER_UI_VIEWS_SSL_CLIENT_CERTIFICATE_SELECTOR_H_
 
-#include <string>
-#include <vector>
-
-#include "base/basictypes.h"
-#include "base/strings/string16.h"
+#include "base/macros.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"
+#include "chrome/browser/ui/views/certificate_selector.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;
 }
 
-namespace views {
-class LabelButton;
-class TableView;
-class Widget;
-}
-
-class CertificateSelectorTableModel;
-
-class SSLClientCertificateSelector : public SSLClientAuthObserver,
-                                     public views::DialogDelegateView,
-                                     public views::ButtonListener,
-                                     public views::TableViewObserver {
+class SSLClientCertificateSelector : public chrome::CertificateSelector,
+                                     public SSLClientAuthObserver {
  public:
   SSLClientCertificateSelector(
       content::WebContents* web_contents,
@@ -46,28 +34,12 @@ class SSLClientCertificateSelector : public SSLClientAuthObserver,
 
   void Init();

[msw, 2015/02/19 19:53:53]

nit: avoid overloading the non-virtual base class method name.

[pneubeck (no reviews), 2015/02/19 20:43:50]

I can come up with any other random name (e.g. Initialize()), but Init() seems
to be the most typical?
Note that this also hides the base class declaration and cannot be called
without explicit name qualification.

I'm fine with either.

[msw, 2015/02/19 21:03:31]

Consider CertificateSelector::InitAndShow, since it also triggers the dialog to
actually be shown (which is rather odd for an Init function).

It'd be nice if Init was just rolled into the ctor (with a new text arg) and
chrome::ShowSSLClientCertificateSelector called
constrained_window::ShowWebModalDialogViews explicitly, but then I guess
something else would need to select the first row of the table to start, if that
actually needs to be done after the dialog is created/shown.

[pneubeck (no reviews), 2015/02/20 09:54:00]

I considered the splitting as well. It makes the Init() behave more as expected
but that doesn't help with the overriding.

Regarding putting the initialization into the c'tor, I think that is more
harmful than good in C++, also the style guide explicitly disallows that:

"Avoid doing complex initialization in constructors (in particular,
initialization that can fail or that requires virtual method calls)."

I renamed the Init function in the base class to InitWithText and made it
protected.

 
-  net::X509Certificate* GetSelectedCert() const;
-
-  // SSLClientAuthObserver implementation:
+  // SSLClientAuthObserver:
   void OnCertSelectedByNotification() override;
 
-  // DialogDelegateView:
-  bool CanResize() const override;
-  base::string16 GetWindowTitle() const override;
-  void DeleteDelegate() override;
-  bool IsDialogButtonEnabled(ui::DialogButton button) const override;
+  // chrome::CertificateSelector:
   bool Cancel() override;
   bool Accept() override;
-  views::View* GetInitiallyFocusedView() override;
-  views::View* CreateExtraView() override;
-  ui::ModalType GetModalType() const override;
-
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
-  // views::TableViewObserver:
-  void OnSelectionChanged() override;
-  void OnDoubleClick() override;
 
  private:
   void CreateCertTable();

[msw, 2015/02/19 19:53:54]

nit: avoid overloading the non-virtual base class method name.

[pneubeck (no reviews), 2015/02/19 20:43:50]

deleted.

@@ -75,13 +47,6 @@ class SSLClientCertificateSelector : public SSLClientAuthObserver,
   // Callback after unlocking certificate slot.
   void Unlocked(net::X509Certificate* cert);
 
-  scoped_ptr<CertificateSelectorTableModel> model_;
-
-  content::WebContents* web_contents_;
-
-  views::TableView* table_;
-  views::LabelButton* view_cert_button_;
-
   DISALLOW_COPY_AND_ASSIGN(SSLClientCertificateSelector);
 };