[CacheStorage] Deleting a cache could delete the wrong directory

[CacheStorage] Deleting a cache could delete the wrong directory

The CacheStorageCache::CacheLoader maps cache names to paths. The problem is
that as of recent changes, it's possible to have two caches with the same name
exist simultaneously (the doomed one and a new one). If you then delete the
doomed one from disk, the CacheLoader will delete the new one's directory
instead since it maps name to path.

This CL fixes the above problem by keeping track of doomed caches and their
paths separately from live caches.

BUG=630036
Committed: https://crrev.com/7680d8569c891a84c9eff3aa21d06c3584339965
Cr-Commit-Position: refs/heads/master@{#407920}

[jkarlin, 2016-07-26 16:56:33]

The CQ bit was checked by jkarlin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 16:56:46]

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

[jkarlin, 2016-07-26 17:24:39]

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

[jkarlin, 2016-07-26 17:24:40]

jsbell@ PTAL at everything, thanks!

[commit-bot: I haz the power, 2016-07-26 17:37:49]

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

[commit-bot: I haz the power, 2016-07-26 17:37:50]

Dry run: This issue passed the CQ dry run.

[jsbell, 2016-07-26 17:43:01]

lgtm

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

https://codereview.chromium.org/2186433004/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:130:
std::unique_ptr<CacheStorageCacheHandle> cache_handle){};
nit: space between () and {}

https://codereview.chromium.org/2186433004/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:135:
std::unique_ptr<CacheStorageCacheHandle> cache_handle){};
ditto

[jkarlin, 2016-07-26 17:49:54]

The CQ bit was checked by jkarlin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 17:50:20]

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

[jkarlin, 2016-07-26 17:50:22]

Thanks!

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

https://codereview.chromium.org/2186433004/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:130:
std::unique_ptr<CacheStorageCacheHandle> cache_handle){};
On 2016/07/26 17:43:01, jsbell wrote:
> nit: space between () and {}

Looks like a clang-format bug. Filed https://crbug.com/631531 and will override
clang-format for now.

https://codereview.chromium.org/2186433004/diff/20001/content/browser/cache_s...
content/browser/cache_storage/cache_storage.cc:135:
std::unique_ptr<CacheStorageCacheHandle> cache_handle){};
On 2016/07/26 17:43:01, jsbell wrote:
> ditto

Done.

[jkarlin, 2016-07-26 17:59:20]

The CQ bit was unchecked by jkarlin@chromium.org

[jkarlin, 2016-07-26 17:59:27]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2016-07-26 17:59:27]

The patchset sent to the CQ was uploaded after l-g-t-m from jsbell@chromium.org
Link to the patchset: https://codereview.chromium.org/2186433004/#ps40001
(title: "formatting")

[commit-bot: I haz the power, 2016-07-26 17:59:51]

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-26 18:03:13]

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

[commit-bot: I haz the power, 2016-07-26 18:03:14]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[jkarlin, 2016-07-26 20:35:53]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2016-07-26 20:35:53]

The patchset sent to the CQ was uploaded after l-g-t-m from jsbell@chromium.org
Link to the patchset: https://codereview.chromium.org/2186433004/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-26 20:36:42]

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-26 21:26:26]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-26 21:28:03]

Description was changed from

==========
[CacheStorage] Deleting a cache could delete the wrong directory

The CacheStorageCache::CacheLoader maps cache names to paths. The problem is
that as of recent changes, it's possible to have two caches with the same name
exist simultaneously (the doomed one and a new one). If you then delete the
doomed one from disk, the CacheLoader will delete the new one's directory
instead since it maps name to path.

This CL fixes the above problem by keeping track of doomed caches and their
paths separately from live caches.

BUG=630036
==========

to

==========
[CacheStorage] Deleting a cache could delete the wrong directory

The CacheStorageCache::CacheLoader maps cache names to paths. The problem is
that as of recent changes, it's possible to have two caches with the same name
exist simultaneously (the doomed one and a new one). If you then delete the
doomed one from disk, the CacheLoader will delete the new one's directory
instead since it maps name to path.

This CL fixes the above problem by keeping track of doomed caches and their
paths separately from live caches.

BUG=630036

Committed: https://crrev.com/7680d8569c891a84c9eff3aa21d06c3584339965
Cr-Commit-Position: refs/heads/master@{#407920}
==========

[commit-bot: I haz the power, 2016-07-26 21:28:06]

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