SimpleCache: merge the first and second stream in one file

net/disk_cache/simple/simple_synchronous_entry.cc

Index: net/disk_cache/simple/simple_synchronous_entry.cc
diff --git a/net/disk_cache/simple/simple_synchronous_entry.cc b/net/disk_cache/simple/simple_synchronous_entry.cc
index a1ff1ac6adaab1664d6a61656ed1a1af93a0d308..013c6bf0e4d3a9c6480415ea807b04437d5f2364 100644
--- a/net/disk_cache/simple/simple_synchronous_entry.cc
+++ b/net/disk_cache/simple/simple_synchronous_entry.cc
@@ -143,10 +143,10 @@ namespace disk_cache {
 
 using simple_util::ConvertEntryHashKeyToHexString;

[pasko, 2013/09/17 16:40:52]

this one is not used

[clamy, 2013/09/18 16:17:15]

Done.

 using simple_util::GetEntryHashKey;
-using simple_util::GetFilenameFromEntryHashAndIndex;
+using simple_util::GetFilenameFromEntryHashAndFileIndex;
 using simple_util::GetDataSizeFromKeyAndFileSize;
 using simple_util::GetFileSizeFromKeyAndDataSize;
-using simple_util::GetFileOffsetFromKeyAndDataOffset;
+using simple_util::GetFileIndexFromStreamIndex;
 
 SimpleEntryStat::SimpleEntryStat() {}

[pasko, 2013/09/17 16:40:52]

we do not need this constructor

[clamy, 2013/09/18 16:17:15]

Done.

 
@@ -158,6 +158,34 @@ SimpleEntryStat::SimpleEntryStat(base::Time last_used_p,
   memcpy(data_size, data_size_p, sizeof(data_size));
 }
 
+int SimpleEntryStat::GetOffsetInFile(const std::string& key,
+                                     int offset,
+                                     int stream_index) const {
+  const int64 headers_size = sizeof(SimpleFileHeader) + key.size();
+  const int64 additional_offset =
+      stream_index == 0 ? data_size[1] + sizeof(SimpleFileEOF) : 0;
+  return headers_size + offset + additional_offset;
+}
+
+int SimpleEntryStat::GetEOFOffsetInFile(const std::string& key,
+                                        int stream_index) const {
+  return GetOffsetInFile(key, data_size[stream_index], stream_index);
+}
+
+int SimpleEntryStat::GetLastEOFOffsetInFile(const std::string& key,
+                                            int stream_index) const {
+  const int file_index = GetFileIndexFromStreamIndex(stream_index);
+  const int eof_data_offset = file_index == 0 ?
+      data_size[0] + data_size[1] + sizeof(SimpleFileEOF) : data_size[2];
+  return GetOffsetInFile(key, eof_data_offset, stream_index);
+}
+
+int SimpleEntryStat::GetFileSize(const std::string& key, int file_index) const {
+  const int total_data_size = file_index == 0 ?
+      data_size[0] + data_size[1] + sizeof(SimpleFileEOF) : data_size[2];
+  return GetFileSizeFromKeyAndDataSize(key, total_data_size);
+}
+
 SimpleEntryCreationResults::SimpleEntryCreationResults(
     SimpleEntryStat entry_stat)
     : sync_entry(NULL),
@@ -206,11 +234,12 @@ void SimpleSynchronousEntry::OpenEntry(
   SimpleSynchronousEntry* sync_entry =
       new SimpleSynchronousEntry(cache_type, path, "", entry_hash);
   out_results->result = sync_entry->InitializeForOpen(
-      had_index, &out_results->entry_stat);
+      had_index, &out_results->entry_stat, &out_results->stream_0_data);
   if (out_results->result != net::OK) {
     sync_entry->Doom();
     delete sync_entry;
     out_results->sync_entry = NULL;
+    out_results->stream_0_data = NULL;
     return;
   }
   out_results->sync_entry = sync_entry;
@@ -246,8 +275,8 @@ bool SimpleSynchronousEntry::DeleteFilesForEntryHash(
     const uint64 entry_hash) {
   bool result = true;
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
-    FilePath to_delete = path.AppendASCII(
-        GetFilenameFromEntryHashAndIndex(entry_hash, i));
+    FilePath to_delete =
+        path.AppendASCII(GetFilenameFromEntryHashAndFileIndex(entry_hash, i));
     if (!base::DeleteFile(to_delete, false)) {
       result = false;
       DLOG(ERROR) << "Could not delete " << to_delete.MaybeAsASCII();
@@ -279,17 +308,16 @@ int SimpleSynchronousEntry::DoomEntrySet(
 void SimpleSynchronousEntry::ReadData(const EntryOperationData& in_entry_op,
                                       net::IOBuffer* out_buf,
                                       uint32* out_crc32,
-                                      base::Time* out_last_used,
+                                      SimpleEntryStat* out_entry_stat,
                                       int* out_result) const {
   DCHECK(initialized_);
-  int64 file_offset =
-      GetFileOffsetFromKeyAndDataOffset(key_, in_entry_op.offset);
-  int bytes_read = ReadPlatformFile(files_[in_entry_op.index],
-                                    file_offset,
-                                    out_buf->data(),
-                                    in_entry_op.buf_len);
+  const int64 file_offset = out_entry_stat->GetOffsetInFile(

[gavinp, 2013/09/17 15:07:43]

That doesn't look like an output parameter! Perhaps rename it to "entry_stat" ? 

Is this necessary, though? Is it legal to call ReadData for stream 0? If we make
that illegal, we can get away without this parameter being in/out.

[pasko, 2013/09/17 16:40:52]

seems like a cool simple strategy to assert against reading from stream 0

[pasko, 2013/09/18 12:03:19]

.. but on the other hand I also like the uniformity of using the
SimpleEntryStat::GetOffsetInFile() here

[clamy, 2013/09/18 16:17:15]

All offset functions are now used part of EntryStat, so we need one to compute
the offset. I don't really want to reintroduce them. I could create a fake entry
stat with 0 in data_size, since it is not called on stream 0, wdyt?

[gavinp, 2013/09/18 17:20:35]

Or you could just rename the parameter to remove the "out_" and call that good
enough. Probably we should have the asserts though, just because it's too weird
if anyone reads from stream 0.

[clamy, 2013/09/18 18:29:38]

Done.

+      key_, in_entry_op.offset, in_entry_op.index);
+  int file_index = GetFileIndexFromStreamIndex(in_entry_op.index);
+  int bytes_read = ReadPlatformFile(
+      files_[file_index], file_offset, out_buf->data(), in_entry_op.buf_len);
   if (bytes_read > 0) {
-    *out_last_used = Time::Now();
+    out_entry_stat->last_used = Time::Now();
     *out_crc32 = crc32(crc32(0L, Z_NULL, 0),
                        reinterpret_cast<const Bytef*>(out_buf->data()),
                        bytes_read);
@@ -308,26 +336,28 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
                                        int* out_result) const {
   DCHECK(initialized_);
   int index = in_entry_op.index;

[pasko, 2013/09/18 12:03:19]

please add a DCHECK_NE(0, index), similarly in SSE::ReadData

[clamy, 2013/09/18 16:17:15]

Done.

+  int file_index = GetFileIndexFromStreamIndex(index);
   int offset = in_entry_op.offset;
   int buf_len = in_entry_op.buf_len;
   int truncate = in_entry_op.truncate;
-
+  const int64 file_offset = out_entry_stat->GetOffsetInFile(
+      key_, in_entry_op.offset, in_entry_op.index);
   bool extending_by_write = offset + buf_len > out_entry_stat->data_size[index];
   if (extending_by_write) {
-    // We are extending the file, and need to insure the EOF record is zeroed.
-    const int64 file_eof_offset = GetFileOffsetFromKeyAndDataOffset(
-        key_, out_entry_stat->data_size[index]);
-    if (!TruncatePlatformFile(files_[index], file_eof_offset)) {
+    // The EOF record and the eventual stream afterward need to be zeroed out.
+    const int64 file_eof_offset =
+        out_entry_stat->GetEOFOffsetInFile(key_, index);
+    if (!TruncatePlatformFile(files_[file_index], file_eof_offset)) {
       RecordWriteResult(cache_type_, WRITE_RESULT_PRETRUNCATE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
   }
-  const int64 file_offset = GetFileOffsetFromKeyAndDataOffset(key_, offset);
   if (buf_len > 0) {
     if (WritePlatformFile(
-            files_[index], file_offset, in_buf->data(), buf_len) != buf_len) {
+            files_[file_index], file_offset, in_buf->data(), buf_len) !=
+        buf_len) {
       RecordWriteResult(cache_type_, WRITE_RESULT_WRITE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
@@ -338,13 +368,14 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
     out_entry_stat->data_size[index] =
         std::max(out_entry_stat->data_size[index], offset + buf_len);
   } else {
-    if (!TruncatePlatformFile(files_[index], file_offset + buf_len)) {
+    out_entry_stat->data_size[index] = offset + buf_len;
+    int file_eof_offset = out_entry_stat->GetLastEOFOffsetInFile(key_, index);
+    if (!TruncatePlatformFile(files_[file_index], file_eof_offset)) {
       RecordWriteResult(cache_type_, WRITE_RESULT_TRUNCATE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
-    out_entry_stat->data_size[index] = offset + buf_len;

[pasko, 2013/09/18 12:03:19]

nit:

is there a reason to move this line above the truncation? In case truncation
fails it seems more natural to leave the data size unchanged. This would be on a
Doomed entry, so not too interesting, but still raises unnecessary questions
when reading.

[clamy, 2013/09/18 16:17:15]

Otherwise out_entry_stat will return the wrong offset for truncation.

   }
 
   RecordWriteResult(cache_type_, WRITE_RESULT_SUCCESS);
@@ -353,61 +384,63 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
 }
 
 void SimpleSynchronousEntry::CheckEOFRecord(int index,
-                                            int32 data_size,
+                                            const SimpleEntryStat& entry_stat,
                                             uint32 expected_crc32,
                                             int* out_result) const {
   DCHECK(initialized_);
-
-  SimpleFileEOF eof_record;
-  int64 file_offset = GetFileOffsetFromKeyAndDataOffset(key_, data_size);
-  if (ReadPlatformFile(files_[index],
-                       file_offset,
-                       reinterpret_cast<char*>(&eof_record),
-                       sizeof(eof_record)) != sizeof(eof_record)) {
-    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_READ_FAILURE);
-    Doom();
-    *out_result = net::ERR_CACHE_CHECKSUM_READ_FAILURE;
-    return;
-  }
-
-  if (eof_record.final_magic_number != kSimpleFinalMagicNumber) {
-    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH);
-    DLOG(INFO) << "eof record had bad magic number.";
+  uint32 crc32;
+  bool has_crc32;
+  int stream_size;
+  *out_result =
+      GetEOFRecordData(index, entry_stat, &has_crc32, &crc32, &stream_size);
+  if (*out_result != net::OK) {
     Doom();
-    *out_result = net::ERR_CACHE_CHECKSUM_READ_FAILURE;
     return;
   }
-
-  const bool has_crc = (eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) ==
-                       SimpleFileEOF::FLAG_HAS_CRC32;
-  SIMPLE_CACHE_UMA(BOOLEAN, "SyncCheckEOFHasCrc", cache_type_, has_crc);
-  if (has_crc && eof_record.data_crc32 != expected_crc32) {
+  if (has_crc32 && crc32 != expected_crc32) {
+    DLOG(INFO) << "Eof record had bad crc.";

[pasko, 2013/09/18 12:03:19]

nit: s/Eof/EOF/, same in other places

[clamy, 2013/09/18 16:17:15]

Done.

+    *out_result = net::ERR_CACHE_CHECKSUM_MISMATCH;
     RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH);
-    DLOG(INFO) << "eof record had bad crc.";
     Doom();
-    *out_result = net::ERR_CACHE_CHECKSUM_MISMATCH;
     return;
   }
-
   RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_SUCCESS);
-  *out_result = net::OK;
 }
 
 void SimpleSynchronousEntry::Close(
     const SimpleEntryStat& entry_stat,
-    scoped_ptr<std::vector<CRCRecord> > crc32s_to_write) {
+    scoped_ptr<std::vector<CRCRecord> > crc32s_to_write,
+    net::GrowableIOBuffer* stream_0_data) {
+  if (!stream_0_data) {
+    if (entry_stat.data_size[0])
+      Doom();
+  } else {
+    // Write stream 0 data.
+    int stream_0_offset = entry_stat.GetOffsetInFile(key_, 0, 0);
+    if (WritePlatformFile(files_[0],
+                          stream_0_offset,
+                          stream_0_data->data(),
+                          entry_stat.data_size[0]) != entry_stat.data_size[0]) {
+      RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
+      DLOG(INFO) << "Could not write stream 0 data.";
+      Doom();
+    }
+  }
+
   for (std::vector<CRCRecord>::const_iterator it = crc32s_to_write->begin();
        it != crc32s_to_write->end(); ++it) {
     SimpleFileEOF eof_record;
+    int index = it->index;
+    eof_record.stream_size = entry_stat.data_size[index];
     eof_record.final_magic_number = kSimpleFinalMagicNumber;
     eof_record.flags = 0;
     if (it->has_crc32)
       eof_record.flags |= SimpleFileEOF::FLAG_HAS_CRC32;
     eof_record.data_crc32 = it->data_crc32;
-    int64 file_offset = GetFileOffsetFromKeyAndDataOffset(
-        key_, entry_stat.data_size[it->index]);
-    if (WritePlatformFile(files_[it->index],
-                          file_offset,
+    int file_index = GetFileIndexFromStreamIndex(index);
+    int eof_offset = entry_stat.GetEOFOffsetInFile(key_, index);
+    if (WritePlatformFile(files_[file_index],

[pasko, 2013/09/18 15:08:16]

With your other CL for testing all streams (crrev.com/23702024 awesome change
btw) I found an interesting case. If the file was previously larger, this write
would not change the size, hence we'd get wrong stream sizes at the next open.

this is fixable with:
if (index != 1 && !TruncatePlatformFile(files_[file_index], eof_offset)) {
  // handle error
}

The "index != 1" is necessary because we write EOFs in the backwards order. This
cute detail needs a comment. 

[clamy, 2013/09/18 16:17:15]

Done.

+                          eof_offset,
                           reinterpret_cast<const char*>(&eof_record),
                           sizeof(eof_record)) != sizeof(eof_record)) {
       RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
@@ -415,7 +448,11 @@ void SimpleSynchronousEntry::Close(
       Doom();
       break;
     }
-    const int64 file_size = file_offset + sizeof(eof_record);
+  }
+  for (int i = 0; i < kSimpleEntryFileCount; ++i) {
+    bool did_close_file = ClosePlatformFile(files_[i]);
+    CHECK(did_close_file);

[pasko, 2013/09/18 12:03:19]

we don't want to crash the browser here on a file error, so DCHECK

[clamy, 2013/09/18 16:17:15]

Done.

+    const int64 file_size = entry_stat.GetFileSize(key_, i);
     SIMPLE_CACHE_UMA(CUSTOM_COUNTS,
                      "LastClusterSize", cache_type_,
                      file_size % 4096, 0, 4097, 50);
@@ -424,11 +461,6 @@ void SimpleSynchronousEntry::Close(
                      "LastClusterLossPercent", cache_type_,
                      cluster_loss * 100 / (cluster_loss + file_size));
   }
-
-  for (int i = 0; i < kSimpleEntryFileCount; ++i) {
-    bool did_close_file = ClosePlatformFile(files_[i]);
-    CHECK(did_close_file);
-  }
   RecordCloseResult(cache_type_, CLOSE_RESULT_SUCCESS);
   have_open_files_ = false;
   delete this;
@@ -460,8 +492,8 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(
     bool had_index,
     SimpleEntryStat* out_entry_stat) {
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
-    FilePath filename = path_.AppendASCII(
-        GetFilenameFromEntryHashAndIndex(entry_hash_, i));
+    FilePath filename =
+        path_.AppendASCII(GetFilenameFromEntryHashAndFileIndex(entry_hash_, i));
     int flags = PLATFORM_FILE_READ | PLATFORM_FILE_WRITE;
     if (create)
       flags |= PLATFORM_FILE_CREATE;
@@ -517,7 +549,7 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(
   have_open_files_ = true;
   if (create) {
     out_entry_stat->last_modified = out_entry_stat->last_used = Time::Now();
-    for (int i = 0; i < kSimpleEntryFileCount; ++i)
+    for (int i = 0; i < kSimpleEntryStreamCount; ++i)
       out_entry_stat->data_size[i] = 0;
   } else {
     base::TimeDelta entry_age = base::Time::Now() - base::Time::UnixEpoch();
@@ -540,9 +572,9 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(
       if (stream_age < entry_age)
         entry_age = stream_age;
 
-      // Keep the file size in |data size_| briefly until the key is initialized
-      // properly.
-      out_entry_stat->data_size[i] = file_info.size;
+      // Keep the file size in |data size_| temporarily until the data sizes are
+      // initialized correctly.

[pasko, 2013/09/18 12:03:19]

Let's write a more detailed comment here. The 'data' is confusing. Here we can
explain that sizes of the data streams are not known before the key length is
not read, as well as size of the stream 0 in the final EOF record.

The [i + 1] is a puzzle, explained in InitializeForOpen, but given the
"elegance" of the trick, please explain it in the comment as well.

Frankly, I think this should be refactored to avoid having tricks like these,
but that's certainly for later CLs.

[clamy, 2013/09/18 16:17:15]

Done.

+      out_entry_stat->data_size[i + 1] = file_info.size;
     }
     SIMPLE_CACHE_UMA(CUSTOM_COUNTS,
                      "SyncOpenEntryAge", cache_type_,
@@ -560,12 +592,13 @@ void SimpleSynchronousEntry::CloseFiles() {
   }
 }
 
-int SimpleSynchronousEntry::InitializeForOpen(bool had_index,
-                                              SimpleEntryStat* out_entry_stat) {
+int SimpleSynchronousEntry::InitializeForOpen(
+    bool had_index,
+    SimpleEntryStat* out_entry_stat,
+    scoped_refptr<net::GrowableIOBuffer>* stream_0_data) {
   DCHECK(!initialized_);
   if (!OpenOrCreateFiles(false, had_index, out_entry_stat))
     return net::ERR_FAILED;
-
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     SimpleFileHeader header;
     int header_read_result =
@@ -602,12 +635,19 @@ int SimpleSynchronousEntry::InitializeForOpen(bool had_index,
     }
 
     key_ = std::string(key.get(), header.key_length);
-    out_entry_stat->data_size[i] =
-        GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size[i]);
-    if (out_entry_stat->data_size[i] < 0) {
-      // This entry can't possibly be valid, as it does not have enough space to
-      // store a valid SimpleFileEOF record.
-      return net::ERR_FAILED;
+    if (i == 0) {
+      // File size for stream 0 has been stored temporarily in data_size[1].
+      int total_data_size =
+          GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size[1]);
+      int ret_value_stream_0 = ReadAndValidateStream0(
+          total_data_size, out_entry_stat, stream_0_data);
+      if (ret_value_stream_0 != net::OK)
+        return ret_value_stream_0;
+    } else {
+      out_entry_stat->data_size[2] =
+          GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size[2]);
+      if (out_entry_stat->data_size[2] < 0)
+        return net::ERR_FAILED;
     }
 
     if (base::Hash(key.get(), header.key_length) != header.key_hash) {
@@ -638,16 +678,18 @@ int SimpleSynchronousEntry::InitializeForCreate(
     header.key_length = key_.size();
     header.key_hash = base::Hash(key_);
 
-    if (WritePlatformFile(files_[i], 0, reinterpret_cast<char*>(&header),
-                          sizeof(header)) != sizeof(header)) {
-      DLOG(WARNING) << "Could not write headers to new cache entry.";
+    if (WritePlatformFile(
+            files_[i], 0, reinterpret_cast<char*>(&header), sizeof(header)) !=
+        sizeof(header)) {
+      DLOG(WARNING) << "Could not write cache file header to cache entry.";
       RecordSyncCreateResult(
           cache_type_, CREATE_ENTRY_CANT_WRITE_HEADER, had_index);
       return net::ERR_FAILED;
     }
 
-    if (WritePlatformFile(files_[i], sizeof(header), key_.data(),
-                          key_.size()) != implicit_cast<int>(key_.size())) {
+    if (WritePlatformFile(
+            files_[i], sizeof(SimpleFileHeader), key_.data(), key_.size()) !=
+        implicit_cast<int>(key_.size())) {
       DLOG(WARNING) << "Could not write keys to new cache entry.";
       RecordSyncCreateResult(
           cache_type_, CREATE_ENTRY_CANT_WRITE_KEY, had_index);
@@ -659,6 +701,88 @@ int SimpleSynchronousEntry::InitializeForCreate(
   return net::OK;
 }
 
+int SimpleSynchronousEntry::ReadAndValidateStream0(
+    int total_data_size,
+    SimpleEntryStat* out_entry_stat,
+    scoped_refptr<net::GrowableIOBuffer>* stream_0_data) const {
+  // Temporarily assign all the data size to stream 1 in order to read the
+  // EOF record for stream 0, which contains the size of stream 0.
+  out_entry_stat->data_size[0] = 0;
+  out_entry_stat->data_size[1] = total_data_size - sizeof(SimpleFileEOF);
+
+  bool has_crc32;
+  uint32 read_crc32;
+  int stream_0_size;
+  int ret_value_crc32 = GetEOFRecordData(
+      0, *out_entry_stat, &has_crc32, &read_crc32, &stream_0_size);
+  if (ret_value_crc32 != net::OK)
+    return ret_value_crc32;
+
+  if (stream_0_size > out_entry_stat->data_size[1])
+    return net::ERR_FAILED;
+
+  // These are the real values of data size.
+  out_entry_stat->data_size[0] = stream_0_size;

[pasko, 2013/09/18 12:03:19]

so the function ReadAndValidateStream0() also updates data_size[*] which is
counter-intuitive, but I don't know what to suggest beyond a refactoring to
initialize all stream sizes early. I believe this should be postponed for later.
The quick hack to keep a file size in data_size has just backfired.

+  out_entry_stat->data_size[1] -= stream_0_size;
+
+  // Put stream 0 data in memory.
+  *stream_0_data = NULL;

[pasko, 2013/09/18 12:03:19]

is there a reason in touching this here? the value would not be checked on the
error path

[clamy, 2013/09/18 16:17:15]

Done.

+  if (stream_0_size != 0) {
+    *stream_0_data = new net::GrowableIOBuffer();
+    (*stream_0_data)->SetCapacity(stream_0_size);
+    int file_offset = out_entry_stat->GetOffsetInFile(key_, 0, 0);
+    int bytes_read = ReadPlatformFile(
+        files_[0], file_offset, (*stream_0_data)->data(), stream_0_size);
+    if (bytes_read != stream_0_size)
+      return net::ERR_FAILED;
+  }
+
+  // Check the CRC32.
+  uint32 expected_crc32 =
+      stream_0_size == 0
+          ? crc32(0, Z_NULL, 0)
+          : crc32(crc32(0, Z_NULL, 0),
+                  reinterpret_cast<const Bytef*>((*stream_0_data)->data()),
+                  stream_0_size);
+  if (has_crc32 && read_crc32 != expected_crc32) {
+    DLOG(INFO) << "Eof record had bad crc.";
+    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH);
+    return net::ERR_FAILED;
+  }
+  RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_SUCCESS);
+  return net::OK;
+}
+
+int SimpleSynchronousEntry::GetEOFRecordData(int index,
+                                             const SimpleEntryStat& entry_stat,
+                                             bool* out_has_crc32,
+                                             uint32* out_crc32,
+                                             int* out_data_size) const {
+  SimpleFileEOF eof_record;
+  int file_offset = entry_stat.GetEOFOffsetInFile(key_, index);
+  int file_index = GetFileIndexFromStreamIndex(index);
+  if (ReadPlatformFile(files_[file_index],
+                       file_offset,
+                       reinterpret_cast<char*>(&eof_record),
+                       sizeof(eof_record)) != sizeof(eof_record)) {
+    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_READ_FAILURE);
+    return net::ERR_CACHE_CHECKSUM_READ_FAILURE;

[pasko, 2013/09/18 12:03:19]

fun fact: cache checksum read failure can happen even if we did not have the
checksum in the EOF record

+  }
+
+  if (eof_record.final_magic_number != kSimpleFinalMagicNumber) {
+    RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH);
+    DLOG(INFO) << "Eof record had bad magic number.";
+    return net::ERR_CACHE_CHECKSUM_READ_FAILURE;
+  }
+
+  *out_has_crc32 = (eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) ==
+                   SimpleFileEOF::FLAG_HAS_CRC32;
+  *out_crc32 = eof_record.data_crc32;
+  *out_data_size = eof_record.stream_size;
+  SIMPLE_CACHE_UMA(BOOLEAN, "SyncCheckEOFHasCrc", cache_type_, *out_has_crc32);
+  return net::OK;
+}
+
 void SimpleSynchronousEntry::Doom() const {
   // TODO(gavinp): Consider if we should guard against redundant Doom() calls.
   DeleteFilesForEntryHash(path_, entry_hash_);