SimpleCache: open with fewer seeks.

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 864c05ac8ac7263cb69726376b0c4827f8b96004..2153c158de1209a0609ceff8e8b1f78065d531c3 100644
--- a/net/disk_cache/simple/simple_synchronous_entry.cc
+++ b/net/disk_cache/simple/simple_synchronous_entry.cc
@@ -9,6 +9,7 @@
 #include <functional>
 #include <limits>
 
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_util.h"
 #include "base/hash.h"
@@ -39,7 +40,7 @@ enum OpenEntryResult {
   OPEN_ENTRY_BAD_MAGIC_NUMBER = 3,
   OPEN_ENTRY_BAD_VERSION = 4,
   OPEN_ENTRY_CANT_READ_KEY = 5,
-  // OPEN_ENTRY_KEY_MISMATCH = 6, Deprecated.
+  OPEN_ENTRY_KEY_MISMATCH = 6,
   OPEN_ENTRY_KEY_HASH_MISMATCH = 7,
   OPEN_ENTRY_SPARSE_OPEN_FAILED = 8,
   OPEN_ENTRY_MAX = 9,
@@ -72,6 +73,11 @@ enum CloseResult {
   CLOSE_RESULT_WRITE_FAILURE,
 };
 
+// When opening an entry without knowing the key, the header must be read
+// without knowing the size of the key. This is how much to read initially, to
+// make it likely the entire key is read.
+const size_t kInitialHeaderRead = 64 * 1024;
+
 void RecordSyncOpenResult(net::CacheType cache_type,
                           OpenEntryResult result,
                           bool had_index) {
@@ -130,8 +136,9 @@ namespace disk_cache {
 using simple_util::GetEntryHashKey;
 using simple_util::GetFilenameFromEntryHashAndFileIndex;
 using simple_util::GetSparseFilenameFromEntryHash;
-using simple_util::GetDataSizeFromKeyAndFileSize;
-using simple_util::GetFileSizeFromKeyAndDataSize;
+using simple_util::GetHeaderSize;
+using simple_util::GetDataSizeFromFileSize;
+using simple_util::GetFileSizeFromDataSize;
 using simple_util::GetFileIndexFromStreamIndex;
 
 SimpleEntryStat::SimpleEntryStat(base::Time last_used,
@@ -144,35 +151,34 @@ SimpleEntryStat::SimpleEntryStat(base::Time last_used,
   memcpy(data_size_, data_size, sizeof(data_size_));
 }
 
-int SimpleEntryStat::GetOffsetInFile(const std::string& key,
+int SimpleEntryStat::GetOffsetInFile(size_t key_length,
                                      int offset,
                                      int stream_index) const {
-  const size_t headers_size = sizeof(SimpleFileHeader) + key.size();
+  const size_t headers_size = sizeof(SimpleFileHeader) + key_length;
   const size_t 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 SimpleEntryStat::GetEOFOffsetInFile(size_t key_length,
                                         int stream_index) const {
-  return GetOffsetInFile(key, data_size_[stream_index], stream_index);
+  return GetOffsetInFile(key_length, data_size_[stream_index], stream_index);
 }
 
-int SimpleEntryStat::GetLastEOFOffsetInFile(const std::string& key,
+int SimpleEntryStat::GetLastEOFOffsetInFile(size_t key_length,
                                             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);
+  return GetOffsetInFile(key_length, eof_data_offset, stream_index);
 }
 
-int64_t SimpleEntryStat::GetFileSize(const std::string& key,
-                                     int file_index) const {
+int64_t SimpleEntryStat::GetFileSize(size_t key_length, int file_index) const {
   const int32_t total_data_size =
       file_index == 0 ? data_size_[0] + data_size_[1] + sizeof(SimpleFileEOF)
                       : data_size_[2];
-  return GetFileSizeFromKeyAndDataSize(key, total_data_size);
+  return GetFileSizeFromDataSize(key_length, total_data_size);
 }
 
 SimpleEntryCreationResults::SimpleEntryCreationResults(
@@ -223,17 +229,16 @@ SimpleSynchronousEntry::EntryOperationData::EntryOperationData(
 void SimpleSynchronousEntry::OpenEntry(
     net::CacheType cache_type,
     const FilePath& path,
+    const std::string& key,
     const uint64_t entry_hash,
-    bool had_index,
+    const bool had_index,
     SimpleEntryCreationResults* out_results) {
   base::ElapsedTimer open_time;
   SimpleSynchronousEntry* sync_entry =
-      new SimpleSynchronousEntry(cache_type, path, "", entry_hash);
-  out_results->result =
-      sync_entry->InitializeForOpen(had_index,
-                                    &out_results->entry_stat,
-                                    &out_results->stream_0_data,
-                                    &out_results->stream_0_crc32);
+      new SimpleSynchronousEntry(cache_type, path, key, entry_hash, had_index);
+  out_results->result = sync_entry->InitializeForOpen(
+      &out_results->entry_stat, &out_results->stream_0_data,
+      &out_results->stream_0_crc32);
   if (out_results->result != net::OK) {
     sync_entry->Doom();
     delete sync_entry;
@@ -251,13 +256,13 @@ void SimpleSynchronousEntry::CreateEntry(
     const FilePath& path,
     const std::string& key,
     const uint64_t entry_hash,
-    bool had_index,
+    const bool had_index,
     SimpleEntryCreationResults* out_results) {
   DCHECK_EQ(entry_hash, GetEntryHashKey(key));
   SimpleSynchronousEntry* sync_entry =
-      new SimpleSynchronousEntry(cache_type, path, key, entry_hash);
-  out_results->result = sync_entry->InitializeForCreate(
-      had_index, &out_results->entry_stat);
+      new SimpleSynchronousEntry(cache_type, path, key, entry_hash, had_index);
+  out_results->result =
+      sync_entry->InitializeForCreate(&out_results->entry_stat);
   if (out_results->result != net::OK) {
     if (out_results->result != net::ERR_FILE_EXISTS)
       sync_entry->Doom();
@@ -298,19 +303,23 @@ void SimpleSynchronousEntry::ReadData(const EntryOperationData& in_entry_op,
                                       net::IOBuffer* out_buf,
                                       uint32_t* out_crc32,
                                       SimpleEntryStat* entry_stat,
-                                      int* out_result) const {
+                                      int* out_result) {
   DCHECK(initialized_);
   DCHECK_NE(0, in_entry_op.index);
-  const int64_t file_offset =
-      entry_stat->GetOffsetInFile(key_, in_entry_op.offset, in_entry_op.index);
   int file_index = GetFileIndexFromStreamIndex(in_entry_op.index);
+  if (header_and_key_check_needed_ && !CheckHeaderAndKeyForOpen(file_index)) {
+    *out_result = net::ERR_FAILED;
+    Doom();
+    return;
+  }
+  const int64_t file_offset = entry_stat->GetOffsetInFile(
+      key_.size(), in_entry_op.offset, in_entry_op.index);
   // Zero-length reads and reads to the empty streams of omitted files should
   // be handled in the SimpleEntryImpl.
   DCHECK_GT(in_entry_op.buf_len, 0);
   DCHECK(!empty_file_omitted_[file_index]);
-  File* file = const_cast<File*>(&files_[file_index]);
-  int bytes_read =
-      file->Read(file_offset, out_buf->data(), in_entry_op.buf_len);
+  int bytes_read = files_[file_index].Read(file_offset, out_buf->data(),
+                                           in_entry_op.buf_len);
   if (bytes_read > 0) {
     entry_stat->set_last_used(Time::Now());
     *out_crc32 = crc32(crc32(0L, Z_NULL, 0),
@@ -338,7 +347,7 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
   bool truncate = in_entry_op.truncate;
   bool doomed = in_entry_op.doomed;
   const int64_t file_offset = out_entry_stat->GetOffsetInFile(
-      key_, in_entry_op.offset, in_entry_op.index);
+      key_.size(), in_entry_op.offset, in_entry_op.index);
   bool extending_by_write = offset + buf_len > out_entry_stat->data_size(index);
 
   if (empty_file_omitted_[file_index]) {
@@ -371,7 +380,7 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
   if (extending_by_write) {
     // The EOF record and the eventual stream afterward need to be zeroed out.
     const int64_t file_eof_offset =
-        out_entry_stat->GetEOFOffsetInFile(key_, index);
+        out_entry_stat->GetEOFOffsetInFile(key_.size(), index);
     if (!files_[file_index].SetLength(file_eof_offset)) {
       RecordWriteResult(cache_type_, WRITE_RESULT_PRETRUNCATE_FAILURE);
       Doom();
@@ -393,7 +402,8 @@ void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
         index, std::max(out_entry_stat->data_size(index), offset + buf_len));
   } else {
     out_entry_stat->set_data_size(index, offset + buf_len);
-    int file_eof_offset = out_entry_stat->GetLastEOFOffsetInFile(key_, index);
+    int file_eof_offset =
+        out_entry_stat->GetLastEOFOffsetInFile(key_.size(), index);
     if (!files_[file_index].SetLength(file_eof_offset)) {
       RecordWriteResult(cache_type_, WRITE_RESULT_TRUNCATE_FAILURE);
       Doom();
@@ -644,7 +654,7 @@ void SimpleSynchronousEntry::Close(
     net::GrowableIOBuffer* stream_0_data) {
   DCHECK(stream_0_data);
   // Write stream 0 data.
-  int stream_0_offset = entry_stat.GetOffsetInFile(key_, 0, 0);
+  int stream_0_offset = entry_stat.GetOffsetInFile(key_.size(), 0, 0);
   if (files_[0].Write(stream_0_offset, stream_0_data->data(),
                       entry_stat.data_size(0)) !=
       entry_stat.data_size(0)) {
@@ -667,7 +677,7 @@ void SimpleSynchronousEntry::Close(
     if (it->has_crc32)
       eof_record.flags |= SimpleFileEOF::FLAG_HAS_CRC32;
     eof_record.data_crc32 = it->data_crc32;
-    int eof_offset = entry_stat.GetEOFOffsetInFile(key_, stream_index);
+    int eof_offset = entry_stat.GetEOFOffsetInFile(key_.size(), stream_index);
     // If stream 0 changed size, the file needs to be resized, otherwise the
     // next open will yield wrong stream sizes. On stream 1 and stream 2 proper
     // resizing of the file is handled in SimpleSynchronousEntry::WriteData().
@@ -693,7 +703,7 @@ void SimpleSynchronousEntry::Close(
       continue;
 
     files_[i].Close();
-    const int64_t file_size = entry_stat.GetFileSize(key_, i);
+    const int64_t file_size = entry_stat.GetFileSize(key_.size(), i);
     SIMPLE_CACHE_UMA(CUSTOM_COUNTS,
                      "LastClusterSize", cache_type_,
                      file_size % 4096, 0, 4097, 50);
@@ -720,13 +730,16 @@ void SimpleSynchronousEntry::Close(
 SimpleSynchronousEntry::SimpleSynchronousEntry(net::CacheType cache_type,
                                                const FilePath& path,
                                                const std::string& key,
-                                               const uint64_t entry_hash)
+                                               const uint64_t entry_hash,
+                                               const bool had_index)
     : cache_type_(cache_type),
       path_(path),
       entry_hash_(entry_hash),
+      had_index_(had_index),
       key_(key),
       have_open_files_(false),
-      initialized_(false) {
+      initialized_(false),
+      header_and_key_check_needed_(false) {
   for (int i = 0; i < kSimpleEntryFileCount; ++i)
     empty_file_omitted_[i] = false;
 }
@@ -779,21 +792,19 @@ bool SimpleSynchronousEntry::MaybeCreateFile(
   return files_[file_index].IsValid();
 }
 
-bool SimpleSynchronousEntry::OpenFiles(
-    bool had_index,
-    SimpleEntryStat* out_entry_stat) {
+bool SimpleSynchronousEntry::OpenFiles(SimpleEntryStat* out_entry_stat) {
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     File::Error error;
     if (!MaybeOpenFile(i, &error)) {
       // TODO(juliatuttle,gavinp): Remove one each of these triplets of
       // histograms. We can calculate the third as the sum or difference of the
       // other two.
-      RecordSyncOpenResult(
-          cache_type_, OPEN_ENTRY_PLATFORM_FILE_ERROR, had_index);
+      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_PLATFORM_FILE_ERROR,
+                           had_index_);
       SIMPLE_CACHE_UMA(ENUMERATION,
                        "SyncOpenPlatformFileError", cache_type_,
                        -error, -base::File::FILE_ERROR_MAX);
-      if (had_index) {
+      if (had_index_) {
         SIMPLE_CACHE_UMA(ENUMERATION,
                          "SyncOpenPlatformFileError_WithIndex", cache_type_,
                          -error, -base::File::FILE_ERROR_MAX);
@@ -860,20 +871,18 @@ bool SimpleSynchronousEntry::OpenFiles(
   return true;
 }
 
-bool SimpleSynchronousEntry::CreateFiles(
-    bool had_index,
-    SimpleEntryStat* out_entry_stat) {
+bool SimpleSynchronousEntry::CreateFiles(SimpleEntryStat* out_entry_stat) {
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     File::Error error;
     if (!MaybeCreateFile(i, FILE_NOT_REQUIRED, &error)) {
       // TODO(juliatuttle,gavinp): Remove one each of these triplets of
       // histograms. We can calculate the third as the sum or difference of the
       // other two.
-      RecordSyncCreateResult(CREATE_ENTRY_PLATFORM_FILE_ERROR, had_index);
+      RecordSyncCreateResult(CREATE_ENTRY_PLATFORM_FILE_ERROR, had_index_);
       SIMPLE_CACHE_UMA(ENUMERATION,
                        "SyncCreatePlatformFileError", cache_type_,
                        -error, -base::File::FILE_ERROR_MAX);
-      if (had_index) {
+      if (had_index_) {
         SIMPLE_CACHE_UMA(ENUMERATION,
                          "SyncCreatePlatformFileError_WithIndex", cache_type_,
                          -error, -base::File::FILE_ERROR_MAX);
@@ -919,13 +928,79 @@ void SimpleSynchronousEntry::CloseFiles() {
     CloseFile(i);
 }
 
+bool SimpleSynchronousEntry::CheckHeaderAndKeyForOpen(int file_index) {

[Randy Smith (Not in Mondays), 2016/05/18 16:41:30]

Given that this function is no longer being called during open, maybe a
different name?  Maybe just deleting "For Open"?

[gavinp, 2016/05/18 19:05:02]

Well, it's still only for open... It's just that open completes later sometimes.
But yes, it doesn't contribute meaning, so I'm removing it.

+  // TODO(gavinp): Frequently we are doing this at the same time as we read from
+  // the beginning of an entry. It might improve performance to make a single
+  // read(2) call rather than two separate reads. On the other hand, it would
+  // mean an extra memory to memory copy.

[Randy Smith (Not in Mondays), 2016/05/17 20:04:07]

Hmmm.  The other issue I'd add to this comment (presuming you agree) is the
waste if kInitialHeaderRead actually reads into the entry; then it's not just
the two separate reads, but the two reads doing the same thing.  I presume that
won't happen often because you'll usually have the key_ and GetHeaderSize() is
mostly reliable?

[gavinp, 2016/05/18 15:46:00]

Yeah, the key is almost always present. And in iteration, it's unlikely that the
entry will subsequently be read. More typically, it's doomed. 

Called it out.

[Randy Smith (Not in Mondays), 2016/05/18 16:41:29]

Ah, good point; I hadn't kept in mind no key -> iteration -> probably just
deleting based on time.  You're welcome to go back to the old comment or not as
you choose.

+  std::vector<char> header_data(key_.empty() ? kInitialHeaderRead
+                                             : GetHeaderSize(key_.size()));
+  int bytes_read =
+      files_[file_index].Read(0, header_data.data(), header_data.size());
+  const SimpleFileHeader* header;
+
+  if (bytes_read == -1 || static_cast<size_t>(bytes_read) < sizeof(*header)) {
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_HEADER, had_index_);
+    return false;
+  }
+  header_data.resize(bytes_read);
+  header = reinterpret_cast<const SimpleFileHeader*>(header_data.data());

[Randy Smith (Not in Mondays), 2016/05/17 20:04:07]

Maybe comment here about dodging the resize() repositioning chainsaw?

[gavinp, 2016/05/18 15:46:00]

Actually, I checked, and resizes down in size (which this is) does not adjust
capacity(), so there's no iterator invalidation.

+
+  if (header->initial_magic_number != kSimpleInitialMagicNumber) {
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_BAD_MAGIC_NUMBER, had_index_);
+    return false;
+  }
+
+  if (header->version != kSimpleEntryVersionOnDisk) {
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_BAD_VERSION, had_index_);
+    return false;
+  }
+
+  size_t expected_header_size = GetHeaderSize(header->key_length);

[gavinp, 2016/05/16 23:19:28]

I think this needs a unit test; just something with a key size that's large
enough to trigger this read, and confirmation that it can be iterated.

[Randy Smith (Not in Mondays), 2016/05/17 20:04:07]

Agreed.

[gavinp, 2016/05/18 15:46:00]

Done.

+  if (header_data.size() < expected_header_size) {
+    size_t old_size = header_data.size();
+    header_data.resize(expected_header_size);
+    int bytes_read =
+        files_[file_index].Read(old_size, header_data.data() + old_size,
+                                expected_header_size - old_size);
+    if (bytes_read == -1)
+      header_data.resize(old_size);

[Randy Smith (Not in Mondays), 2016/05/17 20:04:07]

Call out that this is an error path?

[gavinp, 2016/05/18 15:46:00]

Done.

+    else
+      header_data.resize(old_size + bytes_read);
+    // The resize calls invalidated |*header|, so it must be reset.

[Randy Smith (Not in Mondays), 2016/05/18 16:41:30]

Why delete the comment?  This was an expansion, so it may invalidate iterators.

[gavinp, 2016/05/18 19:05:02]

This was not an expansion, because of the resize a few lines above. It's gone
now anyway, so moot.

+    header = reinterpret_cast<const SimpleFileHeader*>(header_data.data());
+  }
+
+  if (header_data.size() < sizeof(*header) + header->key_length) {
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_KEY, had_index_);
+    return false;
+  }
+  char* key_data = header_data.data() + sizeof(*header);
+  if (base::Hash(key_data, header->key_length) != header->key_hash) {
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_KEY_HASH_MISMATCH, had_index_);
+    return false;
+  }
+
+  std::string key_from_header(key_data, header->key_length);
+  if (key_.empty()) {
+    key_.swap(key_from_header);
+  } else {
+    if (key_ != key_from_header) {
+      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_KEY_MISMATCH, had_index_);
+      return false;
+    }
+  }
+
+  header_and_key_check_needed_ = false;
+  return true;
+}
+
 int SimpleSynchronousEntry::InitializeForOpen(
-    bool had_index,
     SimpleEntryStat* out_entry_stat,
     scoped_refptr<net::GrowableIOBuffer>* stream_0_data,
     uint32_t* out_stream_0_crc32) {
   DCHECK(!initialized_);
-  if (!OpenFiles(had_index, out_entry_stat)) {
+  if (!OpenFiles(out_entry_stat)) {
     DLOG(WARNING) << "Could not open platform files for entry.";
     return net::ERR_FAILED;
   }
@@ -933,69 +1008,38 @@ int SimpleSynchronousEntry::InitializeForOpen(
     if (empty_file_omitted_[i])
       continue;
 
-    SimpleFileHeader header;
-    int header_read_result =
-        files_[i].Read(0, reinterpret_cast<char*>(&header), sizeof(header));
-    if (header_read_result != sizeof(header)) {
-      DLOG(WARNING) << "Cannot read header from entry.";
-      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_HEADER, had_index);
-      return net::ERR_FAILED;
-    }
-
-    if (header.initial_magic_number != kSimpleInitialMagicNumber) {
-      // TODO(gavinp): This seems very bad; for now we log at WARNING, but we
-      // should give consideration to not saturating the log with these if that
-      // becomes a problem.
-      DLOG(WARNING) << "Magic number did not match.";
-      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_BAD_MAGIC_NUMBER, had_index);
-      return net::ERR_FAILED;
-    }
-
-    if (header.version != kSimpleEntryVersionOnDisk) {
-      DLOG(WARNING) << "Unreadable version.";
-      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_BAD_VERSION, had_index);
-      return net::ERR_FAILED;
-    }
-
-    std::unique_ptr<char[]> key(new char[header.key_length]);
-    int key_read_result = files_[i].Read(sizeof(header), key.get(),
-                                         header.key_length);
-    if (key_read_result != base::checked_cast<int>(header.key_length)) {
-      DLOG(WARNING) << "Cannot read key from entry.";
-      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_KEY, had_index);
-      return net::ERR_FAILED;
+    if (!key_.empty() &&
+        base::CommandLine::ForCurrentProcess()->HasSwitch(
+            "simple-cache-dont-read-initial-header")) {

[gavinp, 2016/05/16 23:19:28]

Oh, yes, this must be removed for landing. I was using this for testing the
impact of this change using the disk cache perftool.

[Randy Smith (Not in Mondays), 2016/05/17 20:04:07]

Acknowledged.

[Randy Smith (Not in Mondays), 2016/05/18 16:41:29]

I'd feel better about it being gone in the PS I stamp?  What's blocking its
removal?

[gavinp, 2016/05/18 19:05:02]

Nothing, done. It was just convenient for me to use this CL to move it around
platform to platform. But we're far enough along now that removing it is right.

+      header_and_key_check_needed_ = true;
+    } else {
+      if (!CheckHeaderAndKeyForOpen(i))
+        return net::ERR_FAILED;
     }
 
-    key_ = std::string(key.get(), header.key_length);
     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));
+          GetDataSizeFromFileSize(key_.size(), out_entry_stat->data_size(1));
       int ret_value_stream_0 = ReadAndValidateStream0(
           total_data_size, out_entry_stat, stream_0_data, out_stream_0_crc32);
       if (ret_value_stream_0 != net::OK)
         return ret_value_stream_0;
     } else {
       out_entry_stat->set_data_size(
-          2, GetDataSizeFromKeyAndFileSize(key_, out_entry_stat->data_size(2)));
+          2,
+          GetDataSizeFromFileSize(key_.size(), out_entry_stat->data_size(2)));
       if (out_entry_stat->data_size(2) < 0) {
         DLOG(WARNING) << "Stream 2 file is too small.";
         return net::ERR_FAILED;
       }
     }
-
-    if (base::Hash(key.get(), header.key_length) != header.key_hash) {
-      DLOG(WARNING) << "Hash mismatch on key.";
-      RecordSyncOpenResult(
-          cache_type_, OPEN_ENTRY_KEY_HASH_MISMATCH, had_index);
-      return net::ERR_FAILED;
-    }
   }
 
   int32_t sparse_data_size = 0;
   if (!OpenSparseFileIfExists(&sparse_data_size)) {
-    RecordSyncOpenResult(
-        cache_type_, OPEN_ENTRY_SPARSE_OPEN_FAILED, had_index);
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_SPARSE_OPEN_FAILED,
+                         had_index_);
     return net::ERR_FAILED;
   }
   out_entry_stat->set_sparse_data_size(sparse_data_size);
@@ -1015,7 +1059,7 @@ int SimpleSynchronousEntry::InitializeForOpen(
   SIMPLE_CACHE_UMA(BOOLEAN, "EntryOpenedAndStream2Removed", cache_type_,
                    removed_stream2);
 
-  RecordSyncOpenResult(cache_type_, OPEN_ENTRY_SUCCESS, had_index);
+  RecordSyncOpenResult(cache_type_, OPEN_ENTRY_SUCCESS, had_index_);
   initialized_ = true;
   return net::OK;
 }
@@ -1048,10 +1092,9 @@ bool SimpleSynchronousEntry::InitializeCreatedFile(
 }
 
 int SimpleSynchronousEntry::InitializeForCreate(
-    bool had_index,
     SimpleEntryStat* out_entry_stat) {
   DCHECK(!initialized_);
-  if (!CreateFiles(had_index, out_entry_stat)) {
+  if (!CreateFiles(out_entry_stat)) {
     DLOG(WARNING) << "Could not create platform files.";
     return net::ERR_FILE_EXISTS;
   }
@@ -1061,11 +1104,11 @@ int SimpleSynchronousEntry::InitializeForCreate(
 
     CreateEntryResult result;
     if (!InitializeCreatedFile(i, &result)) {
-      RecordSyncCreateResult(result, had_index);
+      RecordSyncCreateResult(result, had_index_);
       return net::ERR_FAILED;
     }
   }
-  RecordSyncCreateResult(CREATE_ENTRY_SUCCESS, had_index);
+  RecordSyncCreateResult(CREATE_ENTRY_SUCCESS, had_index_);
   initialized_ = true;
   return net::OK;
 }
@@ -1099,7 +1142,7 @@ int SimpleSynchronousEntry::ReadAndValidateStream0(
   // Put stream 0 data in memory.
   *stream_0_data = new net::GrowableIOBuffer();
   (*stream_0_data)->SetCapacity(stream_0_size);
-  int file_offset = out_entry_stat->GetOffsetInFile(key_, 0, 0);
+  int file_offset = out_entry_stat->GetOffsetInFile(key_.size(), 0, 0);
   File* file = const_cast<File*>(&files_[0]);
   int bytes_read =
       file->Read(file_offset, (*stream_0_data)->data(), stream_0_size);
@@ -1129,7 +1172,7 @@ int SimpleSynchronousEntry::GetEOFRecordData(int index,
                                              uint32_t* out_crc32,
                                              int* out_data_size) const {
   SimpleFileEOF eof_record;
-  int file_offset = entry_stat.GetEOFOffsetInFile(key_, index);
+  int file_offset = entry_stat.GetEOFOffsetInFile(key_.size(), index);
   int file_index = GetFileIndexFromStreamIndex(index);
   File* file = const_cast<File*>(&files_[file_index]);
   if (file->Read(file_offset, reinterpret_cast<char*>(&eof_record),