0

base: Remove OwningMRUCache in favor of scoped_ptrs in MRUCache

Along with removing OwningMRUCache, this patch also removes
the Deletor concept from the cache, since it can be implemented
by storing scoped_ptrs with custom deleters.

BUG=592823

Review URL: https://codereview.chromium.org/1763273002

Cr-Commit-Position: refs/heads/master@{#380239}
This commit is contained in:
vmpstr
2016-03-09 14:08:48 -08:00
committed by Commit bot
parent cc6220fa81
commit aaab4d14f3
17 changed files with 98 additions and 251 deletions

@@ -41,12 +41,9 @@ struct MRUCacheStandardMap {
}; };
// Base class for the MRU cache specializations defined below. // Base class for the MRU cache specializations defined below.
// The deletor will get called on all payloads that are being removed or
// replaced.
template <class KeyType, template <class KeyType,
class PayloadType, class PayloadType,
class HashOrCompareType, class HashOrCompareType,
class DeletorType,
template <typename, typename, typename> class MapType = template <typename, typename, typename> class MapType =
MRUCacheStandardMap> MRUCacheStandardMap>
class MRUCacheBase { class MRUCacheBase {
@@ -75,18 +72,9 @@ class MRUCacheBase {
// a new item is inserted. If the caller wants to manager this itself (for // a new item is inserted. If the caller wants to manager this itself (for
// example, maybe it has special work to do when something is evicted), it // example, maybe it has special work to do when something is evicted), it
// can pass NO_AUTO_EVICT to not restrict the cache size. // can pass NO_AUTO_EVICT to not restrict the cache size.
explicit MRUCacheBase(size_type max_size) : max_size_(max_size) { explicit MRUCacheBase(size_type max_size) : max_size_(max_size) {}
}
MRUCacheBase(size_type max_size, const DeletorType& deletor) virtual ~MRUCacheBase() {}
: max_size_(max_size), deletor_(deletor) {
}
virtual ~MRUCacheBase() {
iterator i = begin();
while (i != end())
i = Erase(i);
}
size_type max_size() const { return max_size_; } size_type max_size() const { return max_size_; }
@@ -94,14 +82,14 @@ class MRUCacheBase {
// the same key, it is removed prior to insertion. An iterator indicating the // the same key, it is removed prior to insertion. An iterator indicating the
// inserted item will be returned (this will always be the front of the list). // inserted item will be returned (this will always be the front of the list).
// //
// The payload will be copied. In the case of an OwningMRUCache, this function // The payload will be forwarded.
// will take ownership of the pointer. template <typename Payload>
iterator Put(const KeyType& key, const PayloadType& payload) { iterator Put(const KeyType& key, Payload&& payload) {
// Remove any existing payload with that key. // Remove any existing payload with that key.
typename KeyIndex::iterator index_iter = index_.find(key); typename KeyIndex::iterator index_iter = index_.find(key);
if (index_iter != index_.end()) { if (index_iter != index_.end()) {
// Erase the reference to it. This will call the deletor on the removed // Erase the reference to it. The index reference will be replaced in the
// element. The index reference will be replaced in the code below. // code below.
Erase(index_iter->second); Erase(index_iter->second);
} else if (max_size_ != NO_AUTO_EVICT) { } else if (max_size_ != NO_AUTO_EVICT) {
// New item is being inserted which might make it larger than the maximum // New item is being inserted which might make it larger than the maximum
@@ -109,7 +97,7 @@ class MRUCacheBase {
ShrinkToSize(max_size_ - 1); ShrinkToSize(max_size_ - 1);
} }
ordering_.push_front(value_type(key, payload)); ordering_.push_front(value_type(key, std::forward<Payload>(payload)));
index_.insert(std::make_pair(key, ordering_.begin())); index_.insert(std::make_pair(key, ordering_.begin()));
return ordering_.begin(); return ordering_.begin();
} }
@@ -150,14 +138,12 @@ class MRUCacheBase {
void Swap(MRUCacheBase& other) { void Swap(MRUCacheBase& other) {
ordering_.swap(other.ordering_); ordering_.swap(other.ordering_);
index_.swap(other.index_); index_.swap(other.index_);
std::swap(deletor_, other.deletor_);
std::swap(max_size_, other.max_size_); std::swap(max_size_, other.max_size_);
} }
// Erases the item referenced by the given iterator. An iterator to the item // Erases the item referenced by the given iterator. An iterator to the item
// following it will be returned. The iterator must be valid. // following it will be returned. The iterator must be valid.
iterator Erase(iterator pos) { iterator Erase(iterator pos) {
deletor_(pos->second);
index_.erase(pos->first); index_.erase(pos->first);
return ordering_.erase(pos); return ordering_.erase(pos);
} }
@@ -180,9 +166,6 @@ class MRUCacheBase {
// Deletes everything from the cache. // Deletes everything from the cache.
void Clear() { void Clear() {
for (typename PayloadList::iterator i(ordering_.begin());
i != ordering_.end(); ++i)
deletor_(i->second);
index_.clear(); index_.clear();
ordering_.clear(); ordering_.clear();
} }
@@ -219,83 +202,28 @@ class MRUCacheBase {
size_type max_size_; size_type max_size_;
DeletorType deletor_;
DISALLOW_COPY_AND_ASSIGN(MRUCacheBase); DISALLOW_COPY_AND_ASSIGN(MRUCacheBase);
}; };
// MRUCache -------------------------------------------------------------------- // MRUCache --------------------------------------------------------------------
// A functor that does nothing. Used by the MRUCache.
template<class PayloadType>
class MRUCacheNullDeletor {
public:
void operator()(const PayloadType& payload) {}
};
// A container that does not do anything to free its data. Use this when storing // A container that does not do anything to free its data. Use this when storing
// value types (as opposed to pointers) in the list. // value types (as opposed to pointers) in the list.
template <class KeyType, class PayloadType> template <class KeyType, class PayloadType>
class MRUCache : public MRUCacheBase<KeyType, class MRUCache : public MRUCacheBase<KeyType, PayloadType, std::less<KeyType>> {
PayloadType,
std::less<KeyType>,
MRUCacheNullDeletor<PayloadType>> {
private: private:
typedef MRUCacheBase<KeyType, using ParentType = MRUCacheBase<KeyType, PayloadType, std::less<KeyType>>;
PayloadType,
std::less<KeyType>,
MRUCacheNullDeletor<PayloadType>>
ParentType;
public: public:
// See MRUCacheBase, noting the possibility of using NO_AUTO_EVICT. // See MRUCacheBase, noting the possibility of using NO_AUTO_EVICT.
explicit MRUCache(typename ParentType::size_type max_size) explicit MRUCache(typename ParentType::size_type max_size)
: ParentType(max_size) { : ParentType(max_size) {}
} virtual ~MRUCache() {}
virtual ~MRUCache() {
}
private: private:
DISALLOW_COPY_AND_ASSIGN(MRUCache); DISALLOW_COPY_AND_ASSIGN(MRUCache);
}; };
// OwningMRUCache --------------------------------------------------------------
template<class PayloadType>
class MRUCachePointerDeletor {
public:
void operator()(const PayloadType& payload) { delete payload; }
};
// A cache that owns the payload type, which must be a non-const pointer type.
// The pointers will be deleted when they are removed, replaced, or when the
// cache is destroyed.
// TODO(vmpstr): This should probably go away in favor of storing scoped_ptrs.
template <class KeyType, class PayloadType>
class OwningMRUCache
: public MRUCacheBase<KeyType,
PayloadType,
std::less<KeyType>,
MRUCachePointerDeletor<PayloadType>> {
private:
typedef MRUCacheBase<KeyType,
PayloadType,
std::less<KeyType>,
MRUCachePointerDeletor<PayloadType>>
ParentType;
public:
// See MRUCacheBase, noting the possibility of using NO_AUTO_EVICT.
explicit OwningMRUCache(typename ParentType::size_type max_size)
: ParentType(max_size) {
}
virtual ~OwningMRUCache() {
}
private:
DISALLOW_COPY_AND_ASSIGN(OwningMRUCache);
};
// HashingMRUCache ------------------------------------------------------------ // HashingMRUCache ------------------------------------------------------------
template <class KeyType, class ValueType, class HashType> template <class KeyType, class ValueType, class HashType>
@@ -307,26 +235,17 @@ struct MRUCacheHashMap {
// the map type instead of std::map. Note that your KeyType must be hashable to // the map type instead of std::map. Note that your KeyType must be hashable to
// use this cache or you need to provide a hashing class. // use this cache or you need to provide a hashing class.
template <class KeyType, class PayloadType, class HashType = std::hash<KeyType>> template <class KeyType, class PayloadType, class HashType = std::hash<KeyType>>
class HashingMRUCache : public MRUCacheBase<KeyType, class HashingMRUCache
PayloadType, : public MRUCacheBase<KeyType, PayloadType, HashType, MRUCacheHashMap> {
HashType,
MRUCacheNullDeletor<PayloadType>,
MRUCacheHashMap> {
private: private:
typedef MRUCacheBase<KeyType, using ParentType =
PayloadType, MRUCacheBase<KeyType, PayloadType, HashType, MRUCacheHashMap>;
HashType,
MRUCacheNullDeletor<PayloadType>,
MRUCacheHashMap>
ParentType;
public: public:
// See MRUCacheBase, noting the possibility of using NO_AUTO_EVICT. // See MRUCacheBase, noting the possibility of using NO_AUTO_EVICT.
explicit HashingMRUCache(typename ParentType::size_type max_size) explicit HashingMRUCache(typename ParentType::size_type max_size)
: ParentType(max_size) { : ParentType(max_size) {}
} virtual ~HashingMRUCache() {}
virtual ~HashingMRUCache() {
}
private: private:
DISALLOW_COPY_AND_ASSIGN(HashingMRUCache); DISALLOW_COPY_AND_ASSIGN(HashingMRUCache);

@@ -5,6 +5,7 @@
#include <stddef.h> #include <stddef.h>
#include "base/containers/mru_cache.h" #include "base/containers/mru_cache.h"
#include "base/memory/scoped_ptr.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace { namespace {
@@ -187,15 +188,15 @@ TEST(MRUCacheTest, KeyReplacement) {
// Make sure that the owning version release its pointers properly. // Make sure that the owning version release its pointers properly.
TEST(MRUCacheTest, Owning) { TEST(MRUCacheTest, Owning) {
typedef base::OwningMRUCache<int, CachedItem*> Cache; using Cache = base::MRUCache<int, scoped_ptr<CachedItem>>;
Cache cache(Cache::NO_AUTO_EVICT); Cache cache(Cache::NO_AUTO_EVICT);
int initial_count = cached_item_live_count; int initial_count = cached_item_live_count;
// First insert and item and then overwrite it. // First insert and item and then overwrite it.
static const int kItem1Key = 1; static const int kItem1Key = 1;
cache.Put(kItem1Key, new CachedItem(20)); cache.Put(kItem1Key, make_scoped_ptr(new CachedItem(20)));
cache.Put(kItem1Key, new CachedItem(22)); cache.Put(kItem1Key, make_scoped_ptr(new CachedItem(22)));
// There should still be one item, and one extra live item. // There should still be one item, and one extra live item.
Cache::iterator iter = cache.Get(kItem1Key); Cache::iterator iter = cache.Get(kItem1Key);
@@ -211,8 +212,8 @@ TEST(MRUCacheTest, Owning) {
// go away. // go away.
{ {
Cache cache2(Cache::NO_AUTO_EVICT); Cache cache2(Cache::NO_AUTO_EVICT);
cache2.Put(1, new CachedItem(20)); cache2.Put(1, make_scoped_ptr(new CachedItem(20)));
cache2.Put(2, new CachedItem(20)); cache2.Put(2, make_scoped_ptr(new CachedItem(20)));
} }
// There should be no objects leaked. // There should be no objects leaked.
@@ -221,8 +222,8 @@ TEST(MRUCacheTest, Owning) {
// Check that Clear() also frees things correctly. // Check that Clear() also frees things correctly.
{ {
Cache cache2(Cache::NO_AUTO_EVICT); Cache cache2(Cache::NO_AUTO_EVICT);
cache2.Put(1, new CachedItem(20)); cache2.Put(1, make_scoped_ptr(new CachedItem(20)));
cache2.Put(2, new CachedItem(20)); cache2.Put(2, make_scoped_ptr(new CachedItem(20)));
EXPECT_EQ(initial_count + 2, cached_item_live_count); EXPECT_EQ(initial_count + 2, cached_item_live_count);
cache2.Clear(); cache2.Clear();
EXPECT_EQ(initial_count, cached_item_live_count); EXPECT_EQ(initial_count, cached_item_live_count);
@@ -230,7 +231,7 @@ TEST(MRUCacheTest, Owning) {
} }
TEST(MRUCacheTest, AutoEvict) { TEST(MRUCacheTest, AutoEvict) {
typedef base::OwningMRUCache<int, CachedItem*> Cache; using Cache = base::MRUCache<int, scoped_ptr<CachedItem>>;
static const Cache::size_type kMaxSize = 3; static const Cache::size_type kMaxSize = 3;
int initial_count = cached_item_live_count; int initial_count = cached_item_live_count;
@@ -239,10 +240,10 @@ TEST(MRUCacheTest, AutoEvict) {
Cache cache(kMaxSize); Cache cache(kMaxSize);
static const int kItem1Key = 1, kItem2Key = 2, kItem3Key = 3, kItem4Key = 4; static const int kItem1Key = 1, kItem2Key = 2, kItem3Key = 3, kItem4Key = 4;
cache.Put(kItem1Key, new CachedItem(20)); cache.Put(kItem1Key, make_scoped_ptr(new CachedItem(20)));
cache.Put(kItem2Key, new CachedItem(21)); cache.Put(kItem2Key, make_scoped_ptr(new CachedItem(21)));
cache.Put(kItem3Key, new CachedItem(22)); cache.Put(kItem3Key, make_scoped_ptr(new CachedItem(22)));
cache.Put(kItem4Key, new CachedItem(23)); cache.Put(kItem4Key, make_scoped_ptr(new CachedItem(23)));
// The cache should only have kMaxSize items in it even though we inserted // The cache should only have kMaxSize items in it even though we inserted
// more. // more.

@@ -96,9 +96,9 @@ BitmapFetcherService::RequestId BitmapFetcherService::RequestImage(
return REQUEST_ID_INVALID; return REQUEST_ID_INVALID;
// Check for existing images first. // Check for existing images first.
base::OwningMRUCache<GURL, CacheEntry*>::iterator iter = cache_.Get(url); auto iter = cache_.Get(url);
if (iter != cache_.end()) { if (iter != cache_.end()) {
BitmapFetcherService::CacheEntry* entry = iter->second; BitmapFetcherService::CacheEntry* entry = iter->second.get();
request->NotifyImageChanged(entry->bitmap.get()); request->NotifyImageChanged(entry->bitmap.get());
// There is no request ID associated with this - data is already delivered. // There is no request ID associated with this - data is already delivered.
@@ -184,9 +184,9 @@ void BitmapFetcherService::OnFetchComplete(const GURL& url,
} }
if (bitmap && !bitmap->isNull()) { if (bitmap && !bitmap->isNull()) {
CacheEntry* entry = new CacheEntry; scoped_ptr<CacheEntry> entry(new CacheEntry);
entry->bitmap.reset(new SkBitmap(*bitmap)); entry->bitmap.reset(new SkBitmap(*bitmap));
cache_.Put(fetcher->url(), entry); cache_.Put(fetcher->url(), std::move(entry));
} }
RemoveFetcher(fetcher); RemoveFetcher(fetcher);

@@ -96,7 +96,7 @@ class BitmapFetcherService : public KeyedService,
scoped_ptr<const SkBitmap> bitmap; scoped_ptr<const SkBitmap> bitmap;
}; };
base::OwningMRUCache<GURL, CacheEntry*> cache_; base::MRUCache<GURL, scoped_ptr<CacheEntry>> cache_;
// Current request ID to be used. // Current request ID to be used.
int current_request_id_; int current_request_id_;

@@ -26,10 +26,6 @@ const char kPeopleQueryPrefix[] = "people:";
} // namespace } // namespace
void WebserviceCache::CacheDeletor::operator()(const Payload& payload) {
delete payload.result;
}
WebserviceCache::WebserviceCache(content::BrowserContext* context) WebserviceCache::WebserviceCache(content::BrowserContext* context)
: cache_(Cache::NO_AUTO_EVICT), : cache_(Cache::NO_AUTO_EVICT),
cache_loaded_(false) { cache_loaded_(false) {
@@ -49,11 +45,11 @@ const CacheResult WebserviceCache::Get(QueryType type,
std::string typed_query = PrependType(type, query); std::string typed_query = PrependType(type, query);
Cache::iterator iter = cache_.Get(typed_query); Cache::iterator iter = cache_.Get(typed_query);
if (iter != cache_.end()) { if (iter != cache_.end()) {
if (base::Time::Now() - iter->second.time <= if (base::Time::Now() - iter->second->time <=
base::TimeDelta::FromMinutes(kWebserviceCacheTimeLimitInMinutes)) { base::TimeDelta::FromMinutes(kWebserviceCacheTimeLimitInMinutes)) {
return std::make_pair(FRESH, iter->second.result); return std::make_pair(FRESH, iter->second->result.get());
} else { } else {
return std::make_pair(STALE, iter->second.result); return std::make_pair(STALE, iter->second->result.get());
} }
} }
return std::make_pair(STALE, static_cast<base::DictionaryValue*>(NULL)); return std::make_pair(STALE, static_cast<base::DictionaryValue*>(NULL));
@@ -64,15 +60,17 @@ void WebserviceCache::Put(QueryType type,
scoped_ptr<base::DictionaryValue> result) { scoped_ptr<base::DictionaryValue> result) {
if (result) { if (result) {
std::string typed_query = PrependType(type, query); std::string typed_query = PrependType(type, query);
Payload payload(base::Time::Now(), result.release()); scoped_ptr<Payload> scoped_payload(
new Payload(base::Time::Now(), std::move(result)));
Payload* payload = scoped_payload.get();
cache_.Put(typed_query, payload); cache_.Put(typed_query, std::move(scoped_payload));
// If the cache isn't loaded yet, we're fine with losing queries since // If the cache isn't loaded yet, we're fine with losing queries since
// a 1000 entry cache should load really quickly so the chance of a user // a 1000 entry cache should load really quickly so the chance of a user
// already having typed a 3 character search before the cache has loaded is // already having typed a 3 character search before the cache has loaded is
// very unlikely. // very unlikely.
if (cache_loaded_) { if (cache_loaded_) {
data_store_->cached_dict()->Set(typed_query, DictFromPayload(payload)); data_store_->cached_dict()->Set(typed_query, DictFromPayload(*payload));
data_store_->ScheduleWrite(); data_store_->ScheduleWrite();
if (cache_.size() > kWebserviceCacheMaxSize) if (cache_.size() > kWebserviceCacheMaxSize)
TrimCache(); TrimCache();
@@ -89,16 +87,16 @@ void WebserviceCache::OnCacheLoaded(scoped_ptr<base::DictionaryValue>) {
!it.IsAtEnd(); !it.IsAtEnd();
it.Advance()) { it.Advance()) {
const base::DictionaryValue* payload_dict; const base::DictionaryValue* payload_dict;
Payload payload; scoped_ptr<Payload> payload(new Payload);
if (!it.value().GetAsDictionary(&payload_dict) || if (!it.value().GetAsDictionary(&payload_dict) ||
!payload_dict || !payload_dict ||
!PayloadFromDict(payload_dict, &payload)) { !PayloadFromDict(payload_dict, payload.get())) {
// In case we don't have a valid payload associated with a given query, // In case we don't have a valid payload associated with a given query,
// clean up that query from our data store. // clean up that query from our data store.
cleanup_keys.push_back(it.key()); cleanup_keys.push_back(it.key());
continue; continue;
} }
cache_.Put(it.key(), payload); cache_.Put(it.key(), std::move(payload));
} }
if (!cleanup_keys.empty()) { if (!cleanup_keys.empty()) {
@@ -125,7 +123,7 @@ bool WebserviceCache::PayloadFromDict(const base::DictionaryValue* dict,
// instead of returning the original reference. The new dictionary will be // instead of returning the original reference. The new dictionary will be
// owned by our MRU cache. // owned by our MRU cache.
*payload = Payload(base::Time::FromInternalValue(time_val), *payload = Payload(base::Time::FromInternalValue(time_val),
result->DeepCopy()); make_scoped_ptr(result->DeepCopy()));
return true; return true;
} }
@@ -163,4 +161,18 @@ std::string WebserviceCache::PrependType(
} }
} }
WebserviceCache::Payload::Payload(const base::Time& time,
scoped_ptr<base::DictionaryValue> result)
: time(time), result(std::move(result)) {}
WebserviceCache::Payload::Payload() = default;
WebserviceCache::Payload::~Payload() = default;
WebserviceCache::Payload& WebserviceCache::Payload::operator=(Payload&& other) {
time = std::move(other.time);
result = std::move(other.result);
return *this;
}
} // namespace app_list } // namespace app_list

@@ -64,22 +64,17 @@ class WebserviceCache : public KeyedService,
private: private:
struct Payload { struct Payload {
Payload(const base::Time& time, Payload(const base::Time& time, scoped_ptr<base::DictionaryValue> result);
const base::DictionaryValue* result) Payload();
: time(time), result(result) {} ~Payload();
Payload() {}
Payload& operator=(Payload&& other);
base::Time time; base::Time time;
const base::DictionaryValue* result; scoped_ptr<base::DictionaryValue> result;
}; };
class CacheDeletor { using Cache = base::MRUCache<std::string, scoped_ptr<Payload>>;
public:
void operator()(const Payload& payload);
};
typedef base::
MRUCacheBase<std::string, Payload, std::less<std::string>, CacheDeletor>
Cache;
// Callback for when the cache is loaded from the dictionary data store. // Callback for when the cache is loaded from the dictionary data store.
void OnCacheLoaded(scoped_ptr<base::DictionaryValue>); void OnCacheLoaded(scoped_ptr<base::DictionaryValue>);
@@ -87,8 +82,7 @@ class WebserviceCache : public KeyedService,
// Populates the payload parameter with the corresponding payload stored // Populates the payload parameter with the corresponding payload stored
// in the given dictionary. If the dictionary is invalid for any reason, // in the given dictionary. If the dictionary is invalid for any reason,
// this method will return false. // this method will return false.
bool PayloadFromDict(const base::DictionaryValue* dict, bool PayloadFromDict(const base::DictionaryValue* dict, Payload* payload);
Payload* payload);
// Returns a dictionary value for a given payload. The returned dictionary // Returns a dictionary value for a given payload. The returned dictionary
// will be owned by the data_store_ cached_dict, and freed on the destruction // will be owned by the data_store_ cached_dict, and freed on the destruction

@@ -9,8 +9,7 @@
namespace dom_distiller { namespace dom_distiller {
InMemoryContentStore::InMemoryContentStore(const int max_num_entries) InMemoryContentStore::InMemoryContentStore(const int max_num_entries)
: cache_(max_num_entries, CacheDeletor(this)) { : cache_(max_num_entries) {}
}
InMemoryContentStore::~InMemoryContentStore() { InMemoryContentStore::~InMemoryContentStore() {
// Clear the cache before destruction to ensure the CacheDeletor is not called // Clear the cache before destruction to ensure the CacheDeletor is not called
@@ -52,7 +51,7 @@ void InMemoryContentStore::LoadContent(
} }
scoped_ptr<DistilledArticleProto> distilled_article; scoped_ptr<DistilledArticleProto> distilled_article;
if (success) { if (success) {
distilled_article.reset(new DistilledArticleProto(it->second)); distilled_article.reset(new DistilledArticleProto(*it->second));
} else { } else {
distilled_article.reset(new DistilledArticleProto()); distilled_article.reset(new DistilledArticleProto());
} }
@@ -63,7 +62,9 @@ void InMemoryContentStore::LoadContent(
void InMemoryContentStore::InjectContent(const ArticleEntry& entry, void InMemoryContentStore::InjectContent(const ArticleEntry& entry,
const DistilledArticleProto& proto) { const DistilledArticleProto& proto) {
cache_.Put(entry.entry_id(), proto); cache_.Put(entry.entry_id(),
scoped_ptr<DistilledArticleProto, CacheDeletor>(
new DistilledArticleProto(proto), CacheDeletor(this)));
AddUrlToIdMapping(entry, proto); AddUrlToIdMapping(entry, proto);
} }
@@ -96,11 +97,12 @@ InMemoryContentStore::CacheDeletor::~CacheDeletor() {
} }
void InMemoryContentStore::CacheDeletor::operator()( void InMemoryContentStore::CacheDeletor::operator()(
const DistilledArticleProto& proto) { DistilledArticleProto* proto) {
// When InMemoryContentStore is deleted, the |store_| pointer becomes invalid, // When InMemoryContentStore is deleted, the |store_| pointer becomes invalid,
// but since the ContentMap is cleared in the InMemoryContentStore destructor, // but since the ContentMap is cleared in the InMemoryContentStore destructor,
// this should never be called after the destructor. // this should never be called after the destructor.
store_->EraseUrlToIdMapping(proto); store_->EraseUrlToIdMapping(*proto);
delete proto;
} }
} // namespace dom_distiller } // namespace dom_distiller

@@ -64,7 +64,7 @@ class InMemoryContentStore : public DistilledContentStore {
public: public:
explicit CacheDeletor(InMemoryContentStore* store); explicit CacheDeletor(InMemoryContentStore* store);
~CacheDeletor(); ~CacheDeletor();
void operator()(const DistilledArticleProto& proto); void operator()(DistilledArticleProto* proto);
private: private:
InMemoryContentStore* store_; InMemoryContentStore* store_;
@@ -75,10 +75,9 @@ class InMemoryContentStore : public DistilledContentStore {
void EraseUrlToIdMapping(const DistilledArticleProto& proto); void EraseUrlToIdMapping(const DistilledArticleProto& proto);
typedef base::MRUCacheBase<std::string, typedef base::MRUCache<std::string,
DistilledArticleProto, scoped_ptr<DistilledArticleProto, CacheDeletor>>
std::less<std::string>,
InMemoryContentStore::CacheDeletor>
ContentMap; ContentMap;
typedef base::hash_map<std::string, std::string> UrlMap; typedef base::hash_map<std::string, std::string> UrlMap;

@@ -28,9 +28,9 @@ LargeIconCache::~LargeIconCache() {}
void LargeIconCache::SetCachedResult( void LargeIconCache::SetCachedResult(
const GURL& url, const GURL& url,
const favicon_base::LargeIconResult& result) { const favicon_base::LargeIconResult& result) {
LargeIconCacheEntry* entry = new LargeIconCacheEntry; scoped_ptr<LargeIconCacheEntry> entry(new LargeIconCacheEntry);
entry->result = CloneLargeIconResult(result); entry->result = CloneLargeIconResult(result);
cache_.Put(url, entry); cache_.Put(url, std::move(entry));
} }
scoped_ptr<favicon_base::LargeIconResult> LargeIconCache::GetCachedResult( scoped_ptr<favicon_base::LargeIconResult> LargeIconCache::GetCachedResult(

@@ -47,7 +47,7 @@ class LargeIconCache : public KeyedService {
scoped_ptr<favicon_base::LargeIconResult> CloneLargeIconResult( scoped_ptr<favicon_base::LargeIconResult> CloneLargeIconResult(
const favicon_base::LargeIconResult& large_icon_result); const favicon_base::LargeIconResult& large_icon_result);
base::OwningMRUCache<GURL, LargeIconCacheEntry*> cache_; base::MRUCache<GURL, scoped_ptr<LargeIconCacheEntry>> cache_;
DISALLOW_COPY_AND_ASSIGN(LargeIconCache); DISALLOW_COPY_AND_ASSIGN(LargeIconCache);
}; };

@@ -7,21 +7,11 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
// The LRUCache delegate is called before an item is evicted from the cache.
@protocol LRUCacheDelegate
- (void)lruCacheWillEvictObject:(id<NSObject>)object;
@end
// This class implements a cache with a limited size. Once the cache reach its // This class implements a cache with a limited size. Once the cache reach its
// size limit, it will start to evict items in a Least Recently Used order // size limit, it will start to evict items in a Least Recently Used order
// (where the term "used" is determined in terms of query to the cache). // (where the term "used" is determined in terms of query to the cache).
@interface LRUCache : NSObject @interface LRUCache : NSObject
// The delegate of the LRUCache called when objects are evicted from the cache.
@property(nonatomic, assign) id<LRUCacheDelegate> delegate;
// The maximum amount of items that the cache can hold before starting to // The maximum amount of items that the cache can hold before starting to
// evict. The value 0 is used to signify that the cache can hold an unlimited // evict. The value 0 is used to signify that the cache can hold an unlimited
// amount of elements (i.e. never evicts). // amount of elements (i.e. never evicts).
@@ -43,18 +33,13 @@
// Adds the pair |key|, |obj| to the cache. If the value of the maxCacheSize // Adds the pair |key|, |obj| to the cache. If the value of the maxCacheSize
// property is non zero, the cache may evict an elements if the maximum cache // property is non zero, the cache may evict an elements if the maximum cache
// size is reached. If the |key| is already present in the cache, the value for // size is reached. If the |key| is already present in the cache, the value for
// that key is replaced by |object|. For any evicted object and if the delegate // that key is replaced by |object|.
// is
// non nil, it will receive a call to the lruCacheWillEvictObject: selector.
- (void)setObject:(id<NSObject>)object forKey:(NSObject*)key; - (void)setObject:(id<NSObject>)object forKey:(NSObject*)key;
// Remove the key, value pair corresponding to the given |key|. If the delegate // Remove the key, value pair corresponding to the given |key|.
// is non nil, it will receive a call to the lruCacheWillEvictObject: selector.
- (void)removeObjectForKey:(id<NSObject>)key; - (void)removeObjectForKey:(id<NSObject>)key;
// Remove all objects from the cache. For all evicted objects and if the // Remove all objects from the cache.
// delegate is non nil, it will receive a call to the lruCacheWillEvictObject:
// selector.
- (void)removeAllObjects; - (void)removeAllObjects;
// Returns the amount of items that the cache currently hold. // Returns the amount of items that the cache currently hold.

@@ -14,21 +14,6 @@
namespace { namespace {
class MRUCacheNSObjectDelegate {
public:
MRUCacheNSObjectDelegate(id<LRUCacheDelegate> delegate)
: delegate_(delegate) {}
MRUCacheNSObjectDelegate(const MRUCacheNSObjectDelegate& other) = default;
void operator()(const base::scoped_nsprotocol<id<NSObject>>& payload) const {
[delegate_ lruCacheWillEvictObject:payload.get()];
}
private:
id<LRUCacheDelegate> delegate_; // Weak.
};
struct NSObjectEqualTo { struct NSObjectEqualTo {
bool operator()(const base::scoped_nsprotocol<id<NSObject>>& obj1, bool operator()(const base::scoped_nsprotocol<id<NSObject>>& obj1,
const base::scoped_nsprotocol<id<NSObject>>& obj2) const { const base::scoped_nsprotocol<id<NSObject>>& obj2) const {
@@ -52,20 +37,17 @@ class NSObjectMRUCache
: public base::MRUCacheBase<base::scoped_nsprotocol<id<NSObject>>, : public base::MRUCacheBase<base::scoped_nsprotocol<id<NSObject>>,
base::scoped_nsprotocol<id<NSObject>>, base::scoped_nsprotocol<id<NSObject>>,
NSObjectHash, NSObjectHash,
MRUCacheNSObjectDelegate,
MRUCacheNSObjectHashMap> { MRUCacheNSObjectHashMap> {
private: private:
typedef base::MRUCacheBase<base::scoped_nsprotocol<id<NSObject>>, typedef base::MRUCacheBase<base::scoped_nsprotocol<id<NSObject>>,
base::scoped_nsprotocol<id<NSObject>>, base::scoped_nsprotocol<id<NSObject>>,
NSObjectHash, NSObjectHash,
MRUCacheNSObjectDelegate,
MRUCacheNSObjectHashMap> MRUCacheNSObjectHashMap>
ParentType; ParentType;
public: public:
NSObjectMRUCache(typename ParentType::size_type max_size, explicit NSObjectMRUCache(typename ParentType::size_type max_size)
const MRUCacheNSObjectDelegate& deletor) : ParentType(max_size) {}
: ParentType(max_size, deletor) {}
private: private:
DISALLOW_COPY_AND_ASSIGN(NSObjectMRUCache); DISALLOW_COPY_AND_ASSIGN(NSObjectMRUCache);
@@ -73,14 +55,10 @@ class NSObjectMRUCache
} // namespace } // namespace
@interface LRUCache ()<LRUCacheDelegate>
@end
@implementation LRUCache { @implementation LRUCache {
scoped_ptr<NSObjectMRUCache> _cache; scoped_ptr<NSObjectMRUCache> _cache;
} }
@synthesize delegate = _delegate;
@synthesize maxCacheSize = _maxCacheSize; @synthesize maxCacheSize = _maxCacheSize;
- (instancetype)init { - (instancetype)init {
@@ -92,8 +70,7 @@ class NSObjectMRUCache
self = [super init]; self = [super init];
if (self) { if (self) {
_maxCacheSize = maxCacheSize; _maxCacheSize = maxCacheSize;
MRUCacheNSObjectDelegate cacheDelegateDeletor(self); _cache.reset(new NSObjectMRUCache(self.maxCacheSize));
_cache.reset(new NSObjectMRUCache(self.maxCacheSize, cacheDelegateDeletor));
} }
return self; return self;
} }
@@ -131,10 +108,4 @@ class NSObjectMRUCache
return _cache->empty(); return _cache->empty();
} }
#pragma mark - Private
- (void)lruCacheWillEvictObject:(id<NSObject>)obj {
[self.delegate lruCacheWillEvictObject:obj];
}
@end @end

@@ -6,32 +6,10 @@
#import "ios/chrome/browser/snapshots/lru_cache.h" #import "ios/chrome/browser/snapshots/lru_cache.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
@interface LRUCacheTestDelegate : NSObject<LRUCacheDelegate>
@property(nonatomic, retain) id<NSObject> lastEvictedObject;
@property(nonatomic, assign) NSInteger evictedObjectsCount;
@end
@implementation LRUCacheTestDelegate
@synthesize lastEvictedObject = _lastEvictedObject;
@synthesize evictedObjectsCount = _evictedObjectsCount;
- (void)lruCacheWillEvictObject:(id<NSObject>)obj {
self.lastEvictedObject = obj;
++_evictedObjectsCount;
}
@end
namespace { namespace {
TEST(LRUCacheTest, Basic) { TEST(LRUCacheTest, Basic) {
base::scoped_nsobject<LRUCache> cache([[LRUCache alloc] initWithCacheSize:3]); base::scoped_nsobject<LRUCache> cache([[LRUCache alloc] initWithCacheSize:3]);
base::scoped_nsobject<LRUCacheTestDelegate> delegate(
[[LRUCacheTestDelegate alloc] init]);
[cache setDelegate:delegate];
base::scoped_nsobject<NSString> value1( base::scoped_nsobject<NSString> value1(
[[NSString alloc] initWithString:@"Value 1"]); [[NSString alloc] initWithString:@"Value 1"]);
@@ -51,8 +29,6 @@ TEST(LRUCacheTest, Basic) {
[cache setObject:value4 forKey:@"VALUE 4"]; [cache setObject:value4 forKey:@"VALUE 4"];
EXPECT_TRUE([cache count] == 3); EXPECT_TRUE([cache count] == 3);
EXPECT_TRUE([delegate evictedObjectsCount] == 1);
EXPECT_TRUE([delegate lastEvictedObject] == value1.get());
// Check LRU behaviour, the value least recently added value should have been // Check LRU behaviour, the value least recently added value should have been
// evicted. // evicted.

@@ -40,7 +40,7 @@ ScopedSSL_SESSION SSLClientSessionCacheOpenSSL::Lookup(
CacheEntryMap::iterator iter = cache_.Get(cache_key); CacheEntryMap::iterator iter = cache_.Get(cache_key);
if (iter == cache_.end()) if (iter == cache_.end())
return nullptr; return nullptr;
if (IsExpired(iter->second, clock_->Now())) { if (IsExpired(iter->second.get(), clock_->Now())) {
cache_.Erase(iter); cache_.Erase(iter);
return nullptr; return nullptr;
} }
@@ -52,12 +52,12 @@ void SSLClientSessionCacheOpenSSL::Insert(const std::string& cache_key,
base::AutoLock lock(lock_); base::AutoLock lock(lock_);
// Make a new entry. // Make a new entry.
CacheEntry* entry = new CacheEntry; scoped_ptr<CacheEntry> entry(new CacheEntry);
entry->session.reset(SSL_SESSION_up_ref(session)); entry->session.reset(SSL_SESSION_up_ref(session));
entry->creation_time = clock_->Now(); entry->creation_time = clock_->Now();
// Takes ownership. // Takes ownership.
cache_.Put(cache_key, entry); cache_.Put(cache_key, std::move(entry));
} }
void SSLClientSessionCacheOpenSSL::Flush() { void SSLClientSessionCacheOpenSSL::Flush() {
@@ -88,7 +88,7 @@ void SSLClientSessionCacheOpenSSL::FlushExpiredSessions() {
base::Time now = clock_->Now(); base::Time now = clock_->Now();
CacheEntryMap::iterator iter = cache_.begin(); CacheEntryMap::iterator iter = cache_.begin();
while (iter != cache_.end()) { while (iter != cache_.end()) {
if (IsExpired(iter->second, now)) { if (IsExpired(iter->second.get(), now)) {
iter = cache_.Erase(iter); iter = cache_.Erase(iter);
} else { } else {
++iter; ++iter;

@@ -66,11 +66,7 @@ class NET_EXPORT SSLClientSessionCacheOpenSSL {
}; };
using CacheEntryMap = using CacheEntryMap =
base::MRUCacheBase<std::string, base::HashingMRUCache<std::string, scoped_ptr<CacheEntry>>;
CacheEntry*,
std::hash<std::string>,
base::MRUCachePointerDeletor<CacheEntry*>,
base::MRUCacheHashMap>;
// Returns true if |entry| is expired as of |now|. // Returns true if |entry| is expired as of |now|.
bool IsExpired(CacheEntry* entry, const base::Time& now); bool IsExpired(CacheEntry* entry, const base::Time& now);

@@ -66,11 +66,7 @@ class DrmOverlayValidator {
ScanoutBufferGenerator* buffer_generator_; // Not owned. ScanoutBufferGenerator* buffer_generator_; // Not owned.
// List of all configurations which have been validated. // List of all configurations which have been validated.
base::MRUCacheBase<OverlayPlaneList, base::MRUCache<OverlayPlaneList, OverlayHintsList> overlay_hints_cache_;
OverlayHintsList,
std::less<OverlayPlaneList>,
base::MRUCacheNullDeletor<OverlayHintsList>>
overlay_hints_cache_;
DISALLOW_COPY_AND_ASSIGN(DrmOverlayValidator); DISALLOW_COPY_AND_ASSIGN(DrmOverlayValidator);
}; };

@@ -55,11 +55,7 @@ class DrmOverlayManager : public OverlayManagerOzone {
// List of all OverlayCheck_Params which have been validated in GPU side. // List of all OverlayCheck_Params which have been validated in GPU side.
// Value is set to true if we are waiting for validation results from GPU. // Value is set to true if we are waiting for validation results from GPU.
base::MRUCacheBase<std::vector<OverlayCheck_Params>, base::MRUCache<std::vector<OverlayCheck_Params>, bool> cache_;
bool,
std::less<std::vector<OverlayCheck_Params>>,
base::MRUCacheNullDeletor<bool>>
cache_;
DISALLOW_COPY_AND_ASSIGN(DrmOverlayManager); DISALLOW_COPY_AND_ASSIGN(DrmOverlayManager);
}; };