Pull implementation of CertStore out to be reused.

content/browser/cert_store_impl.cc

Index: content/browser/cert_store_impl.cc
diff --git a/content/browser/cert_store_impl.cc b/content/browser/cert_store_impl.cc
index e7fc1d24060f52a1a3865a2a23d90db0f415fbd6..f20f530d5119f7ff13c05a9ea607d5966276919a 100644
--- a/content/browser/cert_store_impl.cc
+++ b/content/browser/cert_store_impl.cc
@@ -15,168 +15,27 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"

[wtc, 2013/11/14 22:04:58]

Please trim the header list. Some of them are not necessary (for example, the
notification-related headers and base/bind.h).

[alcutter, 2013/11/15 13:33:07]

Done.

 
-template <typename T>
-struct MatchSecond {
-  explicit MatchSecond(const T& t) : value(t) {}
-
-  template<typename Pair>
-  bool operator()(const Pair& p) const {
-    return (value == p.second);
-  }
-  T value;
-};
-
 namespace content {
 
 // static
-CertStore* CertStore::GetInstance() {
-  return CertStoreImpl::GetInstance();
-}
+CertStore* CertStore::GetInstance() { return CertStoreImpl::GetInstance(); }

[wtc, 2013/11/14 22:04:58]

Nit: in general it's not necessary to reformat code like this, unless it is
suggested by some automated tool.

[agl, 2013/11/14 22:09:28]

(p.s. this is my fault. One of the files had numerous style issues and I asked
for it to be run through clang-format. It looks like this one was also
processed.)

[alcutter, 2013/11/15 13:33:07]

Undone!

 
 //  static
 CertStoreImpl* CertStoreImpl::GetInstance() {
   return Singleton<CertStoreImpl>::get();
 }
 
-CertStoreImpl::CertStoreImpl() : next_cert_id_(1) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    RegisterForNotification();
-  } else {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        base::Bind(&CertStoreImpl::RegisterForNotification,
-                   base::Unretained(this)));
-  }
-}
+CertStoreImpl::CertStoreImpl() : store_() {}
 
