Use a scoped class for managing test root certs in unit tests

BUG=none
TEST=none
R=rtenneti@chromium.org


Review URL: http://codereview.chromium.org/9605026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@125363 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 53 insertions, 27 deletions

diff --git a/net/base/test_root_certs.cc b/net/base/test_root_certs.cc
index 1742e71..5b742e1 100644
--- a/net/base/test_root_certs.cc
+++ b/net/base/test_root_certs.cc
@@ -55,4 +55,22 @@ TestRootCerts::TestRootCerts() {
   g_has_instance = true;
 }
 
+ScopedTestRoot::ScopedTestRoot() {}
+
+ScopedTestRoot::ScopedTestRoot(X509Certificate* cert) {
+  Reset(cert);
+}
+
+ScopedTestRoot::~ScopedTestRoot() {
+  Reset(NULL);
+}
+
+void ScopedTestRoot::Reset(X509Certificate* cert) {
+  if (cert_)
+    TestRootCerts::GetInstance()->Clear();
+  if (cert)
+    TestRootCerts::GetInstance()->Add(cert);
+  cert_ = cert;
+}
+
 }  // namespace net
diff --git a/net/base/test_root_certs.h b/net/base/test_root_certs.h
index e2812a5..3011fe3 100644
--- a/net/base/test_root_certs.h
+++ b/net/base/test_root_certs.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -7,6 +7,7 @@
 #pragma once
 
 #include "base/lazy_instance.h"
+#include "base/memory/ref_counted.h"
 #include "build/build_config.h"
 #include "net/base/net_export.h"
 
@@ -99,6 +100,27 @@ class NET_EXPORT_PRIVATE TestRootCerts {
   DISALLOW_COPY_AND_ASSIGN(TestRootCerts);
 };
 
+// Scoped helper for unittests to handle safely managing trusted roots.
+class ScopedTestRoot {
+ public:
+  ScopedTestRoot();
+  // Creates a ScopedTestRoot that will adds|cert| to the TestRootCerts store.
+  explicit ScopedTestRoot(X509Certificate* cert);
+  ~ScopedTestRoot();
+
+  // Assigns |cert| to be the new test root cert. If |cert| is NULL, undoes
+  // any work the ScopedTestRoot may have previously done.
+  // If |cert_| contains a certificate (due to a prior call to Reset or due to
+  // a cert being passed at construction), the existing TestRootCerts store is
+  // cleared.
+  void Reset(X509Certificate* cert);
+
+ private:
+  scoped_refptr<X509Certificate> cert_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedTestRoot);
+};
+
 }  // namespace net
 
 #endif  // NET_BASE_TEST_ROOT_CERTS_H_
diff --git a/net/base/transport_security_state_unittest.cc b/net/base/transport_security_state_unittest.cc
index eb0b1a5..3cb0f9a 100644
--- a/net/base/transport_security_state_unittest.cc
+++ b/net/base/transport_security_state_unittest.cc
@@ -281,7 +281,7 @@ TEST_F(TransportSecurityStateTest, ValidPinsHeaders) {
   scoped_refptr<X509Certificate> root_cert =
       ImportCertFromFile(certs_dir, "2048-rsa-root.pem");
   ASSERT_NE(static_cast<X509Certificate*>(NULL), root_cert);
-  TestRootCerts::GetInstance()->Add(root_cert.get());
+  ScopedTestRoot scoped_root(root_cert);
 
   // Verify has the side-effect of populating public_key_hashes, which
   // ParsePinsHeader needs. (It wants to check pins against the validated
@@ -347,8 +347,6 @@ TEST_F(TransportSecurityStateTest, ValidPinsHeaders) {
           backup_pin + ";" + good_pin + ";   ",
       ssl_info, &state));
   EXPECT_EQ(state.max_age, TransportSecurityState::kMaxHSTSAgeSecs);
-
-  TestRootCerts::GetInstance()->Clear();
 }
 
 TEST_F(TransportSecurityStateTest, SimpleMatches) {
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index 25ddafa..7e25cbb 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -570,9 +570,9 @@ TEST(X509CertificateTest, IntermediateCARequireExplicitPolicy) {
       ImportCertFromFile(certs_dir, "dod_ca_17_cert.der");
   ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate_cert);
 
-  FilePath root_cert_path = certs_dir.AppendASCII("dod_root_ca_2_cert.der");
-  TestRootCerts* root_certs = TestRootCerts::GetInstance();
-  ASSERT_TRUE(root_certs->AddFromFile(root_cert_path));
+  scoped_refptr<X509Certificate> root_cert =
+      ImportCertFromFile(certs_dir, "dod_root_ca_2_cert.der");
+  ScopedTestRoot scoped_root(root_cert);
 
   X509Certificate::OSCertHandles intermediates;
   intermediates.push_back(intermediate_cert->os_cert_handle());
@@ -590,7 +590,6 @@ TEST(X509CertificateTest, IntermediateCARequireExplicitPolicy) {
     EXPECT_EQ(ERR_CERT_DATE_INVALID, error);
     EXPECT_EQ(CERT_STATUS_DATE_INVALID, verify_result.cert_status);
   }
-  root_certs->Clear();
 }
 
 // Test for bug 58437.
