Commit 053b22e8 authored by Ben Kelly's avatar Ben Kelly Committed by Commit Bot

CacheStorage: Write index after simple disk_cache.

When we load the cache_storage index we try to determine if its fresh
or stale by comparing its timestamp against the timestamp of the
various simple disk_cache directories.  Both cache_storage and simple
disk_cache write their index files out using a delay.  The
cache_storage delay is 5 seconds and the simple disk_cache delay is 20
seconds.  This means that the simple disk_cache index is always written
last and as a result the cache_storage index is almost always
considered stale.

This CL fix this by extending the cache_storage delay to match the
simple disk_cache delay.  It also implements a shorter delay on android
when in the background like simple disk_cache uses to avoid losing
data when the app is killed by the operating system.  Finally, the CL
also now properly flushes the index to disk if there is a pending write
when the cache_storage subsystem is torn down.

Over time this should cause ServiceWorkerCache.UsedIndexFileSize to
show more cases where the index file is used to provide the size.

Bug: 988001
Change-Id: Idaa09b956edcfa9e70ac1fdb9b43adcd73ec0508
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721389
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682409}
parent 4e553a28
...@@ -1402,8 +1402,9 @@ TEST_F(CacheStorageManagerTest, TestErrorInitializingCache) { ...@@ -1402,8 +1402,9 @@ TEST_F(CacheStorageManagerTest, TestErrorInitializingCache) {
CacheStorageHandle cache_storage = CacheStorageForOrigin(origin1_); CacheStorageHandle cache_storage = CacheStorageForOrigin(origin1_);
auto cache_handle = auto cache_handle =
LegacyCacheStorage::From(cache_storage)->GetLoadedCache(kCacheName); LegacyCacheStorage::From(cache_storage)->GetLoadedCache(kCacheName);
base::FilePath index_path = base::FilePath cache_path =
LegacyCacheStorageCache::From(cache_handle)->path().AppendASCII("index"); LegacyCacheStorageCache::From(cache_handle)->path();
base::FilePath index_path = cache_path.AppendASCII("index");
cache_handle = CacheStorageCacheHandle(); cache_handle = CacheStorageCacheHandle();
DestroyStorageManager(); DestroyStorageManager();
...@@ -1412,6 +1413,16 @@ TEST_F(CacheStorageManagerTest, TestErrorInitializingCache) { ...@@ -1412,6 +1413,16 @@ TEST_F(CacheStorageManagerTest, TestErrorInitializingCache) {
ASSERT_FALSE(index_path.empty()); ASSERT_FALSE(index_path.empty());
ASSERT_EQ(5, base::WriteFile(index_path, "hello", 5)); ASSERT_EQ(5, base::WriteFile(index_path, "hello", 5));
// The cache_storage index is written from a background thread and may be
// written just after our overwrite of the simple disk_cache index here.
// We need the cache directory to have a newer time stamp, though, in order
// for the Size() method to actually try calculating the size from the
// corrupted simple disk_cache. Therefore we force the cache directory
// to have a much newer time to ensure the cache_storage index is not used
// in the following Size() call.
base::Time t = base::Time::Now() + base::TimeDelta::FromHours(1);
EXPECT_TRUE(base::TouchFile(cache_path, t, t));
CreateStorageManager(); CreateStorageManager();
EXPECT_TRUE(Open(origin1_, kCacheName)); EXPECT_TRUE(Open(origin1_, kCacheName));
......
...@@ -571,18 +571,27 @@ LegacyCacheStorage::LegacyCacheStorage( ...@@ -571,18 +571,27 @@ LegacyCacheStorage::LegacyCacheStorage(
quota_manager_proxy_(quota_manager_proxy), quota_manager_proxy_(quota_manager_proxy),
owner_(owner), owner_(owner),
cache_storage_manager_(cache_storage_manager) { cache_storage_manager_(cache_storage_manager) {
if (memory_only) if (memory_only) {
cache_loader_.reset(new MemoryLoader( cache_loader_.reset(new MemoryLoader(
cache_task_runner_.get(), std::move(scheduler_task_runner), cache_task_runner_.get(), std::move(scheduler_task_runner),
quota_manager_proxy.get(), blob_context, this, origin, owner)); quota_manager_proxy.get(), blob_context, this, origin, owner));
else return;
cache_loader_.reset(new SimpleCacheLoader( }
origin_path_, cache_task_runner_.get(),
std::move(scheduler_task_runner), quota_manager_proxy.get(), cache_loader_.reset(new SimpleCacheLoader(
blob_context, this, origin, owner)); origin_path_, cache_task_runner_.get(), std::move(scheduler_task_runner),
quota_manager_proxy.get(), blob_context, this, origin, owner));
#if defined(OS_ANDROID)
app_status_listener_ = base::android::ApplicationStatusListener::New(
base::BindRepeating(&LegacyCacheStorage::OnApplicationStateChange,
weak_factory_.GetWeakPtr()));
#endif
} }
LegacyCacheStorage::~LegacyCacheStorage() {} LegacyCacheStorage::~LegacyCacheStorage() {
FlushIndexIfDirty();
}
CacheStorageHandle LegacyCacheStorage::CreateHandle() { CacheStorageHandle LegacyCacheStorage::CreateHandle() {
return CacheStorageHandle(weak_factory_.GetWeakPtr()); return CacheStorageHandle(weak_factory_.GetWeakPtr());
...@@ -810,14 +819,19 @@ void LegacyCacheStorage::NotifyCacheContentChanged( ...@@ -810,14 +819,19 @@ void LegacyCacheStorage::NotifyCacheContentChanged(
} }
void LegacyCacheStorage::ScheduleWriteIndex() { void LegacyCacheStorage::ScheduleWriteIndex() {
static const int64_t kWriteIndexDelaySecs = 5; // These values are chosen to be equal or greater than the simple disk_cache
// index write delays. We want the cache_storage index to be written last.
static const int64_t kWriteIndexDelayMilliseconds = 20050;
static const int64_t kWriteIndexBackgroundDelayMilliseconds = 150;
int64_t delay_ms = app_on_background_ ? kWriteIndexBackgroundDelayMilliseconds
: kWriteIndexDelayMilliseconds;
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
index_write_task_.Reset(base::BindOnce(&LegacyCacheStorage::WriteIndex, index_write_task_.Reset(base::BindOnce(&LegacyCacheStorage::WriteIndex,
weak_factory_.GetWeakPtr(), weak_factory_.GetWeakPtr(),
base::DoNothing::Once<bool>())); base::DoNothing::Once<bool>()));
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, index_write_task_.callback(), FROM_HERE, index_write_task_.callback(),
base::TimeDelta::FromSeconds(kWriteIndexDelaySecs)); base::TimeDelta::FromMilliseconds(delay_ms));
} }
void LegacyCacheStorage::WriteIndex(base::OnceCallback<void(bool)> callback) { void LegacyCacheStorage::WriteIndex(base::OnceCallback<void(bool)> callback) {
...@@ -1014,6 +1028,7 @@ void LegacyCacheStorage::CreateCacheDidCreateCache( ...@@ -1014,6 +1028,7 @@ void LegacyCacheStorage::CreateCacheDidCreateCache(
cache_ptr->cache_padding_key()->key())); cache_ptr->cache_padding_key()->key()));
CacheStorageCacheHandle handle = cache_ptr->CreateHandle(); CacheStorageCacheHandle handle = cache_ptr->CreateHandle();
index_write_task_.Cancel();
cache_loader_->WriteIndex( cache_loader_->WriteIndex(
*cache_index_, *cache_index_,
base::BindOnce(&LegacyCacheStorage::CreateCacheDidWriteIndex, base::BindOnce(&LegacyCacheStorage::CreateCacheDidWriteIndex,
...@@ -1072,6 +1087,7 @@ void LegacyCacheStorage::DoomCacheImpl(const std::string& cache_name, ...@@ -1072,6 +1087,7 @@ void LegacyCacheStorage::DoomCacheImpl(const std::string& cache_name,
DCHECK(scheduler_->IsRunningExclusiveOperation()); DCHECK(scheduler_->IsRunningExclusiveOperation());
LegacyCacheStorageCache::From(cache_handle)->SetObserver(nullptr); LegacyCacheStorageCache::From(cache_handle)->SetObserver(nullptr);
cache_index_->DoomCache(cache_name); cache_index_->DoomCache(cache_name);
index_write_task_.Cancel();
cache_loader_->WriteIndex( cache_loader_->WriteIndex(
*cache_index_, *cache_index_,
base::BindOnce(&LegacyCacheStorage::DeleteCacheDidWriteIndex, base::BindOnce(&LegacyCacheStorage::DeleteCacheDidWriteIndex,
...@@ -1397,4 +1413,24 @@ void LegacyCacheStorage::SizeImpl(SizeCallback callback) { ...@@ -1397,4 +1413,24 @@ void LegacyCacheStorage::SizeImpl(SizeCallback callback) {
} }
} }
void LegacyCacheStorage::FlushIndexIfDirty() {
if (!index_write_pending())
return;
index_write_task_.Cancel();
cache_loader_->WriteIndex(*cache_index_, base::DoNothing::Once<bool>());
}
#if defined(OS_ANDROID)
void LegacyCacheStorage::OnApplicationStateChange(
base::android::ApplicationState state) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (state == base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES) {
app_on_background_ = false;
} else if (state == base::android::APPLICATION_STATE_HAS_STOPPED_ACTIVITIES) {
app_on_background_ = true;
FlushIndexIfDirty();
}
}
#endif
} // namespace content } // namespace content
...@@ -15,11 +15,16 @@ ...@@ -15,11 +15,16 @@
#include "base/memory/memory_pressure_listener.h" #include "base/memory/memory_pressure_listener.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "build/build_config.h"
#include "content/browser/cache_storage/cache_storage.h" #include "content/browser/cache_storage/cache_storage.h"
#include "content/browser/cache_storage/cache_storage_cache_observer.h" #include "content/browser/cache_storage/cache_storage_cache_observer.h"
#include "content/browser/cache_storage/cache_storage_scheduler_types.h" #include "content/browser/cache_storage/cache_storage_scheduler_types.h"
#include "content/browser/cache_storage/legacy/legacy_cache_storage_cache.h" #include "content/browser/cache_storage/legacy/legacy_cache_storage_cache.h"
#if defined(OS_ANDROID)
#include "base/android/application_status_listener.h"
#endif
namespace base { namespace base {
class SequencedTaskRunner; class SequencedTaskRunner;
} }
...@@ -260,6 +265,12 @@ class CONTENT_EXPORT LegacyCacheStorage : public CacheStorage, ...@@ -260,6 +265,12 @@ class CONTENT_EXPORT LegacyCacheStorage : public CacheStorage,
bool InitiateScheduledIndexWriteForTest( bool InitiateScheduledIndexWriteForTest(
base::OnceCallback<void(bool)> callback); base::OnceCallback<void(bool)> callback);
void FlushIndexIfDirty();
#if defined(OS_ANDROID)
void OnApplicationStateChange(base::android::ApplicationState state);
#endif
// Whether or not we've loaded the list of cache names into memory. // Whether or not we've loaded the list of cache names into memory.
bool initialized_; bool initialized_;
bool initializing_; bool initializing_;
...@@ -305,6 +316,14 @@ class CONTENT_EXPORT LegacyCacheStorage : public CacheStorage, ...@@ -305,6 +316,14 @@ class CONTENT_EXPORT LegacyCacheStorage : public CacheStorage,
base::CancelableOnceClosure index_write_task_; base::CancelableOnceClosure index_write_task_;
size_t handle_ref_count_ = 0; size_t handle_ref_count_ = 0;
#if defined(OS_ANDROID)
std::unique_ptr<base::android::ApplicationStatusListener>
app_status_listener_;
#endif
// True if running on android and the app is in the background.
bool app_on_background_ = false;
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
base::WeakPtrFactory<LegacyCacheStorage> weak_factory_{this}; base::WeakPtrFactory<LegacyCacheStorage> weak_factory_{this};
......
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