1110491 - re-open Service Worker Cache objects if underlying quota directory is wiped

Status: RESOLVED WONTFIX

(Core :: Storage: Cache API, defect, P5)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently there is a bug in the Service Worker Cache implementation on maple.  Consider the following steps:

1) content runs caches.open('foo')
2) content gets back a Cache object
3) QuotaManager decides to wipe the origin directory for some reason
4) content calls cache.put() or cache.match()
5) content gets back an error because the Cache's ID no longer exists in the database

The Cache implementation should be smart enough to detect this case.  I think the only reasonable action here is to automatically perform the open('foo') again.  The Cache will be empty, but that is an expected case when using Cache.  The main upside is content won't be stuck with a permanently broken Cache object.

I believe the QuotaManager v2 will allow us to indicate that the quota directory is still being referenced by content and thus avoid this situation.

Comment 1

[Jan Varga [:janv]]

QuotaManager v1 provides registerStorage/unregisterStorage() for this, where storage is actually the cache objects in this context. QuotaManager v1 won't evict the origin directory if there are any live storages for it.

Comment 2

[Jan Varga [:janv]]

I'm not sure if I was clear in my previous comment, if you implement nsIOfflineStorage, then you shouldn't encounter this problem. The idea is that all these Cache objects are registered in quota manager, so it knows that it shouldn't clear origins which belong to these objects.

quota manager v2 removes nsIOfflineStorage entirely because it uses directory locks which unify WaitForOpenAllowed/AllowNextSyncOp/RegisterStorage/UnregisterStorage.

Hopefully, qm2 lands before the cache needs to be enabled by default.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Jan, isn't it still possible in QM v1 for the user to use the UI to clear storage for the origin causing an invalidate on existing storages?

I think this will still trigger the condition I mention in comment 0 if the user does this while content script holds a Cache object alive.

Comment 5

[Jan Varga [:janv]]

Yeah, the cache DOM object won't be able to process a .put() or .match(), but I think that's expected after an origin clear.
An automatic re-open would be needed if there's just one global cache object, but there's the caches.open('foo') call which returns a cache object.

.put() or any other operation could return a specific error, so content would be able to detect it and re-open the cache

Anyway, I'm not an expert on this API, so feel free to propose something else.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Yea, I'm inclined not to fix this for now.  Lets just leave it open and if people complain about origin clearing effecting them then we can fix then.  Either way, their state is going to be hosed.  I'll just add code for now to make sure those stale Cache objects don't accidentally re-used a cache ID in the new database.

Comment 7

[Jens Stutte [:jstutte]]

As the bug talks about QM v1 and we are working on v4 - is this still an issue to consider?

Comment 8

[Jens Stutte [:jstutte]]

Based on comment 6 I'd say, if people have real problems with this, they'll fill a new bug for it.

Comment 9

[Jan Varga [:janv]]

(In reply to Jens Stutte [:jstutte] from comment #7)

As the bug talks about QM v1 and we are working on v4 - is this still an issue to consider?

Even if we wanted to do what was proposed in this bug, I don't think it depends on new QM major version.