NSS cert database cleanups

NSS cert database cleanups

Use ScopedPK11Slot.
Move common test functionality into functions.
Move common test conditions into SetUp/TearDown.
Move common test variables into attributes of the test class.

BUG=19991
TEST=tests still pass

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

[mattm, 2010-08-19 22:21:01]

Just some cleanups in preparation for my next CL.  Split out to make the next CL
leaner.

[Paweł Hajdan Jr., 2010-08-19 22:25:23]

Drive-by with a test comment.

http://codereview.chromium.org/3189014/diff/6001/7001
File net/base/cert_database_nss_unittest.cc (right):

http://codereview.chromium.org/3189014/diff/6001/7001#newcode58
net/base/cert_database_nss_unittest.cc:58: LOG(ERROR) << "error reading cert "
<< cert_path.value() << " " << errno;
Why not EXPECT_TRUE(file_util::ReadFileToString(... ?

[mattm, 2010-08-19 22:49:42]

http://codereview.chromium.org/3189014/diff/6001/7001
File net/base/cert_database_nss_unittest.cc (right):

http://codereview.chromium.org/3189014/diff/6001/7001#newcode58
net/base/cert_database_nss_unittest.cc:58: LOG(ERROR) << "error reading cert "
<< cert_path.value() << " " << errno;
On 2010/08/19 22:25:24, Paweł Hajdan Jr. wrote:
> Why not EXPECT_TRUE(file_util::ReadFileToString(... ?

Oh yeah, it's only ASSERT_foo that doesn't work here.  done.

[wtc, 2010-08-19 22:59:19]

LGTM.  Nice cleanup!

http://codereview.chromium.org/3189014/diff/6001/7001
File net/base/cert_database_nss_unittest.cc (right):

http://codereview.chromium.org/3189014/diff/6001/7001#newcode45
net/base/cert_database_nss_unittest.cc:45: X509Certificate::CreateFromHandle(
Nit: indent this line by 4.

http://codereview.chromium.org/3189014/diff/6001/7001#newcode88
net/base/cert_database_nss_unittest.cc:88: std::string
cert_data(ReadTestCert("client.p12"));
Nit: I prefer using the = operator instead of the copy
constructor here (and at line 98) below, because it makes
the code easier to read.  What do you think?

Terminology issue: a PKCS #12 file also contains a private
key, so it's more than a cert.

[mattm, 2010-08-19 23:47:13]

http://codereview.chromium.org/3189014/diff/6001/7001
File net/base/cert_database_nss_unittest.cc (right):

http://codereview.chromium.org/3189014/diff/6001/7001#newcode45
net/base/cert_database_nss_unittest.cc:45: X509Certificate::CreateFromHandle(
On 2010/08/19 22:59:19, wtc wrote:
> Nit: indent this line by 4.

Done.

http://codereview.chromium.org/3189014/diff/6001/7001#newcode88
net/base/cert_database_nss_unittest.cc:88: std::string
cert_data(ReadTestCert("client.p12"));
On 2010/08/19 22:59:19, wtc wrote:
> Nit: I prefer using the = operator instead of the copy
> constructor here (and at line 98) below, because it makes
> the code easier to read.  What do you think?

Don't feel strongly either way, though I guess it is a little easier to read. 
Changed.

> Terminology issue: a PKCS #12 file also contains a private
> key, so it's more than a cert.

fixed.

[wtc, 2010-08-19 23:58:54]

LGTM!