New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 639034 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Manually deleting caches from Cache Storage causes errors putting entries into a cache with the same name in the future

Project Member Reported by aaronsn@google.com, Aug 18 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce the problem:
1. Create a Cache in Cache Storage.
2. Manually delete the cache in Developer Tools > Application > Cache Storage.
3. Create a Cache with the same name.
4. Try to put entries into the Cache, and it will fail with "DOMException: Entry was not found."

The following code should reproduce. Run it once, then manually delete the cache, then run it again.

fetch(new Request('https://www.google.com/')).then(function(response) {
  caches.open('ExampleCache').then(function(cache) {
    cache.put('google', response);
  });
});

What is the expected behavior?
It shouldn't cause an error when you manually delete a cache using dev tools and then try to use it again.

What went wrong?
This error was thrown:

DOMException: Entry was not found.

Did this work before? N/A 

Chrome version: 52.0.2743.116  Channel: stable
OS Version: 
Flash Version: Shockwave Flash 22.0 r0
 

Comment 1 by aaronsn@google.com, Aug 18 2016

The bug report tool was acting up and submitted the bug twice. The duplicate is  issue 639036 
Cc: tkonch...@chromium.org
 Issue 639036  has been merged into this issue.
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 19 2016

Labels: Hotlist-Google
Components: Blink>Storage>CacheStorage

Comment 5 by jsb...@chromium.org, Aug 24 2016

Cc: jkarlin@chromium.org cmumford@chromium.org
Status: Available (was: Unconfirmed)
In tip-of-tree (~54) I don't see an error at step 4. But if I "Refresh Caches" I don't see the cache reappear.

1. Run; caches.open('ExampleCache').then(c => c.put(new Request('https://example.com'), new Response('body'))).then(r => console.log('ok')).catch(e => console.warn(e))
2. Developer Tools > Application > Cache Storage, find ExampleCache, right-click and delete
3. Run; caches.open('ExampleCache').then(c => c.put(new Request('https://example.com'), new Response('body'))).then(r => console.log('ok')).catch(e => console.warn(e))
4. Developer Tools > Application > Cache Storage, right click and Refresh Caches - does not appear

Also (after reloading page):

1. Run: caches.open('ExampleCache').then(c => c.put(new Request('https://example.com'), new Response('body'))).then(r => console.log('ok')).catch(e => console.warn(e)) // ok
2. caches.keys().then(r => console.log(r)) // list includes "ExampleCache"
3. caches.has('ExampleCache').then(r => console.log(r)) // true

(so far so good)

4. Developer Tools > Application > Cache Storage, find ExampleCache, right-click and delete
5. caches.keys().then(r => console.log(r)) // list does NOT include "ExampleCache"
6. caches.has('ExampleCache').then(r => console.log(r))

Expected: false
Actual: true

7. Run: caches.open('ExampleCache').then(c => c.put(new Request('https://example.com'), new Response('body'))).then(r => console.log('ok')).catch(e => console.warn(e)) // ok
8. caches.keys().then(r => console.log(r))

Expected: includes ExampleCache
Actual: does not include ExampleCache

9. caches.has('ExampleCache').then(r => console.log(r)) // true

Cc: -jkarlin@chromium.org
Labels: -Pri-2 OS-All Pri-1
Owner: jkarlin@chromium.org
Status: Started (was: Available)
Thanks for the report. I can reproduce. 

The issue is that CacheStorage.cpp is returning the deleted cache. CacheStorage.cpp maintains an internal map of caches in m_nameToCacheMap to fast-return subsequent calls to open the same cache. It doesn't expect other clients (e.g., dev tools) to delete the cache. After its deleted, and the second time you call open and put, it opens the old cache and the put fails because the cache is deleted (we fixed the issue of deleted caches not functioning in M53, see  issue 618646 ). 

This appears to be all sorts of broken. Each thread (worker, document, serviceworker) will have its own CacheStorage instance, let alone dev tools. Looks like we need to remove this code.

Another thing of note is that CacheStorage.cpp holds onto the cache in the map until it's deleted. It should let it go when it's no longer in use. But that's moot I suppose since we're removing it.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 24 2016

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

commit db792ded02006fe0e9b4230c1b904c0560302007
Author: jkarlin <jkarlin@chromium.org>
Date: Wed Aug 24 21:15:38 2016

[CacheStorage] Don't store open caches in Blink

Blink objects were storing cache objects so that they could be returned quickly
on subsequent open calls. This is problematic as there can be many contexts
(different windows, workers, service workers, and dev tools) opening/deleting
caches. If one of them deletes the cache then all of the others need to be
invalidated. Rather than synchronize the various contexts, for now just remove
the stored caches from Blink.

BUG= 639034 

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

[modify] https://crrev.com/db792ded02006fe0e9b4230c1b904c0560302007/third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/cache-storage-expected.txt
[modify] https://crrev.com/db792ded02006fe0e9b4230c1b904c0560302007/third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-storage-expected.txt
[modify] https://crrev.com/db792ded02006fe0e9b4230c1b904c0560302007/third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-storage-expected.txt
[modify] https://crrev.com/db792ded02006fe0e9b4230c1b904c0560302007/third_party/WebKit/Source/modules/cachestorage/CacheStorage.cpp
[modify] https://crrev.com/db792ded02006fe0e9b4230c1b904c0560302007/third_party/WebKit/Source/modules/cachestorage/CacheStorage.h

Status: Fixed (was: Started)

Sign in to add a comment