Implement cache counting for the simple and memory backends.

net/disk_cache/backend_unittest.cc

Index: net/disk_cache/backend_unittest.cc
diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index 8833729f6965ce3a78f837a9e08d97f08105d3a5..c0d0fb372b13a6c5d6663d3884cef08c04b4ea57 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -89,6 +89,10 @@ class DiskCacheBackendTest : public DiskCacheTestWithCache {
                              std::set<std::string>* keys_to_match,
                              size_t* count);
 
+  // Computes the expected size of entry metadata, i.e. the total size without
+  // the actual data stored. This depends only on the entry's |key| size.
+  int GetEntryMetadataSize(std::string key);
+
   // Actual tests:
   void BackendBasics();
   void BackendKeying();
@@ -282,6 +286,19 @@ bool DiskCacheBackendTest::EnumerateAndMatchKeys(
   return true;
 }
 
+int DiskCacheBackendTest::GetEntryMetadataSize(std::string key) {
+  // For blockfile and memory backends, it is just the key size.
+  if (!simple_cache_mode_)
+    return key.size();
+
+  // For the simple cache, we must add the file header and EOF, and that for
+  // every stream.

[gavinp, 2015/10/13 15:50:12]

Is this right? We now store stream 0 and stream 1 in the same file, yes? And we
don't store stream 2 if there's no data in it. Tricky.

I realize you're just copy/pasting SimpleEntryImpl::GetDiskUsage() here.

How important is it to be byte-perfect here?

[msramek, 2015/10/13 17:25:50]

In DiskCacheBackendTest::BackendCalculateSizeOfAllEntries() where I alternate
between writing to streams 0 and 1, I tried writing to all three streams
instead, and the tests still pass. Which means that GetEntryMetadataSize() is
correct in the sense that

sum for entry in cache
  GetEntryMetadataSize() + sum for stream in entry
                             stream data size

equals CalculateSizeOfAllEntries(), which is SimpleIndex::cache_size_ in the
case of simple cache.

Now if I understand your concern correctly, GetDiskUsage() does not check
whether stream 2 is empty, and we use GetDiskUsage() in UpdateEntrySize(). This
means that the cache pretends that the file for stream 2 always exists?

It would be nice to be more precise with this computation, to give the user the
exact disk size freed when deleting the cache, but if the cache itself thinks
that it's larger than it really is, well.. I think with ~250 bytes per entry the
relative error should be low enough that it's OK.

+  return disk_cache::kSimpleEntryStreamCount * (
+      sizeof(disk_cache::SimpleFileHeader) +
+      sizeof(disk_cache::SimpleFileEOF) +
+      key.size());
+}
+
 void DiskCacheBackendTest::BackendBasics() {
   InitCache();
   disk_cache::Entry *entry1 = NULL, *entry2 = NULL;
@@ -1679,12 +1696,7 @@ void DiskCacheBackendTest::BackendCalculateSizeOfAllEntries() {
   InitCache();
 
   // The cache is initially empty.
-  if (memory_only_ || simple_cache_mode_) {
-    // TODO(msramek): Implement.
-    EXPECT_EQ(net::ERR_NOT_IMPLEMENTED, CalculateSizeOfAllEntries());
-  } else {
-    EXPECT_EQ(0, CalculateSizeOfAllEntries());
-  }
+  EXPECT_EQ(0, CalculateSizeOfAllEntries());
 
   // Generate random entries and populate them with data of respective
   // sizes 0, 1, ..., count - 1 bytes.
@@ -1697,10 +1709,15 @@ void DiskCacheBackendTest::BackendCalculateSizeOfAllEntries() {
     scoped_refptr<net::StringIOBuffer> buffer = new net::StringIOBuffer(data);
 
     // Alternate between writing to the first and second stream to test that
-    // we are not taking just the first stream into account.
+    // we are not taking just the first stream into account. For convenience,
+    // the last written stream should be 0. This is because writing to
+    // the stream 1 in simple cache triggers a write to the stream 0 as well.
+    // This will happen asynchronously and possibly later than our call to
+    // |CalculateSizeOfAllEntries|.
     disk_cache::Entry* entry;
     ASSERT_EQ(net::OK, OpenEntry(key, &entry));
-    ASSERT_EQ(count, WriteData(entry, count % 2, 0, buffer.get(), count, true));
+    ASSERT_EQ(count,
+              WriteData(entry, (count + 1) % 2, 0, buffer.get(), count, true));
     entry->Close();
 
     ++count;
@@ -1708,16 +1725,10 @@ void DiskCacheBackendTest::BackendCalculateSizeOfAllEntries() {
 
   // The resulting size should be (0 + 1 + ... + count - 1) plus keys.
   int result = CalculateSizeOfAllEntries();
-  if (memory_only_ || simple_cache_mode_) {
-    // TODO(msramek): Implement.
-    EXPECT_EQ(net::ERR_NOT_IMPLEMENTED, result);
-  } else {
-    int total_key_size = 0;
-    for (std::string key : key_pool)
-      total_key_size += key.size();
-
-    EXPECT_EQ((count - 1) * count / 2 + total_key_size, result);
-  }
+  int total_metadata_size = 0;
+  for (std::string key : key_pool)
+    total_metadata_size += GetEntryMetadataSize(key);
+  EXPECT_EQ((count - 1) * count / 2 + total_metadata_size, result);
 
   // Add another entry and test if the size is updated. Then remove it and test
   // if the size is back to original value.
@@ -1734,32 +1745,17 @@ void DiskCacheBackendTest::BackendCalculateSizeOfAllEntries() {
     entry->Close();
 
     int new_result = CalculateSizeOfAllEntries();
-    if (memory_only_ || simple_cache_mode_) {
-      // TODO(msramek): Implement.
-      EXPECT_EQ(net::ERR_NOT_IMPLEMENTED, new_result);
-    } else {
-      EXPECT_EQ(result + last_entry_size + static_cast<int>(key.size()),
-                new_result);
-    }
+    EXPECT_EQ(result + last_entry_size + GetEntryMetadataSize(key),
+              new_result);
 
     DoomEntry(key);
     new_result = CalculateSizeOfAllEntries();
-    if (memory_only_ || simple_cache_mode_) {
-      // TODO(msramek): Implement.
-      EXPECT_EQ(net::ERR_NOT_IMPLEMENTED, new_result);
-    } else {
-      EXPECT_EQ(result, new_result);
-    }
+    EXPECT_EQ(result, new_result);
   }
 
   // After dooming the entries, the size should be back to zero.
   ASSERT_EQ(net::OK, DoomAllEntries());
-  if (memory_only_ || simple_cache_mode_) {
-    // TODO(msramek): Implement.
-    EXPECT_EQ(net::ERR_NOT_IMPLEMENTED, CalculateSizeOfAllEntries());
-  } else {
-    EXPECT_EQ(0, CalculateSizeOfAllEntries());
-  }
+  EXPECT_EQ(0, CalculateSizeOfAllEntries());
 }
 
 TEST_F(DiskCacheBackendTest, CalculateSizeOfAllEntries) {