Reland: Implement ScopedTestNSSDB instead of OpenTestNSSDB()

This CL needs memory suppression because of missing CloseTestNSSDB().
See also TODO.

BUG=136950, 156433


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162327

Review URL: https://chromiumcodereview.appspot.com/11174006

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

chrome/browser/chromeos/cros/network_library_unittest.cc

@@ -138,20 +138,10 @@ class NetworkLibraryStubTest : public testing::Test {
  public:
   NetworkLibraryStubTest() : cros_(NULL) {}
 
-  static void SetUpTestCase() {
-    // Ideally, we'd open a test DB for each test case, and close it
-    // again, removing the temp dir, but unfortunately, there's a
-    // bug in NSS that prevents this from working, so we just open
-    // it once, and empty it for each test case.  Here's the bug:
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=588269
-    ASSERT_TRUE(crypto::OpenTestNSSDB());
-    // There is no matching TearDownTestCase call to close the test NSS DB
-    // because that would leave NSS in a potentially broken state for further
-    // tests, due to https://bugzilla.mozilla.org/show_bug.cgi?id=588269
-  }
-
  protected:
   virtual void SetUp() {
+    ASSERT_TRUE(test_nssdb_.is_open());
+
     slot_ = net::NSSCertDatabase::GetInstance()->GetPublicModule();
     cros_ = CrosLibrary::Get()->GetNetworkLibrary();
     ASSERT_TRUE(cros_) << "GetNetworkLibrary() Failed!";
@@ -207,6 +197,7 @@ class NetworkLibraryStubTest : public testing::Test {
   }
 
   scoped_refptr<net::CryptoModule> slot_;
+  crypto::ScopedTestNSSDB test_nssdb_;
 };
 
 // Default stub state:

chrome/browser/chromeos/cros/onc_network_parser_unittest.cc

@@ -54,19 +54,9 @@ net::CertType GetCertType(const net::X509Certificate* cert) {
 
 class OncNetworkParserTest : public testing::Test {
  public:
-  static void SetUpTestCase() {
-    // Ideally, we'd open a test DB for each test case, and close it
-    // again, removing the temp dir, but unfortunately, there's a
-    // bug in NSS that prevents this from working, so we just open
-    // it once, and empty it for each test case.  Here's the bug:
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=588269
-    ASSERT_TRUE(crypto::OpenTestNSSDB());
-    // There is no matching TearDownTestCase call to close the test NSS DB
-    // because that would leave NSS in a potentially broken state for further
-    // tests, due to https://bugzilla.mozilla.org/show_bug.cgi?id=588269
-  }
-
   virtual void SetUp() {
+    ASSERT_TRUE(test_nssdb_.is_open());
+
     slot_ = net::NSSCertDatabase::GetInstance()->GetPublicModule();
 
     // Don't run the test if the setup failed.
@@ -137,6 +127,7 @@ class OncNetworkParserTest : public testing::Test {
   }
 
   ScopedStubCrosEnabler stub_cros_enabler_;
+  crypto::ScopedTestNSSDB test_nssdb_;
 };
 
 const base::Value* OncNetworkParserTest::GetExpectedProperty(

crypto/crypto_export.h

@@ -5,25 +5,34 @@
 #ifndef CRYPTO_CRYPTO_EXPORT_H_
 #define CRYPTO_CRYPTO_EXPORT_H_
 
+// Defines CRYPTO_EXPORT so that functionality implemented by the crypto module
+// can be exported to consumers, and CRYPTO_EXPORT_PRIVATE that allows unit
+// tests to access features not intended to be used directly by real consumers.
+
 #if defined(COMPONENT_BUILD)
 #if defined(WIN32)
 
 #if defined(CRYPTO_IMPLEMENTATION)
 #define CRYPTO_EXPORT __declspec(dllexport)
+#define CRYPTO_EXPORT_PRIVATE __declspec(dllexport)
 #else
 #define CRYPTO_EXPORT __declspec(dllimport)
+#define CRYPTO_EXPORT_PRIVATE __declspec(dllimport)
 #endif  // defined(CRYPTO_IMPLEMENTATION)
 
 #else  // defined(WIN32)
 #if defined(CRYPTO_IMPLEMENTATION)
 #define CRYPTO_EXPORT __attribute__((visibility("default")))
+#define CRYPTO_EXPORT_PRIVATE __attribute__((visibility("default")))
 #else
 #define CRYPTO_EXPORT
+#define CRYPTO_EXPORT_PRIVATE
 #endif
 #endif
 
 #else  // defined(COMPONENT_BUILD)
 #define CRYPTO_EXPORT
+#define CRYPTO_EXPORT_PRIVATE
 #endif
 
 #endif  // CRYPTO_CRYPTO_EXPORT_H_

crypto/nss_util.cc

@@ -702,8 +702,14 @@ bool CheckNSSVersion(const char* version) {
 }
 
 #if defined(USE_NSS)
-bool OpenTestNSSDB() {
-  return g_nss_singleton.Get().OpenTestNSSDB();
+ScopedTestNSSDB::ScopedTestNSSDB()
+  : is_open_(g_nss_singleton.Get().OpenTestNSSDB()) {
+}
+
+ScopedTestNSSDB::~ScopedTestNSSDB() {
+  // TODO(mattm): Close the dababase once NSS 3.14 is required,
+  // which fixes https://bugzilla.mozilla.org/show_bug.cgi?id=588269
+  // Resource leaks are suppressed. http://crbug.com/156433 .
 }
 
 base::Lock* GetNSSWriteLock() {

crypto/nss_util.h

@@ -128,11 +128,21 @@ CRYPTO_EXPORT int64 BaseTimeToPRTime(base::Time time);
 
 #if defined(USE_NSS)
 // Exposed for unittests only.
-// TODO(mattm): when https://bugzilla.mozilla.org/show_bug.cgi?id=588269 is
-// fixed, switch back to using a separate userdb for each test.  (Maybe refactor
-// to provide a ScopedTestNSSDB instead of open/close methods.)
-CRYPTO_EXPORT bool OpenTestNSSDB();
-// NOTE: due to NSS bug 588269, mentioned above, there is no CloseTestNSSDB.
+// TODO(mattm): When NSS 3.14 is the minimum version required,
+// switch back to using a separate user DB for each test.
+// Because of https://bugzilla.mozilla.org/show_bug.cgi?id=588269 , the
+// opened user DB is not automatically closed.
+class CRYPTO_EXPORT_PRIVATE ScopedTestNSSDB {
+ public:
+  ScopedTestNSSDB();
+  ~ScopedTestNSSDB();
+
+  bool is_open() { return is_open_; }
+
+ private:
+  bool is_open_;
+  DISALLOW_COPY_AND_ASSIGN(ScopedTestNSSDB);
+};
 
 // NSS has a bug which can cause a deadlock or stall in some cases when writing
 // to the certDB and keyDB. It also has a bug which causes concurrent key pair

net/base/nss_cert_database_unittest.cc

@@ -38,20 +38,10 @@
 
 namespace net {
 
-// TODO(mattm): when https://bugzilla.mozilla.org/show_bug.cgi?id=588269 is
-// fixed, switch back to using a separate userdb for each test.
-// (When doing so, remember to add some standalone tests of DeleteCert since it
-// won't be tested by TearDown anymore.)
 class CertDatabaseNSSTest : public testing::Test {
  public:
-  static void SetUpTestCase() {
-    ASSERT_TRUE(crypto::OpenTestNSSDB());
-    // There is no matching TearDownTestCase call to close the test NSS DB
-    // because that would leave NSS in a potentially broken state for further
-    // tests, due to https://bugzilla.mozilla.org/show_bug.cgi?id=588269
-  }
-
   virtual void SetUp() {
+    ASSERT_TRUE(test_nssdb_.is_open());
     cert_db_ = NSSCertDatabase::GetInstance();
     slot_ = cert_db_->GetPublicModule();
 
@@ -129,6 +119,8 @@ class CertDatabaseNSSTest : public testing::Test {
     }
     return ok;
   }
+
+  crypto::ScopedTestNSSDB test_nssdb_;
 };
 
 TEST_F(CertDatabaseNSSTest, ListCerts) {

tools/heapcheck/suppressions.txt

@@ -247,6 +247,25 @@
    fun:WebKit::WebLayerTreeViewImpl::composite
    fun:ui::Compositor::Draw
 }
+# There is not CloseTestNSSDB due to NSS bug 588269.
+# When NSS 3.14 is the minimum version required, this should be removed.
+# http://crbug.com/156433 .
+{
+   bug_156433_a
+   Heapcheck:Leak
+   fun:sqlite3MemMalloc
+   fun:crypto::::NSSInitSingleton::OpenUserDB
+   fun:crypto::::NSSInitSingleton::OpenTestNSSDB
+   fun:crypto::ScopedTestNSSDB::ScopedTestNSSDB
+}
+{
+   bug_156433_b
+   Heapcheck:Leak
+   fun:PORT_Alloc_Util
+   fun:crypto::::NSSInitSingleton::OpenUserDB
+   fun:crypto::::NSSInitSingleton::OpenTestNSSDB
+   fun:crypto::ScopedTestNSSDB::ScopedTestNSSDB
+}
 
 
 #-----------------------------------------------------------------------