Refactor SSLClientCertificateSelector for reuse with platformKeys API.

chrome/browser/ui/views/certificate_selector_browsertest.cc

Index: chrome/browser/ui/views/certificate_selector_browsertest.cc
diff --git a/chrome/browser/ui/views/certificate_selector_browsertest.cc b/chrome/browser/ui/views/certificate_selector_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..94cdff2c6ff6e705ff08434c86fa51299efbbe0a
--- /dev/null
+++ b/chrome/browser/ui/views/certificate_selector_browsertest.cc
@@ -0,0 +1,64 @@
+// 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.
+
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/views/certificate_selector.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/base/test_data_directory.h"
+#include "net/cert/x509_certificate.h"
+#include "net/test/cert_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class CertificateSelectorTest : public InProcessBrowserTest {
+ public:
+  void SetUpInProcessBrowserTestFixture() override {
+    client_1_ =
+        net::ImportCertFromFile(net::GetTestCertsDirectory(), "client_1.pem");
+    ASSERT_TRUE(client_1_);

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

nit: ASSERT_NE to nullptr; ditto below.

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

Done.

+
+    client_2_ =
+        net::ImportCertFromFile(net::GetTestCertsDirectory(), "client_2.pem");
+    ASSERT_TRUE(client_2_);
+  }
+
+  void SetUpOnMainThread() override {
+    ASSERT_TRUE(content::WaitForLoadStop(
+        browser()->tab_strip_model()->GetActiveWebContents()));
+
+    net::CertificateList certificates;
+    certificates.push_back(client_1_);
+    certificates.push_back(client_2_);
+
+    selector_ = new chrome::CertificateSelector(
+        certificates, browser()->tab_strip_model()->GetActiveWebContents());
+    selector_->Init(base::ASCIIToUTF16("some arbitrary text"));
+  }
+
+ protected:
+  scoped_refptr<net::X509Certificate> client_1_;
+  scoped_refptr<net::X509Certificate> client_2_;
+
+  // The selector will be deleted when the browser is shutdown.

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

Perhaps explicitly close the |selector_| dialog in TearDownOnMainThread?

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

Done.

+  chrome::CertificateSelector* selector_ = nullptr;

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

Use the initializer list; don't assign here.

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

As I understand the style-guide this is the preferred way as there's no need for
an explicitly defined constructor in this case.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Initialization
"Decision: Use in-class member initialization for simple initializations [...]"

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

Interesting; I suppose the style has changed since I last looked.
Odd that this guide seems different from the internal guide link that I have:
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head

+};
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(CertificateSelectorTest, GetSelectedCert) {
+  EXPECT_EQ(client_1_.get(), selector_->GetSelectedCert());
+  EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_DOWN, false,
+                                              false, false, false));
+  EXPECT_EQ(client_2_.get(), selector_->GetSelectedCert());
+  EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_UP, false,
+                                              false, false, false));
+  EXPECT_EQ(client_1_.get(), selector_->GetSelectedCert());
+}