New issue
Advanced search Search tips

Issue 631467 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Dropping the last handle to a doomed cache can delete a newer cache object with the same name

Project Member Reported by jkarlin@chromium.org, Jul 26 2016

Issue description

Steps to reproduce:

// 1. Create cache A and hang onto the handle
// 2. Doom the cache
// 3. Create cache B (with the same name)
// 4. Drop handle to A
// 5. Use B

The problem is the way that dropped handles are processed. It first checks to see if an active cache has the name, and if so drops that cache object. If not it checks doomed caches for the cache pointer. It should check doomed caches first.

 
Cc: cmumford@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/956d7aca53f56b5cff228af586ea2e5f895f2e9b

commit 956d7aca53f56b5cff228af586ea2e5f895f2e9b
Author: jkarlin <jkarlin@chromium.org>
Date: Tue Jul 26 17:58:20 2016

[CacheStorage] Check doomed caches first when dropping cache handles

When the last cache handle is dropped, first check if it's a doomed cache
object before removing the cache from the active cache map. Previously we were
checking the other way around, which caused the active cache to be deleted (in
the event that a new cache was created with the same name) instead of the doomed
one.

BUG= 631467 

Review-Url: https://codereview.chromium.org/2179353003
Cr-Commit-Position: refs/heads/master@{#407850}

[modify] https://crrev.com/956d7aca53f56b5cff228af586ea2e5f895f2e9b/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/956d7aca53f56b5cff228af586ea2e5f895f2e9b/content/browser/cache_storage/cache_storage_manager_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-Request-53
Requesting merge to M53 beta as this is a crash fix.

Comment 5 by dimu@chromium.org, Aug 10 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)

Comment 6 by gov...@chromium.org, Aug 11 2016

Please merge your change to M53 branch 2785 ASAP (latest before 5:00 PM PT, Friday 08/12) so we can take it in for next week beta. Thank you.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 11 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f11a8f8d45b8f49980f3eccf71de1f75618ae07

commit 6f11a8f8d45b8f49980f3eccf71de1f75618ae07
Author: Josh Karlin <jkarlin@chromium.org>
Date: Thu Aug 11 14:26:51 2016

This is a merge into M53 (branch 2785)

[CacheStorage] Check doomed caches first when dropping cache handles

When the last cache handle is dropped, first check if it's a doomed cache
object before removing the cache from the active cache map. Previously we were
checking the other way around, which caused the active cache to be deleted (in
the event that a new cache was created with the same name) instead of the doomed
one.

BUG= 631467 

Review-Url: https://codereview.chromium.org/2179353003
Cr-Commit-Position: refs/heads/master@{#407850}
(cherry picked from commit 956d7aca53f56b5cff228af586ea2e5f895f2e9b)

[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 

Review-Url: https://codereview.chromium.org/2111243003
Cr-Commit-Position: refs/heads/master@{#403837}
(cherry picked from commit 0903d9b86a61912208efe95f24d0c79099ebf371)

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

Cr-Commit-Position: refs/branch-heads/2785@{#562}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/content/browser/cache_storage/cache_storage.h
[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-storage.js
[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/cache-storage-expected.txt
[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-storage-expected.txt
[modify] https://crrev.com/6f11a8f8d45b8f49980f3eccf71de1f75618ae07/third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-storage-expected.txt

Sign in to add a comment