Disk Based Certificate Cache Implementation

net/http/disk_based_cert_cache.h

+// 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.
+
+#ifndef NET_HTTP_DISK_BASED_CERT_CACHE_H
+#define NET_HTTP_DISK_BASED_CERT_CACHE_H
+
+#include <string>
+
+#include "base/bind.h"

[Ryan Sleevi, 2014/06/18 22:16:04]

You don't need Bind here; you can move it to the .cc file

+#include "base/callback.h"
+#include "base/containers/hash_tables.h"
+#include "base/memory/weak_ptr.h"
+#include "net/base/completion_callback.h"

[Ryan Sleevi, 2014/06/18 22:16:04]

You don't use net::CompletionCallback in the .h. If you do need it, move it to
the .cc

+#include "net/base/net_export.h"
+#include "net/cert/x509_certificate.h"
+
+namespace disk_cache {
+class Backend;
+}  // namespace disk_cache
+
+namespace net {
+
+class NET_EXPORT_PRIVATE DiskBasedCertCache {
+ public:
+  typedef base::Callback<void(const X509Certificate::OSCertHandle cert_handle)>
+      GetCallback;
+  typedef base::Callback<void(const std::string&)> SetCallback;
+
+  // Constructor takes in a previously initialized backend, which is then
+  // used to store the certificates in the cache.
+  explicit DiskBasedCertCache(disk_cache::Backend* backend);
+  ~DiskBasedCertCache();
+
+  // Fetches the certificate associated with |key|. If the certificate is
+  // found within the cache, |cb| will be called with the certificate.
+  // Otherwise, |cb| will be called with NULL. Document ownership of
+  // |cert_handle| is not transferred.

[Ryan Sleevi, 2014/06/18 22:16:04]

I suspect there may have been confusion about my remark about "Document
ownership of |cert_handle| is not transferred."

That was a directive for you to document

eg:

// Ownership of |cert_handle| is not transferred, and it will be
// invalidated at the end of the callback. Callers that wish to
// store a copy should use X509Certificate::DupOSCertHandle

+  void Get(const std::string& key, const GetCallback& cb);
+
+  // Stores |cert_handle| in the cache. If |cert_handle| is successfully stored,
+  // |cb| will be called with the key. If |cb| is called with an empty
+  // string, then |handle| was not stored.
+  void Set(const X509Certificate::OSCertHandle cert_handle,
+           const SetCallback& cb);
+
+ private:
+  class ReadWorker;
+  class WriteWorker;
+
+  typedef base::hash_map<std::string, ReadWorker*> ReadWorkerMap;
+  typedef base::hash_map<std::string, WriteWorker*> WriteWorkerMap;
+
+  // FinishedReadOperation and FinishedWriteOperation are used by callbacks
+  // given to the workers to signal the DiskBasedCertCache they have completed
+  // their work. The workers are then deleted and removed from their respective
+  // hash maps.
+  void FinishedReadOperation(const std::string& key);
+  void FinishedWriteOperation(const std::string& key);
+
+  ReadWorkerMap read_worker_map_;
+  WriteWorkerMap write_worker_map_;
+
+  disk_cache::Backend* backend_;
+  base::WeakPtrFactory<DiskBasedCertCache> weak_factory_;
+};

[Ryan Sleevi, 2014/06/18 22:16:04]

You will want to use DISALLOW_COPY_AND_ASSIGN(DiskBasedCertCache) here, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Copy_C...

+
+}  // namespace net
+
+#endif  // NET_HTTP_DISK_BASED_CERT_CACHE_H