@@ -673,7 +672,7 @@ TEST(X509CertificateTest, RejectWeakKeys) {
   scoped_refptr<X509Certificate> root_cert =
       ImportCertFromFile(certs_dir, "2048-rsa-root.pem");
   ASSERT_NE(static_cast<X509Certificate*>(NULL), root_cert);
-  TestRootCerts::GetInstance()->Add(root_cert.get());
+  ScopedTestRoot scoped_root(root_cert);
 
   // Now test each chain.
   for (Strings::const_iterator ee_type = key_types.begin();
@@ -711,8 +710,6 @@ TEST(X509CertificateTest, RejectWeakKeys) {
       }
     }
   }
-
-  TestRootCerts::GetInstance()->Clear();
 }
 
 // Test for bug 108514.
@@ -907,7 +904,7 @@ TEST(X509CertificateTest, DISABLED_PublicKeyHashes) {
       ImportCertFromFile(certs_dir, "nist_intermediate.der");
   ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate_cert);
 
-  TestRootCerts::GetInstance()->Add(intermediate_cert.get());
+  ScopedTestRoot scoped_intermediate(intermediate_cert);
 
   X509Certificate::OSCertHandles intermediates;
   intermediates.push_back(intermediate_cert->os_cert_handle());
@@ -926,8 +923,6 @@ TEST(X509CertificateTest, DISABLED_PublicKeyHashes) {
       HexEncode(verify_result.public_key_hashes[0].data, base::kSHA1Length));
   EXPECT_EQ("83244223D6CBF0A26FC7DE27CEBCA4BDA32612AD",
       HexEncode(verify_result.public_key_hashes[1].data, base::kSHA1Length));
-
-  TestRootCerts::GetInstance()->Clear();
 }
 
 // A regression test for http://crbug.com/70293.
@@ -1140,7 +1135,7 @@ TEST(X509CertificateTest, VerifyReturnChainBasic) {
   intermediates.push_back(certs[1]->os_cert_handle());
   intermediates.push_back(certs[2]->os_cert_handle());
 
-  TestRootCerts::GetInstance()->Add(certs[2]);
+  ScopedTestRoot scoped_root(certs[2]);
 
   scoped_refptr<X509Certificate> google_full_chain =
       X509Certificate::CreateFromHandle(certs[0]->os_cert_handle(),
@@ -1165,8 +1160,6 @@ TEST(X509CertificateTest, VerifyReturnChainBasic) {
                                             certs[1]->os_cert_handle()));
   EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[1],
                                             certs[2]->os_cert_handle()));
-
-  TestRootCerts::GetInstance()->Clear();
 }
 
 // Test that the certificate returned in CertVerifyResult is able to reorder
@@ -1186,7 +1179,7 @@ TEST(X509CertificateTest, VerifyReturnChainProperlyOrdered) {
   intermediates.push_back(certs[2]->os_cert_handle());
   intermediates.push_back(certs[1]->os_cert_handle());
 
-  TestRootCerts::GetInstance()->Add(certs[2]);
+  ScopedTestRoot scoped_root(certs[2]);
 
   scoped_refptr<X509Certificate> google_full_chain =
       X509Certificate::CreateFromHandle(certs[0]->os_cert_handle(),
@@ -1211,8 +1204,6 @@ TEST(X509CertificateTest, VerifyReturnChainProperlyOrdered) {
                                             certs[1]->os_cert_handle()));
   EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[1],
                                             certs[2]->os_cert_handle()));
-
-  TestRootCerts::GetInstance()->Clear();
 }
 
 // Test that Verify() filters out certificates which are not related to
@@ -1223,7 +1214,7 @@ TEST(X509CertificateTest, VerifyReturnChainFiltersUnrelatedCerts) {
       certs_dir, "x509_verify_results.chain.pem",
       X509Certificate::FORMAT_AUTO);
   ASSERT_EQ(3U, certs.size());
-  TestRootCerts::GetInstance()->Add(certs[2]);
+  ScopedTestRoot scoped_root(certs[2]);
 
   scoped_refptr<X509Certificate> unrelated_dod_certificate =
       ImportCertFromFile(certs_dir, "dod_ca_17_cert.der");
@@ -1262,7 +1253,6 @@ TEST(X509CertificateTest, VerifyReturnChainFiltersUnrelatedCerts) {
                                             certs[1]->os_cert_handle()));
   EXPECT_TRUE(X509Certificate::IsSameOSCert(return_intermediates[1],
                                             certs[2]->os_cert_handle()));
-  TestRootCerts::GetInstance()->Clear();
 }
 
 #if defined(OS_MACOSX)
@@ -1826,21 +1816,19 @@ class X509CertificateWeakDigestTest
     : public testing::TestWithParam<WeakDigestTestData> {
  public:
   X509CertificateWeakDigestTest() {}
-
-  virtual void TearDown() {
-    TestRootCerts::GetInstance()->Clear();
-  }
+  virtual ~X509CertificateWeakDigestTest() {}
 };
 
 TEST_P(X509CertificateWeakDigestTest, Verify) {
   WeakDigestTestData data = GetParam();
   FilePath certs_dir = GetTestCertsDirectory();
 
+  ScopedTestRoot test_root;
   if (data.root_cert_filename) {
      scoped_refptr<X509Certificate> root_cert =
          ImportCertFromFile(certs_dir, data.root_cert_filename);
      ASSERT_NE(static_cast<X509Certificate*>(NULL), root_cert);
-     TestRootCerts::GetInstance()->Add(root_cert.get());
+     test_root.Reset(root_cert);
   }
 
   scoped_refptr<X509Certificate> intermediate_cert =