[CacheStorage] Keep deleted caches alive until last reference is gone

[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps
the CacheStorageCache alive, waiting to delete it from disk until after the
last cache handle has been dropped. This change is to conform with the
spec.

BUG=618646
Committed: https://crrev.com/0903d9b86a61912208efe95f24d0c79099ebf371
Cr-Commit-Position: refs/heads/master@{#403837}

[jkarlin, 2016-07-01 16:26:35]

Description was changed from

==========
[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps the
CacheStorageCache alive, waiting to delete it from disk until after the last
cache handle has been dropped. This change is to conform with the spec.

BUG=618646
==========

to

==========
[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps
the CacheStorageCache alive, waiting to delete it from disk until after the
last cache handle has been dropped. This change is to conform with the 
spec.

BUG=618646
==========

[jkarlin, 2016-07-01 18:47:15]

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

[jkarlin, 2016-07-01 18:47:15]

nhiroki@ PTAL, thanks!

[nhiroki, 2016-07-04 04:19:34]

lgtm

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

https://codereview.chromium.org/2111243003/diff/40001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:213: void StoreCacheHandle(const
std::string& cache_name,
It looks like no one calls this function.

https://codereview.chromium.org/2111243003/diff/40001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:854:
weak_factory_.GetWeakPtr()));
OPTIONAL: Are we able to remove a callback parameter from CleanUpDeletedCache()?
Apparently, CleanUpDeletedCache() is called only here and a callback does
nothing other than DCHECK_CURRENTLY_ON().

[nhiroki, 2016-07-04 04:22:13]

> This change is to conform with the spec.

Should we have a layout test (w3c test) for this behavior?

[jkarlin, 2016-07-06 00:35:12]

Patchset #4 (id:60001) has been deleted

[jkarlin, 2016-07-06 00:35:33]

Also added layout test. PTAL, thanks!

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

https://codereview.chromium.org/2111243003/diff/40001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:213: void StoreCacheHandle(const
std::string& cache_name,
On 2016/07/04 04:19:33, nhiroki wrote:
> It looks like no one calls this function.

Thanks. I meant to delete that earlier. Done.

https://codereview.chromium.org/2111243003/diff/40001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:854:
weak_factory_.GetWeakPtr()));
On 2016/07/04 04:19:33, nhiroki wrote:
> OPTIONAL: Are we able to remove a callback parameter from
CleanUpDeletedCache()?
> Apparently, CleanUpDeletedCache() is called only here and a callback does
> nothing other than DCHECK_CURRENTLY_ON().

Good point. Done.

[nhiroki, 2016-07-06 00:59:18]

lgtm

[jkarlin, 2016-07-06 01:23:36]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2016-07-06 01:23:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 02:04:32]

Description was changed from

==========
[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps
the CacheStorageCache alive, waiting to delete it from disk until after the
last cache handle has been dropped. This change is to conform with the 
spec.

BUG=618646
==========

to

==========
[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps
the CacheStorageCache alive, waiting to delete it from disk until after the
last cache handle has been dropped. This change is to conform with the 
spec.

BUG=618646
==========

[commit-bot: I haz the power, 2016-07-06 02:04:33]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-06 02:04:43]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-06 02:05:58]

Description was changed from

==========
[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps
the CacheStorageCache alive, waiting to delete it from disk until after the
last cache handle has been dropped. This change is to conform with the 
spec.

BUG=618646
==========

to

==========
[CacheStorage] Keep deleted caches alive until last reference is gone

Currently once a cache is deleted via CacheStorage::Delete the associated
CacheStorageCache is closed and subsequent calls to it fail. This CL keeps
the CacheStorageCache alive, waiting to delete it from disk until after the
last cache handle has been dropped. This change is to conform with the
spec.

BUG=618646

Committed: https://crrev.com/0903d9b86a61912208efe95f24d0c79099ebf371
Cr-Commit-Position: refs/heads/master@{#403837}
==========

[commit-bot: I haz the power, 2016-07-06 02:05:59]

Patchset 4 (id:??) landed as
https://crrev.com/0903d9b86a61912208efe95f24d0c79099ebf371
Cr-Commit-Position: refs/heads/master@{#403837}