Commit 431944cd authored by Matt Falkenhagen's avatar Matt Falkenhagen Committed by Commit Bot

Don't set a size limit for the disk cache in APP_CACHE mode.

In APP_CACHE mode, the eviction is somewhat disabled by design, but
callers were specifying a size limit which caused eviction anyway.  This
was causing undesired eviction for service worker scripts, where we
expect the quota manager to manage the space usage instead.

This CL changes APP_CACHE callers use int64_max as the size limit,
including service worker and appcache. The Cache Storage API
was already using int64_max.

Note that the int64::max is only respected by the Simple cache backend.
Service worker uses Simple cache[1] while AppCache has a build flag
that allows toggling between Simple and Blockfile.[2] The Blockfile backend
goes up to int::max only.

[1] https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_disk_cache.cc?l=12&rcl=c7a9d26cc435f6074ee2372fef360245524e2cf8
[2] https://cs.chromium.org/chromium/src/content/browser/appcache/appcache_disk_cache.cc?l=198&rcl=8e7751dff574550f053ff580f4c174dc54130c74

There is no test added because I can't think of a way to test
this.

Bug: 526915
Change-Id: I808a15312adaa3fc26c78eb165fa79dc13058aae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1608868Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarMaks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659359}
parent 9e54bad4
...@@ -209,16 +209,16 @@ AppCacheDiskCache::~AppCacheDiskCache() { ...@@ -209,16 +209,16 @@ AppCacheDiskCache::~AppCacheDiskCache() {
net::Error AppCacheDiskCache::InitWithDiskBackend( net::Error AppCacheDiskCache::InitWithDiskBackend(
const base::FilePath& disk_cache_directory, const base::FilePath& disk_cache_directory,
int disk_cache_size,
bool force, bool force,
base::OnceClosure post_cleanup_callback, base::OnceClosure post_cleanup_callback,
net::CompletionOnceCallback callback) { net::CompletionOnceCallback callback) {
return Init(net::APP_CACHE, disk_cache_directory, disk_cache_size, force, return Init(net::APP_CACHE, disk_cache_directory,
std::numeric_limits<int64_t>::max(), force,
std::move(post_cleanup_callback), std::move(callback)); std::move(post_cleanup_callback), std::move(callback));
} }
net::Error AppCacheDiskCache::InitWithMemBackend( net::Error AppCacheDiskCache::InitWithMemBackend(
int mem_cache_size, int64_t mem_cache_size,
net::CompletionOnceCallback callback) { net::CompletionOnceCallback callback) {
return Init(net::MEMORY_CACHE, base::FilePath(), mem_cache_size, false, return Init(net::MEMORY_CACHE, base::FilePath(), mem_cache_size, false,
base::OnceClosure(), std::move(callback)); base::OnceClosure(), std::move(callback));
...@@ -339,7 +339,7 @@ AppCacheDiskCache::PendingCall::~PendingCall() = default; ...@@ -339,7 +339,7 @@ AppCacheDiskCache::PendingCall::~PendingCall() = default;
net::Error AppCacheDiskCache::Init(net::CacheType cache_type, net::Error AppCacheDiskCache::Init(net::CacheType cache_type,
const base::FilePath& cache_directory, const base::FilePath& cache_directory,
int cache_size, int64_t cache_size,
bool force, bool force,
base::OnceClosure post_cleanup_callback, base::OnceClosure post_cleanup_callback,
net::CompletionOnceCallback callback) { net::CompletionOnceCallback callback) {
......
...@@ -66,14 +66,13 @@ class CONTENT_EXPORT AppCacheDiskCache { ...@@ -66,14 +66,13 @@ class CONTENT_EXPORT AppCacheDiskCache {
// Initializes the object to use disk backed storage. // Initializes the object to use disk backed storage.
net::Error InitWithDiskBackend(const base::FilePath& disk_cache_directory, net::Error InitWithDiskBackend(const base::FilePath& disk_cache_directory,
int disk_cache_size,
bool force, bool force,
base::OnceClosure post_cleanup_callback, base::OnceClosure post_cleanup_callback,
net::CompletionOnceCallback callback); net::CompletionOnceCallback callback);
// Initializes the object to use memory only storage. // Initializes the object to use memory only storage.
// This is used for Chrome's incognito browsing. // This is used for Chrome's incognito browsing.
net::Error InitWithMemBackend(int disk_cache_size, net::Error InitWithMemBackend(int64_t disk_cache_size,
net::CompletionOnceCallback callback); net::CompletionOnceCallback callback);
void Disable(); void Disable();
...@@ -141,7 +140,7 @@ class CONTENT_EXPORT AppCacheDiskCache { ...@@ -141,7 +140,7 @@ class CONTENT_EXPORT AppCacheDiskCache {
net::Error Init(net::CacheType cache_type, net::Error Init(net::CacheType cache_type,
const base::FilePath& directory, const base::FilePath& directory,
int cache_size, int64_t cache_size,
bool force, bool force,
base::OnceClosure post_cleanup_callback, base::OnceClosure post_cleanup_callback,
net::CompletionOnceCallback callback); net::CompletionOnceCallback callback);
......
...@@ -55,7 +55,7 @@ TEST_F(AppCacheDiskCacheTest, DisablePriorToInitCompletion) { ...@@ -55,7 +55,7 @@ TEST_F(AppCacheDiskCacheTest, DisablePriorToInitCompletion) {
// one of each kind of "entry" function. // one of each kind of "entry" function.
std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache); std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache);
EXPECT_FALSE(disk_cache->is_disabled()); EXPECT_FALSE(disk_cache->is_disabled());
disk_cache->InitWithDiskBackend(directory_.GetPath(), k10MBytes, false, disk_cache->InitWithDiskBackend(directory_.GetPath(), false,
base::OnceClosure(), completion_callback_); base::OnceClosure(), completion_callback_);
disk_cache->CreateEntry(1, &entry, completion_callback_); disk_cache->CreateEntry(1, &entry, completion_callback_);
disk_cache->OpenEntry(2, &entry, completion_callback_); disk_cache->OpenEntry(2, &entry, completion_callback_);
...@@ -85,7 +85,7 @@ TEST_F(AppCacheDiskCacheTest, DisableAfterInitted) { ...@@ -85,7 +85,7 @@ TEST_F(AppCacheDiskCacheTest, DisableAfterInitted) {
// Create an instance and let it fully init. // Create an instance and let it fully init.
std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache); std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache);
EXPECT_FALSE(disk_cache->is_disabled()); EXPECT_FALSE(disk_cache->is_disabled());
disk_cache->InitWithDiskBackend(directory_.GetPath(), k10MBytes, false, disk_cache->InitWithDiskBackend(directory_.GetPath(), false,
base::OnceClosure(), completion_callback_); base::OnceClosure(), completion_callback_);
FlushCacheTasks(); FlushCacheTasks();
EXPECT_EQ(1u, completion_results_.size()); EXPECT_EQ(1u, completion_results_.size());
...@@ -120,7 +120,7 @@ TEST_F(AppCacheDiskCacheTest, DISABLED_DisableWithEntriesOpen) { ...@@ -120,7 +120,7 @@ TEST_F(AppCacheDiskCacheTest, DISABLED_DisableWithEntriesOpen) {
// Create an instance and let it fully init. // Create an instance and let it fully init.
std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache); std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache);
EXPECT_FALSE(disk_cache->is_disabled()); EXPECT_FALSE(disk_cache->is_disabled());
disk_cache->InitWithDiskBackend(directory_.GetPath(), k10MBytes, false, disk_cache->InitWithDiskBackend(directory_.GetPath(), false,
base::OnceClosure(), completion_callback_); base::OnceClosure(), completion_callback_);
FlushCacheTasks(); FlushCacheTasks();
EXPECT_EQ(1u, completion_results_.size()); EXPECT_EQ(1u, completion_results_.size());
...@@ -187,7 +187,7 @@ TEST_F(AppCacheDiskCacheTest, CleanupCallback) { ...@@ -187,7 +187,7 @@ TEST_F(AppCacheDiskCacheTest, CleanupCallback) {
net::TestCompletionCallback init_done; net::TestCompletionCallback init_done;
std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache); std::unique_ptr<AppCacheDiskCache> disk_cache(new AppCacheDiskCache);
EXPECT_FALSE(disk_cache->is_disabled()); EXPECT_FALSE(disk_cache->is_disabled());
disk_cache->InitWithDiskBackend(directory_.GetPath(), k10MBytes, false, disk_cache->InitWithDiskBackend(directory_.GetPath(), false,
cleanup_done.closure(), init_done.callback()); cleanup_done.closure(), init_done.callback());
EXPECT_EQ(net::OK, init_done.WaitForResult()); EXPECT_EQ(net::OK, init_done.WaitForResult());
......
...@@ -49,7 +49,6 @@ constexpr const int kMB = 1024 * 1024; ...@@ -49,7 +49,6 @@ constexpr const int kMB = 1024 * 1024;
// Hard coded default when not using quota management. // Hard coded default when not using quota management.
constexpr const int kDefaultQuota = 5 * kMB; constexpr const int kDefaultQuota = 5 * kMB;
constexpr const int kMaxAppCacheDiskCacheSize = 250 * kMB;
constexpr const int kMaxAppCacheMemDiskCacheSize = 10 * kMB; constexpr const int kMaxAppCacheMemDiskCacheSize = 10 * kMB;
constexpr base::FilePath::CharType kDiskCacheDirectoryName[] = constexpr base::FilePath::CharType kDiskCacheDirectoryName[] =
...@@ -1893,8 +1892,7 @@ AppCacheDiskCache* AppCacheStorageImpl::disk_cache() { ...@@ -1893,8 +1892,7 @@ AppCacheDiskCache* AppCacheStorageImpl::disk_cache() {
expecting_cleanup_complete_on_disable_ = true; expecting_cleanup_complete_on_disable_ = true;
rv = disk_cache_->InitWithDiskBackend( rv = disk_cache_->InitWithDiskBackend(
cache_directory_.Append(kDiskCacheDirectoryName), cache_directory_.Append(kDiskCacheDirectoryName), false,
kMaxAppCacheDiskCacheSize, false,
base::BindOnce(&AppCacheStorageImpl::OnDiskCacheCleanupComplete, base::BindOnce(&AppCacheStorageImpl::OnDiskCacheCleanupComplete,
weak_factory_.GetWeakPtr()), weak_factory_.GetWeakPtr()),
base::BindOnce(&AppCacheStorageImpl::OnDiskCacheInitialized, base::BindOnce(&AppCacheStorageImpl::OnDiskCacheInitialized,
......
...@@ -2196,7 +2196,7 @@ void LegacyCacheStorageCache::CreateBackend(ErrorCallback callback) { ...@@ -2196,7 +2196,7 @@ void LegacyCacheStorageCache::CreateBackend(ErrorCallback callback) {
// The maximum size of each cache. Ultimately, cache size // The maximum size of each cache. Ultimately, cache size
// is controlled per-origin by the QuotaManager. // is controlled per-origin by the QuotaManager.
uint64_t max_bytes = memory_only_ ? std::numeric_limits<int>::max() int64_t max_bytes = memory_only_ ? std::numeric_limits<int>::max()
: std::numeric_limits<int64_t>::max(); : std::numeric_limits<int64_t>::max();
std::unique_ptr<ScopedBackendPtr> backend_ptr(new ScopedBackendPtr()); std::unique_ptr<ScopedBackendPtr> backend_ptr(new ScopedBackendPtr());
......
...@@ -68,8 +68,8 @@ const base::FilePath::CharType kDatabaseName[] = ...@@ -68,8 +68,8 @@ const base::FilePath::CharType kDatabaseName[] =
const base::FilePath::CharType kDiskCacheName[] = const base::FilePath::CharType kDiskCacheName[] =
FILE_PATH_LITERAL("ScriptCache"); FILE_PATH_LITERAL("ScriptCache");
// Taken from AppCache's in-memory cache size.
const int kMaxServiceWorkerStorageMemDiskCacheSize = 10 * 1024 * 1024; const int kMaxServiceWorkerStorageMemDiskCacheSize = 10 * 1024 * 1024;
const int kMaxServiceWorkerStorageDiskCacheSize = 250 * 1024 * 1024;
blink::ServiceWorkerStatusCode DatabaseStatusToStatusCode( blink::ServiceWorkerStatusCode DatabaseStatusToStatusCode(
ServiceWorkerDatabase::Status status) { ServiceWorkerDatabase::Status status) {
...@@ -1749,7 +1749,7 @@ void ServiceWorkerStorage::InitializeDiskCache() { ...@@ -1749,7 +1749,7 @@ void ServiceWorkerStorage::InitializeDiskCache() {
disk_cache_->set_is_waiting_to_initialize(false); disk_cache_->set_is_waiting_to_initialize(false);
expecting_done_with_disk_on_disable_ = true; expecting_done_with_disk_on_disable_ = true;
int rv = disk_cache_->InitWithDiskBackend( int rv = disk_cache_->InitWithDiskBackend(
GetDiskCachePath(), kMaxServiceWorkerStorageDiskCacheSize, false, GetDiskCachePath(), false,
base::BindOnce(&ServiceWorkerStorage::DiskCacheImplDoneWithDisk, base::BindOnce(&ServiceWorkerStorage::DiskCacheImplDoneWithDisk,
weak_factory_.GetWeakPtr()), weak_factory_.GetWeakPtr()),
base::BindOnce(&ServiceWorkerStorage::OnDiskCacheInitialized, base::BindOnce(&ServiceWorkerStorage::OnDiskCacheInitialized,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment