base: Remove OwningMRUCache in favor of scoped_ptrs in MRUCache

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
Committed: https://crrev.com/aaab4d14f3b0266d774592136a1f097cd75c923c
Cr-Commit-Position: refs/heads/master@{#380239}

[vmpstr, 2016-03-07 21:09:43]

Description was changed from

==========
base: Remove OwningMRUCache in favor of scoped_ptrs in MRUCache

BUG=
==========

to

==========
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.
==========

[vmpstr, 2016-03-07 21:09:44]

vmpstr@chromium.org changed reviewers:
+ danakj@chromium.org

[vmpstr, 2016-03-07 21:10:28]

Please take a look.

[danakj, 2016-03-07 21:27:10]

base LGTM

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
File base/containers/mru_cache.h (right):

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache.h:215: typedef MRUCacheBase<KeyType, PayloadType,
std::less<KeyType>> ParentType;
can you change "typedef" to "using" when you touch them?

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache.h:241: typedef MRUCacheBase<KeyType, PayloadType,
HashType, MRUCacheHashMap>
ditto

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
File base/containers/mru_cache_unittest.cc (right):

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache_unittest.cc:191: typedef base::MRUCache<int,
scoped_ptr<CachedItem>> Cache;
dittos

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache_unittest.cc:234: typedef base::MRUCache<int,
scoped_ptr<CachedItem>> Cache;
dittoss

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/search/common/webservice_cache.cc (right):

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/search/common/webservice_cache.cc:125: payload->time
= base::Time::FromInternalValue(time_val);
Might be nice to call the constructor here still like before, ensures new fields
don't get missed

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/search/common/webservice_cache.h (right):

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/search/common/webservice_cache.h:75: typedef
base::MRUCache<std::string, scoped_ptr<Payload>> Cache;
using

https://codereview.chromium.org/1763273002/diff/110001/net/ssl/ssl_client_ses...
File net/ssl/ssl_client_session_cache_openssl.cc (right):

https://codereview.chromium.org/1763273002/diff/110001/net/ssl/ssl_client_ses...
net/ssl/ssl_client_session_cache_openssl.cc:55: scoped_ptr<CacheEntry> entry =
make_scoped_ptr(new CacheEntry);
nit: scoped_ptr<CacheEntry> entry(new CacheEntry) ?

[vmpstr, 2016-03-07 21:45:33]

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
File base/containers/mru_cache.h (right):

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache.h:215: typedef MRUCacheBase<KeyType, PayloadType,
std::less<KeyType>> ParentType;
On 2016/03/07 21:27:10, danakj wrote:
> can you change "typedef" to "using" when you touch them?

Done.

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache.h:241: typedef MRUCacheBase<KeyType, PayloadType,
HashType, MRUCacheHashMap>
On 2016/03/07 21:27:10, danakj wrote:
> ditto

Done.

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
File base/containers/mru_cache_unittest.cc (right):

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache_unittest.cc:191: typedef base::MRUCache<int,
scoped_ptr<CachedItem>> Cache;
On 2016/03/07 21:27:10, danakj wrote:
> dittos

Done.

https://codereview.chromium.org/1763273002/diff/110001/base/containers/mru_ca...
base/containers/mru_cache_unittest.cc:234: typedef base::MRUCache<int,
scoped_ptr<CachedItem>> Cache;
On 2016/03/07 21:27:10, danakj wrote:
> dittoss

Done.

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/search/common/webservice_cache.cc (right):

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/search/common/webservice_cache.cc:125: payload->time
= base::Time::FromInternalValue(time_val);
On 2016/03/07 21:27:10, danakj wrote:
> Might be nice to call the constructor here still like before, ensures new
fields
> don't get missed

Done.

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/search/common/webservice_cache.h (right):

https://codereview.chromium.org/1763273002/diff/110001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/search/common/webservice_cache.h:75: typedef
base::MRUCache<std::string, scoped_ptr<Payload>> Cache;
On 2016/03/07 21:27:10, danakj wrote:
> using

Done.

https://codereview.chromium.org/1763273002/diff/110001/net/ssl/ssl_client_ses...
File net/ssl/ssl_client_session_cache_openssl.cc (right):

https://codereview.chromium.org/1763273002/diff/110001/net/ssl/ssl_client_ses...
net/ssl/ssl_client_session_cache_openssl.cc:55: scoped_ptr<CacheEntry> entry =
make_scoped_ptr(new CacheEntry);
On 2016/03/07 21:27:10, danakj wrote:
> nit: scoped_ptr<CacheEntry> entry(new CacheEntry) ?

Done.

[vmpstr, 2016-03-07 21:46:23]

vmpstr@chromium.org changed reviewers:
+ agl@chromium.org, benwells@chromium.org, dfalcantara@chromium.org,
dnicoara@chromium.org, droger@chromium.org, nyquist@chromium.org

[vmpstr, 2016-03-07 21:46:24]

+dfalcantara@chromium.org for chrome/browser/bitmap_fetcher/
+benwells@chromium.org for chrome/browser/ui/app_list/
+nyquist@chromium.org for components/dom_distinner/
+droger@chromium.org for ios/chrome/
+agl@chromium.org for net/
+dnicoara@chromium.org ui/ozone/platform/drm/

Please take a look.

[dnicoara, 2016-03-07 21:52:16]

ui/ozone/platform/drm lgtm

[gone, 2016-03-07 22:01:32]

bitmap_fetcher lgtm

https://codereview.chromium.org/1763273002/diff/150001/chrome/browser/bitmap_...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc (right):

https://codereview.chromium.org/1763273002/diff/150001/chrome/browser/bitmap_...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc:187:
scoped_ptr<CacheEntry> entry(new CacheEntry);
nit: make_scoped_ptr?

[danakj, 2016-03-07 22:02:42]

https://codereview.chromium.org/1763273002/diff/150001/chrome/browser/bitmap_...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc (right):

https://codereview.chromium.org/1763273002/diff/150001/chrome/browser/bitmap_...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc:187:
scoped_ptr<CacheEntry> entry(new CacheEntry);
On 2016/03/07 22:01:32, dfalcantara wrote:
> nit: make_scoped_ptr?

lol. I asked the opposite in the previous patch set, as it makes the line way
longer.

[gone, 2016-03-07 22:03:33]

https://codereview.chromium.org/1763273002/diff/150001/chrome/browser/bitmap_...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc (right):

https://codereview.chromium.org/1763273002/diff/150001/chrome/browser/bitmap_...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc:187:
scoped_ptr<CacheEntry> entry(new CacheEntry);
On 2016/03/07 22:02:42, danakj wrote:
> On 2016/03/07 22:01:32, dfalcantara wrote:
> > nit: make_scoped_ptr?
> 
> lol. I asked the opposite in the previous patch set, as it makes the line way
> longer.

lol!  I'm fine either way.

[vmpstr, 2016-03-08 00:43:36]

Description was changed from

==========
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.
==========

to

==========
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
==========

[nyquist, 2016-03-08 09:00:20]

components/dom_distiller lgtm

[droger, 2016-03-08 13:41:56]

lgtm

[vmpstr, 2016-03-08 18:52:35]

vmpstr@chromium.org changed reviewers:
+ calamity@chromium.org, gavinp@chromium.org

[vmpstr, 2016-03-08 18:52:36]

Ping for owners, and +more peeps in case owners are busy.

[agl, 2016-03-08 19:13:59]

LGTM for net/

[vmpstr, 2016-03-08 22:43:37]

vmpstr@chromium.org changed reviewers:
+ tapted@chromium.org

[benwells, 2016-03-09 02:40:18]

On 2016/03/08 18:52:36, vmpstr wrote:
> Ping for owners, and +more peeps in case owners are busy.

looking now...

[benwells, 2016-03-09 02:54:30]

lgtm

[vmpstr, 2016-03-09 18:48:01]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2016-03-09 18:48:02]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/1763273002/#ps150001
(title: "rebase + fix")

[commit-bot: I haz the power, 2016-03-09 18:48:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1763273002/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1763273002/150001

[commit-bot: I haz the power, 2016-03-09 20:33:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-09 20:33:13]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[vmpstr, 2016-03-09 20:41:12]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-03-09 20:41:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1763273002/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1763273002/150001

[commit-bot: I haz the power, 2016-03-09 22:08:55]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-03-09 22:08:57]

Committed patchset #9 (id:150001)

[commit-bot: I haz the power, 2016-03-09 22:47:50]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/aaab4d14f3b0266d774592136a1f097cd75c923c
Cr-Commit-Position: refs/heads/master@{#380239}
==========

[commit-bot: I haz the power, 2016-03-09 22:47:51]

Patchset 9 (id:??) landed as
https://crrev.com/aaab4d14f3b0266d774592136a1f097cd75c923c
Cr-Commit-Position: refs/heads/master@{#380239}