[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.

BUG=617683
Committed: https://crrev.com/fdde54f517cf14382dbeb4217a9e647563a2e671
Cr-Commit-Position: refs/heads/master@{#401305}

[jkarlin, 2016-06-10 19:51:34]

Description was changed from

==========
[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.


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

to

==========
[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.


BUG=617683
==========

[jkarlin, 2016-06-11 01:09:30]

jkarlin@chromium.org changed reviewers:
+ horo@chromium.org

[jkarlin, 2016-06-11 01:09:31]

horo: PTAL at everything. Thanks!

[jkarlin, 2016-06-11 01:10:47]

jkarlin@chromium.org changed reviewers:
+ nhiroki@chromium.org

[jkarlin, 2016-06-11 01:10:47]

Whoops, that wasn't the right assignment. Let me try again:

nhiroki@ PTAL at everything.
horo@ PTAL at service_worker/

Thanks!

[nhiroki, 2016-06-14 05:30:25]

lgtm

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage.h (right):

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.h:129: // Return a
CacheStorageCacheHandle for the given name if the name is known.
nit: s/Return/Returns/

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.h:131: // CacheStorageCache has been
deleted, creates a new one.
Can you squash this line into the previous line?

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache_handle.cc (right):

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache_handle.cc:25: if
(cache_storage_cache_)
We should not handle the DCHECK failure case:

"A consequence of this is that you should not handle DCHECK() failures, even if
failure would result in a crash."
http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-

[horo, 2016-06-14 06:31:13]

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage.cc (right):

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:931: void
CacheStorage::AddCacheHandleRef(CacheStorageCache* cache) {
Add DCHECK_CURRENTLY_ON().

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:941: void
CacheStorage::DropCacheHandleRef(CacheStorageCache* cache) {
Add DCHECK_CURRENTLY_ON().

[jkarlin, 2016-06-14 06:41:07]

jkarlin@chromium.org changed reviewers:
+ jochen@chromium.org

[jkarlin, 2016-06-14 06:41:09]

Thanks. PTAL.

jochen@ PTAL at content/browser/render_host/ for owners.

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage.cc (right):

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:931: void
CacheStorage::AddCacheHandleRef(CacheStorageCache* cache) {
On 2016/06/14 06:31:12, horo (slow till June 20) wrote:
> Add DCHECK_CURRENTLY_ON().

Done.

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:941: void
CacheStorage::DropCacheHandleRef(CacheStorageCache* cache) {
On 2016/06/14 06:31:12, horo (slow till June 20) wrote:
> Add DCHECK_CURRENTLY_ON().

Done.

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage.h (right):

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.h:129: // Return a
CacheStorageCacheHandle for the given name if the name is known.
On 2016/06/14 05:30:24, nhiroki (slow until Jun 20) wrote:
> nit: s/Return/Returns/

Done.

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.h:131: // CacheStorageCache has been
deleted, creates a new one.
On 2016/06/14 05:30:24, nhiroki (slow until Jun 20) wrote:
> Can you squash this line into the previous line?

Done.

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
File content/browser/cache_storage/cache_storage_cache_handle.cc (right):

https://codereview.chromium.org/2056983004/diff/60001/content/browser/cache_s...
content/browser/cache_storage/cache_storage_cache_handle.cc:25: if
(cache_storage_cache_)
On 2016/06/14 05:30:24, nhiroki (slow until Jun 20) wrote:
> We should not handle the DCHECK failure case:
> 
> "A consequence of this is that you should not handle DCHECK() failures, even
if
> failure would result in a crash."
>
http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-

Good catch, thanks! Done.

[horo, 2016-06-14 07:27:21]

lgtm

[jkarlin, 2016-06-15 19:13:59]

jochen@ PTAL, thanks!

[jochen (gone - plz use gerrit), 2016-06-21 07:49:54]

lgtm

[jkarlin, 2016-06-22 16:43:04]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2016-06-22 16:43:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
nhiroki@chromium.org, jochen@chromium.org, horo@chromium.org
Link to the patchset: https://codereview.chromium.org/2056983004/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-06-22 16:43:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2056983004/100001

[commit-bot: I haz the power, 2016-06-22 16:49:28]

Description was changed from

==========
[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.


BUG=617683
==========

to

==========
[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.


BUG=617683
==========

[commit-bot: I haz the power, 2016-06-22 16:49:29]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-22 16:52:00]

Description was changed from

==========
[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.


BUG=617683
==========

to

==========
[CacheStorage] Give ownership of all CacheStorageCaches to CacheStorage

This Cl removes ref counting on CacheStorageCache. Caches are now
owned by CacheStorage. CacheStorage hands out handles to the cache
to clients. The handles are ref counted and CacheStorage will delete
the cache in its own destructor or when the last cache handle is
deleted.

BUG=617683

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

[commit-bot: I haz the power, 2016-06-22 16:52:02]

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