(Repro steps entirely thanks to rjfioravanti and cmumford)

Issue 798624 has been merged into this issue.

jsbell mentioned offline that https://bugs.chromium.org/p/chromium/issues/detail?id=720784 may be related

Corrected ZIP attached here

Deleted: cacheStorage.zip 6.0 KB

cacheStorage.zip 6.0 KB Download

Accidentally omitted version info:

Chromium	65.0.3317.0 (Developer Build) (64-bit)
Revision	23ed9160c9bdcde34bffd7e883d3da30b99e01ff-
OS	Linux

Reproducible in stable, beta, dev, and canary channels and in ToT builds.

Assigning to cmumford based on recent quota work; feel free to bounce it to me or back to Available if you need to, though

Chris's notes from email, in case someone else ends up taking this one or it's deferred for a while while Pri<3 tasks are tackled:

Here's what I've learned.

CacheStorageCache is the backend object for a Cache Storage Cache. The CacheStorage object is for the entire origin. When Chrome starts up the CacheStorageManager (singleton per profile) registers a CacheStorageQuotaClient with the QuotaManager which later on calls CacheStorageQuotaClient::GetOriginUsage() to determine how much disk space is being consumed. This winds up calling CacheStorageManager::GetOriginUsage() which creates a CacheStorage object and calls CacheStorage::Size(). CacheStorage::SizeImpl() consults it's CacheStorageIndex instance, and if it's size/padding is known then it returns that. Otherwise it opens all the caches and sums them all up. Once CacheStorage::Size's callback *is* the QuotaManager callback, thus it is now aware of the total size used by that origin.

Whew! OK. so now if a cache is opened then CacheStorageCache::last_reported_size_ defaults to zero, so in CacheStorageCache::UpdateCacheSizeGotSize() when the current size is updated it's reported as larger delta than it should really be.

I'm 90% certain that the correct solution is to add these two lines to the body of the CacheStorageCache constructor:

  if (PaddedCacheSize() != CacheStorage::kSizeUnknown)
    last_reported_size_ = PaddedCacheSize();

This assumes that if both size and padding is known then the size has been reported. This should be validated though. Older caches on disk, which haven't been upgraded to contain their padding may have a -1 for padding, and thus be reported to QM and still PaddedCacheSize() would return -1.

It also needs a new test (or two).

Hopefully that's all it needs to land.

Up for review at crrev.com/c/862366.

Thank you very much, Chris!

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

commit 734aaf51f294e8d08e943522c95d9b974dcd2f94
Author: Chris Mumford <cmumford@chromium.org>
Date: Fri Jan 12 17:08:18 2018

CacheStorage: Fixed incorrect reporting to QuotaManager.

When a cache was reopened and written to it would calculate its size and
always report that as an increase via a call to
QuotaManagerProxy::NotifyStorageModified() in
CacheStorageCache::UpdateCacheSizeGotSize(). This would result in the
calculated storage use for an origin increasing by the original size +
the new resource size each time the cache was closed and reopened.

Bug:  801024 
Change-Id: Ib261159dbc53084e7050d03b53d447992e7dd35a
Reviewed-on: https://chromium-review.googlesource.com/862366
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528977}
[modify] https://crrev.com/734aaf51f294e8d08e943522c95d9b974dcd2f94/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/734aaf51f294e8d08e943522c95d9b974dcd2f94/content/browser/cache_storage/cache_storage_manager_unittest.cc

Thank you, Chris!

Any reason not to backport this to the M64 branch? This seems to me like a fairly self-contained fix for a relatively serious issue impacting web applications using cache storage

The flip side is that M64 is close to being promoted to stable, so we'd need to show serious impact for this. Chris should decide if the problem seems serious enough to warrant a merge at this stage.

 Issue 795134  has been merged into this issue.

This is a very severe issue for my app. Cache usage climbs until my service worker crashes with quota exceeded (using sw-toolbox). At this point service worker is completely broken for my app. Clearing all caches (as below) does not reset the quota so once the quota limit is hit a user can no longer run my service worker. The only way to reset the quota appears to be through developer tools which is not an options for my users.

 caches.keys().then(function(names) {
            for (let name of names)
                caches.delete(name);
        });

What c18 says. I tried switching to Workbox-js but it has the same issue.

Btw, on Chrome for Android the user can clear the site data under settings > site settings. 

#18, #19: I'm sorry for the trouble we're causing your apps. At this point in the release cycle, we will not be merging the fix into M64.

So it's fixed from 65.0.3325.0?

#21: omahaproxy says the fix landed in 65.0.3320.0, so it should be in the current Beta, Dev, and Canary builds. Please let us know if you're still seeing this problem.

 Issue 816433  has been merged into this issue.

I'm seeing something very similar in Version 67.0.3396.62 (Developer Build) built on Debian buster/sid, running on Debian buster/sid (64-bit)

I'm caching map tiles for offline use and seeing ~200MB added to the cache storage while the network monitor reports ~.3MB of traffic. The tile sizes reported for individual tiles in the cache seem normal and disk usage does not change even as the application reports several GB of storage used.

Re: #24, what you're seeing is most likely due to caching opaque responses, which deliberately contribute much more to the cache storage usage than the actual sizes of the underlying response bodies.

For context, see https://bugs.chromium.org/p/chromium/issues/detail?id=796060#c17, and for ideas on how to avoid developer confusion, see https://bugs.chromium.org/p/chromium/issues/detail?id=847462