Improving and adding an in-memory MRU cache to DiskBasedCertCache.

net/http/disk_based_cert_cache_unittest.cc

 // Copyright (c) 2014 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 "net/http/disk_based_cert_cache.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "net/base/completion_callback.h"
 #include "net/base/io_buffer.h"
@@ skipping 250 matching lines @@
   ScopedMockTransaction trans1(
       CreateMockTransaction(kCert1.cache_key, TEST_MODE_NORMAL));
   MockDiskCache backend;
   ASSERT_NO_FATAL_FAILURE(ImportCert(&backend, kCert1, true /* corrupted */));
   DiskBasedCertCache cache(&backend);
   TestGetCallback get_callback;
 
   cache.Get(kCert1.cache_key, get_callback.callback());
   get_callback.WaitForResult();
 
-  scoped_refptr<X509Certificate> cert(
-      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
   EXPECT_FALSE(get_callback.cert_handle());
 }
 
 // Tests that attempting to retrieve a cert that is not in the cache will
 // return NULL.
 TEST(DiskBasedCertCache, GetUncachedCert) {
   ScopedMockTransaction trans1(
       CreateMockTransaction(kCert1.cache_key, TEST_MODE_NORMAL));
   MockDiskCache backend;
   DiskBasedCertCache cache(&backend);
@@ skipping 185 matching lines @@
       ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
   ASSERT_TRUE(cert.get());
   TestSetCallback set_callback;
 
   cache->Set(cert->os_cert_handle(), set_callback.callback());
   cache.reset();
   set_callback.WaitForResult();
   EXPECT_EQ(std::string(), set_callback.key());
 }
 
+// Issues two successive read requests to the DBCC for a certificate,

[wtc, 2014/07/02 20:51:50]

Nit: the DBCC abbreviation is not common. Also, you fully spelled out
DiskBasedCertCache in the next line.

+// and then checks that the DiskBasedCertCache correctly read and recorded
+// reading through the in-memory MRU cache.
+TEST(DiskBasedCertCache, MemCacheGet) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.cache_key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  ASSERT_NO_FATAL_FAILURE(
+      (ImportCert(&backend, kCert1, false /* not corrupted */)));

[wtc, 2014/07/02 20:51:50]

The outer most parentheses on this line don't seem neessary.

+  DiskBasedCertCache cache(&backend);
+
+  TestGetCallback get_callback1, get_callback2;
+  cache.Get(kCert1.cache_key, get_callback1.callback());
+  get_callback1.WaitForResult();
+  EXPECT_EQ(0, cache.MemCacheHits());
+  cache.Get(kCert1.cache_key, get_callback2.callback());
+  get_callback2.WaitForResult();
+  EXPECT_EQ(1, cache.MemCacheHits());
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(get_callback1.cert_handle(),
+                                            get_callback2.cert_handle()));
+}
+
+// Issues two successive write requests to the DBCC for a certificate,
+// and then checks that the DiskBasedCertCache correctly recorded
+// recorded an in-memory cache hit for the second.

[wtc, 2014/07/02 20:51:50]

Delete the second "recorded".

+TEST(DiskBasedCertCache, MemCacheSet) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.cache_key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
+  ASSERT_TRUE(cert.get());
+
+  TestSetCallback set_callback1, set_callback2;
+  cache.Set(cert->os_cert_handle(), set_callback1.callback());
+  set_callback1.WaitForResult();
+  EXPECT_EQ(kCert1.cache_key, set_callback1.key());
+  EXPECT_EQ(0, cache.MemCacheHits());
+
+  cache.Set(cert->os_cert_handle(), set_callback2.callback());
+  set_callback2.WaitForResult();
+  EXPECT_EQ(1, cache.MemCacheHits());
+  ASSERT_NO_FATAL_FAILURE(CheckCertCached(&backend, kCert1));
+}
+
+// Reads a corrupted certificate from the disk cache, and then overwrites
+// it and checks that the uncorrupted version was stored in the in-memory
+// cache.
+TEST(DiskBasedCertCache, CorruptOverwrite) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.cache_key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  backend.set_double_create_check(false);
+  ASSERT_NO_FATAL_FAILURE(ImportCert(&backend, kCert1, true /* corrupted */));
+  DiskBasedCertCache cache(&backend);
+  TestGetCallback get_callback1, get_callback2;
+
+  cache.Get(kCert1.cache_key, get_callback1.callback());
+  get_callback1.WaitForResult();
+  EXPECT_FALSE(get_callback2.cert_handle());
+
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
+  TestSetCallback set_callback;
+
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  set_callback.WaitForResult();
+  EXPECT_EQ(kCert1.cache_key, set_callback.key());
+  EXPECT_EQ(0, cache.MemCacheHits());
+
+  cache.Get(kCert1.cache_key, get_callback2.callback());
+  get_callback2.WaitForResult();
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(get_callback2.cert_handle(),
+                                            cert->os_cert_handle()));
+  EXPECT_EQ(1, cache.MemCacheHits());
+  ASSERT_NO_FATAL_FAILURE(CheckCertCached(&backend, kCert1));
+}
+
 }  // namespace net
+

[wtc, 2014/07/02 20:51:50]

Nit: did you mean to add this blank line?