Adding cache hit/miss histograms to DiskBasedCertCache.

net/http/disk_based_cert_cache.cc

Index: net/http/disk_based_cert_cache.cc
diff --git a/net/http/disk_based_cert_cache.cc b/net/http/disk_based_cert_cache.cc
index 606407999e65eb9339f67504aca2f1fb924573bf..c9afb380aaf05e5a967fdfc6b8e16356058b7179 100644
--- a/net/http/disk_based_cert_cache.cc
+++ b/net/http/disk_based_cert_cache.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/memory/ref_counted.h"
+#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "net/base/io_buffer.h"
@@ -24,7 +25,8 @@ const size_t kMemoryCacheMaxSize = 30;
 
 // Used to obtain a unique cache key for a certificate in the form of
 // "cert:<hash>".
-std::string GetCacheKeyToCert(const X509Certificate::OSCertHandle cert_handle) {
+std::string GetCacheKeyForCert(
+    const X509Certificate::OSCertHandle cert_handle) {
   SHA1HashValue fingerprint =
       X509Certificate::CalculateFingerprint(cert_handle);
 
@@ -32,6 +34,13 @@ std::string GetCacheKeyToCert(const X509Certificate::OSCertHandle cert_handle) {
          base::HexEncode(fingerprint.data, arraysize(fingerprint.data));
 }
 
+enum CacheResult { MEMORY_CACHE_HIT = 0, DISK_CACHE_HIT, CACHE_MISS };

[jar (doing other things), 2014/07/10 00:04:58]

nit: Add one more enum:
CACHE_RESULT_MAX

Use that in the UMA_HISTOGRAM_ENUMERATION.

+
+void RecordCacheResult(CacheResult result) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "DiskBasedCertCache.CertIoCacheResult", result, CACHE_MISS);

[jar (doing other things), 2014/07/10 00:04:58]

nit: Replace CACHE_MISS With CACHE_RESULT_MAX

+}
+
 }  // namespace
 
 // WriteWorkers represent pending Set jobs in the DiskBasedCertCache. Each
@@ -123,7 +132,8 @@ DiskBasedCertCache::WriteWorker::WriteWorker(
 }
 
 DiskBasedCertCache::WriteWorker::~WriteWorker() {
-  X509Certificate::FreeOSCertHandle(cert_handle_);
+  if (cert_handle_)
+    X509Certificate::FreeOSCertHandle(cert_handle_);
   if (entry_)
     entry_->Close();
 }
@@ -428,8 +438,14 @@ int DiskBasedCertCache::ReadWorker::DoOpen() {
 }
 
 int DiskBasedCertCache::ReadWorker::DoOpenComplete(int rv) {
-  if (rv < 0)
+  if (rv < 0) {
+    // Errors other than ERR_CACHE_MISS are not recorded as either a hit

[jar (doing other things), 2014/07/10 00:04:58]

Consider adding one more enum, which captures these error cases.  It is nice to
not lose data... even when you predict it will be non-existant... sometimes we
learn stuff ;-).

+    // or a miss.
+    if (rv == ERR_CACHE_MISS)
+      RecordCacheResult(CACHE_MISS);
     return rv;
+  }
+  RecordCacheResult(DISK_CACHE_HIT);
 
   state_ = STATE_READ;
   return OK;
@@ -506,6 +522,7 @@ void DiskBasedCertCache::Get(const std::string& key, const GetCallback& cb) {
   // list in the MRU cache.
   MRUCertCache::iterator mru_it = mru_cert_cache_.Get(key);
   if (mru_it != mru_cert_cache_.end()) {
+    RecordCacheResult(MEMORY_CACHE_HIT);
     ++mem_cache_hits_;
     cb.Run(mru_it->second);
     return;
@@ -533,7 +550,7 @@ void DiskBasedCertCache::Set(const X509Certificate::OSCertHandle cert_handle,
                              const SetCallback& cb) {
   DCHECK(!cb.is_null());
   DCHECK(cert_handle);
-  std::string key = GetCacheKeyToCert(cert_handle);
+  std::string key = GetCacheKeyForCert(cert_handle);
 
   WriteWorkerMap::iterator it = write_worker_map_.find(key);