-CertStoreImpl::~CertStoreImpl() {
-}
-
-void CertStoreImpl::RegisterForNotification() {
-  // We watch for RenderProcess termination, as this is how we clear
-  // certificates for now.
-  // TODO(jcampan): we should be listening to events such as resource cached/
-  //                removed from cache, and remove the cert when we know it
-  //                is not used anymore.
-
-  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                 NotificationService::AllBrowserContextsAndSources());
-  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                 NotificationService::AllBrowserContextsAndSources());
-}
+CertStoreImpl::~CertStoreImpl() {}
 
 int CertStoreImpl::StoreCert(net::X509Certificate* cert, int process_id) {
-  DCHECK(cert);
-  base::AutoLock auto_lock(cert_lock_);
-
-  int cert_id;
-
-  // Do we already know this cert?
-  ReverseCertMap::iterator cert_iter = cert_to_id_.find(cert);
-  if (cert_iter == cert_to_id_.end()) {
-    cert_id = next_cert_id_++;
-    // We use 0 as an invalid cert_id value.  In the unlikely event that
-    // next_cert_id_ wraps around, we reset it to 1.
-    if (next_cert_id_ == 0)
-      next_cert_id_ = 1;
-    cert->AddRef();
-    id_to_cert_[cert_id] = cert;
-    cert_to_id_[cert] = cert_id;
-  } else {
-    cert_id = cert_iter->second;
-  }
-
-  // Let's update process_id_to_cert_id_.
-  std::pair<IDMap::iterator, IDMap::iterator> process_ids =
-      process_id_to_cert_id_.equal_range(process_id);
-  if (std::find_if(process_ids.first, process_ids.second,
-                   MatchSecond<int>(cert_id)) == process_ids.second) {
-    process_id_to_cert_id_.insert(std::make_pair(process_id, cert_id));
-  }
-
-  // And cert_id_to_process_id_.
-  std::pair<IDMap::iterator, IDMap::iterator> cert_ids =
-      cert_id_to_process_id_.equal_range(cert_id);
-  if (std::find_if(cert_ids.first, cert_ids.second,
-                   MatchSecond<int>(process_id)) == cert_ids.second) {
-    cert_id_to_process_id_.insert(std::make_pair(cert_id, process_id));
-  }
-
-  return cert_id;
+  return store_.Store(cert, process_id);
 }
 
 bool CertStoreImpl::RetrieveCert(int cert_id,
                                  scoped_refptr<net::X509Certificate>* cert) {
-  base::AutoLock auto_lock(cert_lock_);
-
-  CertMap::iterator iter = id_to_cert_.find(cert_id);
-  if (iter == id_to_cert_.end())
-    return false;
-  if (cert)
-    *cert = iter->second;
-  return true;
-}
-
-void CertStoreImpl::RemoveCertInternal(int cert_id) {
-  CertMap::iterator cert_iter = id_to_cert_.find(cert_id);
-  DCHECK(cert_iter != id_to_cert_.end());
-
-  ReverseCertMap::iterator id_iter = cert_to_id_.find(cert_iter->second.get());
-  DCHECK(id_iter != cert_to_id_.end());
-  cert_to_id_.erase(id_iter);
-
-  cert_iter->second->Release();
-  id_to_cert_.erase(cert_iter);
-}
-
-void CertStoreImpl::RemoveCertsForRenderProcesHost(int process_id) {
-  base::AutoLock auto_lock(cert_lock_);
-
-  // We iterate through all the cert ids for that process.
-  std::pair<IDMap::iterator, IDMap::iterator> process_ids =
-      process_id_to_cert_id_.equal_range(process_id);
-  for (IDMap::iterator ids_iter = process_ids.first;
-       ids_iter != process_ids.second; ++ids_iter) {
-    int cert_id = ids_iter->second;
-    // Find all the processes referring to this cert id in
-    // cert_id_to_process_id_, then locate the process being removed within
-    // that range.
-    std::pair<IDMap::iterator, IDMap::iterator> cert_ids =
-        cert_id_to_process_id_.equal_range(cert_id);
-    IDMap::iterator proc_iter =
-        std::find_if(cert_ids.first, cert_ids.second,
-                     MatchSecond<int>(process_id));
-    DCHECK(proc_iter != cert_ids.second);
-
-    // Before removing, determine if no other processes refer to the current
-    // cert id. If |proc_iter| (the current process) is the lower bound of
-    // processes containing the current cert id and if |next_proc_iter| is the
-    // upper bound (the first process that does not), then only one process,
-    // the one being removed, refers to the cert id.
-    IDMap::iterator next_proc_iter = proc_iter;
-    ++next_proc_iter;
-    bool last_process_for_cert_id =
-        (proc_iter == cert_ids.first && next_proc_iter == cert_ids.second);
-    cert_id_to_process_id_.erase(proc_iter);
-
-    if (last_process_for_cert_id) {
-      // The current cert id is not referenced by any other processes, so
-      // remove it from id_to_cert_ and cert_to_id_.
-      RemoveCertInternal(cert_id);
-    }
-  }
-  if (process_ids.first != process_ids.second)
-    process_id_to_cert_id_.erase(process_ids.first, process_ids.second);
-}
-
-void CertStoreImpl::Observe(int type,
-                            const NotificationSource& source,
-                            const NotificationDetails& details) {
-  DCHECK(type == NOTIFICATION_RENDERER_PROCESS_TERMINATED ||
-         type == NOTIFICATION_RENDERER_PROCESS_CLOSED);
-  RenderProcessHost* rph = Source<RenderProcessHost>(source).ptr();
-  DCHECK(rph);
-  RemoveCertsForRenderProcesHost(rph->GetID());
+  return store_.Retrieve(cert_id, cert);
 }
 
 }  // namespace content