Pull implementation of CertStore out to be reused.

content/browser/item_store.h

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[agl, 2013/11/14 16:20:37]

2013

[agl, 2013/11/14 16:20:37]

Note, this file name is very generic. See comments below.

[alcutter, 2013/11/14 16:51:59]

Ack.

[alcutter, 2013/11/14 16:51:59]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <map>
+
+#include "base/bind.h"
+#include "base/stl_util.h"
+#include "base/synchronization/lock.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+
+template <typename T>

[agl, 2013/11/14 16:20:37]

This pollutes the global name space here. I think it can be made a private,
sub-class of ItemStore.

[alcutter, 2013/11/14 16:51:59]

Done.

+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 {
+
+template <typename T>
+class ItemStore : public NotificationObserver {

[agl, 2013/11/14 16:20:37]

ItemStore is an incredibly generic name.

RendererDataMemoizingStore?

[agl, 2013/11/14 16:20:37]

This needs a fairly large comment explaining it:

// RendererDataMemoizingStore is a thread-safe container that retains reference
counted objects that are associated with one or more render processes. Objects
are identified by an int and only a single instance of a given object is
retained. RendererDataMemoizingStore watches for render process termination and
releases objects that are no longer associated with any render process.

[alcutter, 2013/11/14 16:51:59]

Ok, I'll rename once you're happy with everything else to avoid causing too much
churn mid-review.

[alcutter, 2013/11/14 16:51:59]

Fair point - this file is really just the old contents of cert_store.cc so I
didn't mess with it more than I needed to.

Comment added.

+ protected:

[agl, 2013/11/14 16:20:37]

You are inheriting from this object. Are you sure that you don't want to include
an instance of it instead? (That would need the members to be sorted into public
and private.)

[alcutter, 2013/11/14 16:51:59]

I don't really mind either way, I'm happy to change to composition if you prefer
?

+  ItemStore() : next_item_id_(1) {
+    if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+      RegisterForNotification();
+    } else {
+      BrowserThread::PostTask(BrowserThread::UI,
+                              FROM_HERE,
+                              base::Bind(&ItemStore::RegisterForNotification,
+                                         base::Unretained(this)));
+    }
+  }
+
+  virtual ~ItemStore() {}
+
+  void RegisterForNotification() {
+    // We watch for RenderProcess termination, as this is how we clear
+    // items for now.
+    // TODO(jcampan): we should be listening to events such as resource cached/
+    //                removed from cache, and remove the item 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());
+  }
+
+  int Store(T* item, int process_id) {

[agl, 2013/11/14 16:20:37]

// Store adds |item| to this collection, associates it with the given render
process id and returns an opaque identifier for it. If |item| is already known,
the same identifier will be returned. 

[alcutter, 2013/11/14 16:51:59]

Done.

+    DCHECK(item);
+    base::AutoLock auto_lock(lock_);
+
+    int item_id;
+
+    // Do we already know this item?
+    typename ReverseItemMap::iterator item_iter = item_to_id_.find(item);
+    if (item_iter == item_to_id_.end()) {
+      item_id = next_item_id_++;
+      // We use 0 as an invalid item_id value.  In the unlikely event that
+      // next_item_id_ wraps around, we reset it to 1.
+      if (next_item_id_ == 0)
+        next_item_id_ = 1;
+      item->AddRef();
+      id_to_item_[item_id] = item;
+      item_to_id_[item] = item_id;
+    } else {
+      item_id = item_iter->second;
+    }
+
+    // Let's update process_id_to_item_id_.
+    std::pair<IDMap::iterator, IDMap::iterator> process_ids =
+        process_id_to_item_id_.equal_range(process_id);
+    if (std::find_if(process_ids.first,
+                     process_ids.second,
+                     MatchSecond<int>(item_id)) == process_ids.second) {
+      process_id_to_item_id_.insert(std::make_pair(process_id, item_id));
+    }
+
+    // And item_id_to_process_id_.
+    std::pair<IDMap::iterator, IDMap::iterator> item_ids =
+        item_id_to_process_id_.equal_range(item_id);
+    if (std::find_if(item_ids.first,
+                     item_ids.second,
+                     MatchSecond<int>(process_id)) == item_ids.second) {
+      item_id_to_process_id_.insert(std::make_pair(item_id, process_id));
+    }
+
+    return item_id;
+  }
+
+  bool Retrieve(int item_id, scoped_refptr<T>* item) {

[agl, 2013/11/14 16:20:37]

// Retrieve ...

[alcutter, 2013/11/14 16:51:59]

Done.

+    base::AutoLock auto_lock(lock_);
+
+    typename ItemMap::iterator iter = id_to_item_.find(item_id);
+    if (iter == id_to_item_.end())
+      return false;
+    if (item)
+      *item = iter->second;
+    return true;
+  }
+
+  void RemoveInternal(int item_id) {
+    typename ItemMap::iterator item_iter = id_to_item_.find(item_id);
+    DCHECK(item_iter != id_to_item_.end());
+
+    typename ReverseItemMap::iterator id_iter =
+        item_to_id_.find(item_iter->second.get());
+    DCHECK(id_iter != item_to_id_.end());
+    item_to_id_.erase(id_iter);
+
+    item_iter->second->Release();
+    id_to_item_.erase(item_iter);
+  }
+
+  void RemoveForRenderProcessHost(int process_id) {
+    base::AutoLock auto_lock(lock_);
+
+    // We iterate through all the item ids for that process.
+    std::pair<IDMap::iterator, IDMap::iterator> process_ids =
+        process_id_to_item_id_.equal_range(process_id);
+    for (IDMap::iterator ids_iter = process_ids.first;
+         ids_iter != process_ids.second;
+         ++ids_iter) {
+      int item_id = ids_iter->second;
+      // Find all the processes referring to this item id in
+      // item_id_to_process_id_, then locate the process being removed within
+      // that range.
+      std::pair<IDMap::iterator, IDMap::iterator> item_ids =
+          item_id_to_process_id_.equal_range(item_id);
+      IDMap::iterator proc_iter = std::find_if(
+          item_ids.first, item_ids.second, MatchSecond<int>(process_id));
+      DCHECK(proc_iter != item_ids.second);
+
+      // Before removing, determine if no other processes refer to the current
+      // item id. If |proc_iter| (the current process) is the lower bound of
+      // processes containing the current item 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 item id.
+      IDMap::iterator next_proc_iter = proc_iter;
+      ++next_proc_iter;
+      bool last_process_for_item_id =
+          (proc_iter == item_ids.first && next_proc_iter == item_ids.second);
+      item_id_to_process_id_.erase(proc_iter);
+
+      if (last_process_for_item_id) {
+        // The current item id is not referenced by any other processes, so
+        // remove it from id_to_item_ and item_to_id_.
+        RemoveInternal(item_id);
+      }
+    }
+    if (process_ids.first != process_ids.second)
+      process_id_to_item_id_.erase(process_ids.first, process_ids.second);
+  }
+
+  void 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);
+    RemoveForRenderProcessHost(rph->GetID());
+  }
+
+ protected:
+  typedef std::multimap<int, int> IDMap;
+  typedef std::map<int, scoped_refptr<T> > ItemMap;
+  typedef std::map<T*, int, typename T::LessThan> ReverseItemMap;
+
+  // Is only used on the UI Thread.
+  NotificationRegistrar registrar_;
+
+  IDMap process_id_to_item_id_;
+  IDMap item_id_to_process_id_;
+  ItemMap id_to_item_;
+  ReverseItemMap item_to_id_;
+
+  int next_item_id_;
+
+  // This lock protects: process_to_item_id_, item_id_to_process_id_,
+  //                     id_to_item_, and item_to_id_.
+  base::Lock lock_;
+};
+
+}  // namespace content