CacheStorage: Improve reading quota size from index file.

When determining the quota size of cache_storage we would prefer to use
the value stored in the index file.  This reduces the number of files
we need to open and read.  We can't use the index value, though, if the
index file is stale compared to the data stored in cache_storage.

Unfortunately, previous code had a bug that would always consider the
index file as stale.  It only permitted the index file to be used if
the index file's modified time was newer than the modified time of its
containing directory.  This was an impossible condition to meet,
however, because the directory will have the same time or newer than
its contents.

This CL allows us to use the index file value if it has the same time
as its containing directory under certain conditions.  Specifically,
the index file must also be newer than all cache directories
referenced from the index.

Bug: 986474
Change-Id: I11bba1edbf78f9cc99f62823f370842c5f291bff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1712951Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680255}

content/browser/cache_storage/cache_storage_manager_unittest.cc

@@ -1751,14 +1751,30 @@ TEST_F(CacheStorageManagerTest, MAYBE_GetAllOriginsUsageWithOldIndex) {
   EXPECT_TRUE(CachePut(original_handle.value(), kBarURL));
   original_handle = CacheStorageCacheHandle();
 
+  // Capture the size before the index has necessarily flushed to disk.
   std::vector<StorageUsageInfo> usage = GetAllOriginsUsage();
   ASSERT_EQ(1ULL, usage.size());
   int64_t usage_before_close = usage[0].total_size_bytes;
   EXPECT_GT(usage_before_close, 0);
 
+  // Flush the index to ensure we can read it correctly from the index file.
+  EXPECT_TRUE(FlushCacheStorageIndex(origin1_));
+
   // Close the caches and cache manager.
   DestroyStorageManager();
 
+  CreateStorageManager();
+  quota_manager_proxy_->SimulateQuotaManagerDestroyed();
+  RecreateStorageManager();
+
+  // Read the size from the index file.
+  CreateStorageManager();
+  usage = GetAllOriginsUsage();
+  ASSERT_EQ(1ULL, usage.size());
+  EXPECT_EQ(usage_before_close, usage[0].total_size_bytes);
+
+  DestroyStorageManager();
+
   // Restore the index to the V1 state. Make the access/mod times of index file
   // older than the other directories in the store to trigger size
   // recalculation.
@@ -1767,6 +1783,7 @@ TEST_F(CacheStorageManagerTest, MAYBE_GetAllOriginsUsageWithOldIndex) {
   EXPECT_TRUE(base::TouchFile(index_path, t, t));
   EXPECT_FALSE(IsIndexFileCurrent(storage_dir));
 
+  // Read the size with the stale index file forcing a recalculation.
   CreateStorageManager();
   usage = GetAllOriginsUsage();
   ASSERT_EQ(1ULL, usage.size());

content/browser/cache_storage/legacy/legacy_cache_storage_manager.cc

@@ -57,14 +57,45 @@ void DeleteOriginDidDeleteDir(storage::QuotaClient::DeletionCallback callback,
 
 // Calculate the sum of all cache sizes in this store, but only if all sizes are
 // known. If one or more sizes are not known then return kSizeUnknown.
-int64_t GetCacheStorageSize(const proto::CacheStorageIndex& index) {
+int64_t GetCacheStorageSize(const base::FilePath& base_path,
+                            const base::Time& base_path_time,
+                            const base::Time& index_time,
+                            const proto::CacheStorageIndex& index) {
+  // If the base path's modified time is newer than the index, then the
+  // contents of a cache must have changed.  The index is stale.
+  if (base_path_time > index_time)
+    return CacheStorage::kSizeUnknown;
+
+  // It should be impossible for the directory containing the index to
+  // have a modified time older than the index's modified time.  Modifying
+  // the index should update the directories time as well.  Therefore we
+  // should be guaranteed that the time is equal here.
+  DCHECK_EQ(base_path_time, index_time);
+
   int64_t storage_size = 0;
   for (int i = 0, max = index.cache_size(); i < max; ++i) {
     const proto::CacheStorageIndex::Cache& cache = index.cache(i);
-    if (!cache.has_size() || cache.size() == CacheStorage::kSizeUnknown)
+    if (!cache.has_cache_dir() || !cache.has_size() ||
+        cache.size() == CacheStorage::kSizeUnknown) {
+      return CacheStorage::kSizeUnknown;
+    }
+
+    // Check the modified time on each cache directory.  If one of the
+    // directories has the same or newer modified time as the index file, then
+    // its size is most likely not accounted for in the index file.  The
+    // cache can have a newer time here in spite of our base path time check
+    // above since simple disk_cache writes to these directories from a
+    // different thread.
+    base::FilePath path = base_path.AppendASCII(cache.cache_dir());
+    base::File::Info file_info;
+    if (!base::GetFileInfo(path, &file_info) ||
+        file_info.last_modified >= index_time) {
       return CacheStorage::kSizeUnknown;
+    }
+
     storage_size += cache.size();
   }
+
   return storage_size;
 }
 
@@ -145,9 +176,11 @@ void ListOriginsAndLastModifiedOnTaskRunner(
       continue;
     }
 
-    int64_t storage_size = CacheStorage::kSizeUnknown;
-    if (file_info.last_modified < index_last_modified)
-      storage_size = GetCacheStorageSize(index);
+    int64_t storage_size = GetCacheStorageSize(path, file_info.last_modified,
+                                               index_last_modified, index);
+    base::UmaHistogramBoolean("ServiceWorkerCache.UsedIndexFileSize",
+                              storage_size != CacheStorage::kSizeUnknown);
+
     usages->push_back(
         StorageUsageInfo(origin, storage_size, file_info.last_modified));
     RecordIndexValidationResult(IndexResult::kOk);

tools/metrics/histograms/histograms.xml

@@ -122577,6 +122577,17 @@ should be kept until we remove incident reporting. -->
   </summary>
 </histogram>
 
+<histogram name="ServiceWorkerCache.UsedIndexFileSize" enum="Boolean"
+    expires_after="M85">
+  <owner>wanderview@chromium.org</owner>
+  <owner>chrome-owp-storage@google.com</owner>
+  <summary>
+    Whether the size was calculated using the values stored in the origin's
+    index file. If this is false, then the size was recalculated by loading all
+    of the individual cache backends.
+  </summary>
+</histogram>
+
 <histogram name="Servicification.Startup" enum="ServicificationStartupMode"
     expires_after="2019-02-26">
   <obsolete>