Reland "CacheStorage: Write index after simple disk_cache."

This is a reland of 053b22e8

The CacheStorageManagerTest.TestErrorInitializingCache test has been modified
to reduce flakiness.

Original change's description:
> 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: Daniel Murphy <dmurph@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#682409}

Bug: 988001
Change-Id: I12aa006bf1d18be5ae121c0330d97e51a97ff906
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1739408Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684410}

content/browser/cache_storage/cache_storage_manager_unittest.cc

@@ -1402,16 +1402,37 @@ TEST_F(CacheStorageManagerTest, TestErrorInitializingCache) {
   CacheStorageHandle cache_storage = CacheStorageForOrigin(origin1_);
   auto cache_handle =
       LegacyCacheStorage::From(cache_storage)->GetLoadedCache(kCacheName);
-  base::FilePath index_path =
-      LegacyCacheStorageCache::From(cache_handle)->path().AppendASCII("index");
+  base::FilePath cache_path =
+      LegacyCacheStorageCache::From(cache_handle)->path();
+  base::FilePath storage_path = cache_path.DirName();
+  base::FilePath index_path = cache_path.AppendASCII("index");
   cache_handle = CacheStorageCacheHandle();
 
+  // Do our best to flush any pending cache_storage index file writes to disk
+  // before proceeding.  This does not guarantee the simple disk_cache index
+  // is written, though.
+  EXPECT_TRUE(FlushCacheStorageIndex(origin1_));
+  EXPECT_FALSE(FlushCacheStorageIndex(origin1_));
+
   DestroyStorageManager();
 
   // Truncate the SimpleCache index to force an error when next opened.
   ASSERT_FALSE(index_path.empty());
   ASSERT_EQ(5, base::WriteFile(index_path, "hello", 5));
 
+  // The cache_storage index and simple disk_cache index files are written from
+  // background threads.  They may be written in unexpected orders due to timing
+  // differences.  We need to ensure 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_storage index to have a much older time to ensure that it is not used
+  // in the following Size() call.
+  base::FilePath cache_index_path =
+      storage_path.AppendASCII(LegacyCacheStorage::kIndexFileName);
+  base::Time t = base::Time::Now() + base::TimeDelta::FromHours(-1);
+  EXPECT_TRUE(base::TouchFile(cache_index_path, t, t));
+  EXPECT_FALSE(IsIndexFileCurrent(storage_path));
+
   CreateStorageManager();
 
   EXPECT_TRUE(Open(origin1_, kCacheName));

content/browser/cache_storage/legacy/legacy_cache_storage.cc

@@ -571,18 +571,27 @@ LegacyCacheStorage::LegacyCacheStorage(
       quota_manager_proxy_(quota_manager_proxy),
       owner_(owner),
       cache_storage_manager_(cache_storage_manager) {
-  if (memory_only)
+  if (memory_only) {
     cache_loader_.reset(new MemoryLoader(
         cache_task_runner_.get(), std::move(scheduler_task_runner),
         quota_manager_proxy.get(), blob_context, this, origin, owner));
-  else
-    cache_loader_.reset(new SimpleCacheLoader(
-        origin_path_, cache_task_runner_.get(),
-        std::move(scheduler_task_runner), quota_manager_proxy.get(),
-        blob_context, this, origin, owner));
+    return;
+  }
+
+  cache_loader_.reset(new SimpleCacheLoader(
+      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() {
   return CacheStorageHandle(weak_factory_.GetWeakPtr());
@@ -810,14 +819,19 @@ void LegacyCacheStorage::NotifyCacheContentChanged(
 }
 
 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_);
   index_write_task_.Reset(base::BindOnce(&LegacyCacheStorage::WriteIndex,
                                          weak_factory_.GetWeakPtr(),
                                          base::DoNothing::Once<bool>()));
   base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, index_write_task_.callback(),
-      base::TimeDelta::FromSeconds(kWriteIndexDelaySecs));
+      base::TimeDelta::FromMilliseconds(delay_ms));
 }
 
 void LegacyCacheStorage::WriteIndex(base::OnceCallback<void(bool)> callback) {
@@ -1014,6 +1028,7 @@ void LegacyCacheStorage::CreateCacheDidCreateCache(
       cache_ptr->cache_padding_key()->key()));
 
   CacheStorageCacheHandle handle = cache_ptr->CreateHandle();
+  index_write_task_.Cancel();
   cache_loader_->WriteIndex(
       *cache_index_,
       base::BindOnce(&LegacyCacheStorage::CreateCacheDidWriteIndex,
@@ -1072,6 +1087,7 @@ void LegacyCacheStorage::DoomCacheImpl(const std::string& cache_name,
   DCHECK(scheduler_->IsRunningExclusiveOperation());
   LegacyCacheStorageCache::From(cache_handle)->SetObserver(nullptr);
   cache_index_->DoomCache(cache_name);
+  index_write_task_.Cancel();
   cache_loader_->WriteIndex(
       *cache_index_,
       base::BindOnce(&LegacyCacheStorage::DeleteCacheDidWriteIndex,
@@ -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

content/browser/cache_storage/legacy/legacy_cache_storage.h

@@ -15,11 +15,16 @@
 #include "base/memory/memory_pressure_listener.h"
 #include "base/memory/ref_counted.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_cache_observer.h"
 #include "content/browser/cache_storage/cache_storage_scheduler_types.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 {
 class SequencedTaskRunner;
 }
@@ -260,6 +265,12 @@ class CONTENT_EXPORT LegacyCacheStorage : public CacheStorage,
   bool InitiateScheduledIndexWriteForTest(
       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.
   bool initialized_;
   bool initializing_;
@@ -305,6 +316,14 @@ class CONTENT_EXPORT LegacyCacheStorage : public CacheStorage,
   base::CancelableOnceClosure index_write_task_;
   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_);
   base::WeakPtrFactory<LegacyCacheStorage> weak_factory_{this};