SimpleCache: open with fewer seeks.

net/disk_cache/simple/simple_synchronous_entry.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/disk_cache/simple/simple_synchronous_entry.h"
 
 #include <algorithm>
 #include <cstring>
 #include <functional>
 #include <limits>
 
 #include "base/compiler_specific.h"
 #include "base/files/file_util.h"
 #include "base/hash.h"
 #include "base/location.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/sha1.h"
 #include "base/strings/stringprintf.h"
 #include "base/timer/elapsed_timer.h"
+#include "crypto/secure_hash.h"
+#include "net/base/hash_value.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/disk_cache/simple/simple_backend_version.h"
 #include "net/disk_cache/simple/simple_histogram_macros.h"
 #include "net/disk_cache/simple/simple_util.h"
 #include "third_party/zlib/zlib.h"
 
 using base::File;
 using base::FilePath;
 using base::Time;
 
 namespace {
 
 // Used in histograms, please only add entries at the end.
 enum OpenEntryResult {
   OPEN_ENTRY_SUCCESS = 0,
   OPEN_ENTRY_PLATFORM_FILE_ERROR = 1,
   OPEN_ENTRY_CANT_READ_HEADER = 2,
   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,
 };
 
 // Used in histograms, please only add entries at the end.
 enum WriteResult {
   WRITE_RESULT_SUCCESS = 0,
   WRITE_RESULT_PRETRUNCATE_FAILURE,
   WRITE_RESULT_WRITE_FAILURE,
   WRITE_RESULT_TRUNCATE_FAILURE,
   WRITE_RESULT_LAZY_STREAM_ENTRY_DOOMED,
   WRITE_RESULT_LAZY_CREATE_FAILURE,
   WRITE_RESULT_LAZY_INITIALIZE_FAILURE,
   WRITE_RESULT_MAX,
 };
 
 // Used in histograms, please only add entries at the end.
 enum CheckEOFResult {
   CHECK_EOF_RESULT_SUCCESS,
   CHECK_EOF_RESULT_READ_FAILURE,
   CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH,
   CHECK_EOF_RESULT_CRC_MISMATCH,
+  CHECK_EOF_RESULT_KEY_SHA256_MISMATCH,
   CHECK_EOF_RESULT_MAX,
 };
 
 // Used in histograms, please only add entries at the end.
 enum CloseResult {
   CLOSE_RESULT_SUCCESS,
   CLOSE_RESULT_WRITE_FAILURE,
 };
 
+// Used in histograms, please only add entries at the end.
+enum class KeySHA256Result { NOT_PRESENT, MATCHED, NO_MATCH, MAX };
+
 void RecordSyncOpenResult(net::CacheType cache_type,
                           OpenEntryResult result,
                           bool had_index) {
   DCHECK_LT(result, OPEN_ENTRY_MAX);
   SIMPLE_CACHE_UMA(ENUMERATION,
                    "SyncOpenResult", cache_type, result, OPEN_ENTRY_MAX);
   if (had_index) {
     SIMPLE_CACHE_UMA(ENUMERATION,
                      "SyncOpenResult_WithIndex", cache_type,
                      result, OPEN_ENTRY_MAX);
@@ skipping 13 matching lines @@
   SIMPLE_CACHE_UMA(ENUMERATION,
                    "SyncCheckEOFResult", cache_type,
                    result, CHECK_EOF_RESULT_MAX);
 }
 
 void RecordCloseResult(net::CacheType cache_type, CloseResult result) {
   SIMPLE_CACHE_UMA(ENUMERATION,
                    "SyncCloseResult", cache_type, result, WRITE_RESULT_MAX);
 }
 
+void RecordKeySHA256Result(net::CacheType cache_type, KeySHA256Result result) {
+  SIMPLE_CACHE_UMA(ENUMERATION, "SyncKeySHA256Result", cache_type,
+                   static_cast<int>(result),
+                   static_cast<int>(KeySHA256Result::MAX));
+}
+
 bool CanOmitEmptyFile(int file_index) {
   DCHECK_GE(file_index, 0);
   DCHECK_LT(file_index, disk_cache::kSimpleEntryFileCount);
   return file_index == disk_cache::simple_util::GetFileIndexFromStreamIndex(2);
 }
 
 bool TruncatePath(const FilePath& filename_to_truncate) {
   File file_to_truncate;
   int flags = File::FLAG_OPEN | File::FLAG_READ | File::FLAG_WRITE |
               File::FLAG_SHARE_DELETE;
   file_to_truncate.Initialize(filename_to_truncate, flags);
   if (!file_to_truncate.IsValid())
     return false;
   if (!file_to_truncate.SetLength(0))
     return false;
   return true;
 }
 
+void CalculateSHA256OfKey(const std::string& key,
+                          net::SHA256HashValue* out_hash_value) {
+  // Write SHA256 of the key immediately after stream 0 data.
+  std::unique_ptr<crypto::SecureHash> hash(
+      crypto::SecureHash::Create(crypto::SecureHash::SHA256));
+  hash->Update(key.data(), key.size());
+  hash->Finish(out_hash_value, sizeof(*out_hash_value));
+}
+
 }  // namespace
 
 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,
                                  base::Time last_modified,
                                  const int32_t data_size[],
                                  const int32_t sparse_data_size)
     : last_used_(last_used),
       last_modified_(last_modified),
       sparse_data_size_(sparse_data_size) {
   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);
+  size_t additional_offset;
+  if (stream_index != 0)
+    additional_offset = 0;
+  else
+    additional_offset = sizeof(net::SHA256HashValue);
+  return additional_offset +
+         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);
+  if (stream_index == 1)
+    return GetEOFOffsetInFile(key_length, 0);
+  return GetEOFOffsetInFile(key_length, stream_index);
 }
 
-int64_t SimpleEntryStat::GetFileSize(const std::string& key,
-                                     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);
+int64_t SimpleEntryStat::GetFileSize(size_t key_length, int file_index) const {
+  int32_t total_data_size;

[Randy Smith (Not in Mondays), 2016/05/19 19:38:50]

(Suggestion for a followup CL; this problem predates this CL)

I find the use of the "data size" concept in this function confusing;
GetFileSizeFromDataSize() assumes that there's only a single stream in a file,
and that's compensated for here by including in total_data_size the extra EOF
record (and hash value, which is new) that's implied by the two stream file
case.  In my ideal world the variables and would be named in some way that makes
that distinction clearer, possibly with a re-jigger of responsibility between
functions.  I'm not going to make specific suggestions since I don't think the
fix belongs in this CL, but I wanted to point out another place where I did more
tripping over figuring out what was going on than I'd ideally like to have in
Chromium code.

[gavinp, 2016/05/20 01:42:32]

I agree. this has been a problem for a long time.

+  if (file_index == 0) {
+    total_data_size = data_size_[0] + data_size_[1] +
+                      sizeof(net::SHA256HashValue) + sizeof(SimpleFileEOF);
+  } else {
+    total_data_size = data_size_[2];
+  }
+  return GetFileSizeFromDataSize(key_length, total_data_size);
 }
 
 SimpleEntryCreationResults::SimpleEntryCreationResults(
     SimpleEntryStat entry_stat)
     : sync_entry(NULL),
       entry_stat(entry_stat),
       stream_0_crc32(crc32(0, Z_NULL, 0)),
       result(net::OK) {
 }
 
@@ skipping 30 matching lines @@
 
 SimpleSynchronousEntry::EntryOperationData::EntryOperationData(
     int64_t sparse_offset_p,
     int buf_len_p)
     : sparse_offset(sparse_offset_p), buf_len(buf_len_p) {}
 
 // static
 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;
     out_results->sync_entry = NULL;
     out_results->stream_0_data = NULL;
     return;
   }
   UMA_HISTOGRAM_TIMES("SimpleCache.DiskOpenLatency", open_time.Elapsed());
   out_results->sync_entry = sync_entry;
 }
 
 // static
 void SimpleSynchronousEntry::CreateEntry(
     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) {
   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();
     delete sync_entry;
     out_results->sync_entry = NULL;
     return;
   }
   out_results->sync_entry = sync_entry;
 }
 
@@ skipping 20 matching lines @@
       [&path](const uint64_t& key_hash) {
         return SimpleSynchronousEntry::DeleteFilesForEntryHash(path, key_hash);
       });
   return (did_delete_count == key_hashes->size()) ? net::OK : net::ERR_FAILED;
 }
 
 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_[file_index] &&
+      !CheckHeaderAndKey(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),
                        reinterpret_cast<const Bytef*>(out_buf->data()),
                        bytes_read);
   }
   if (bytes_read >= 0) {
     *out_result = bytes_read;
   } else {
     *out_result = net::ERR_CACHE_READ_FAILURE;
     Doom();
   }
 }
 
 void SimpleSynchronousEntry::WriteData(const EntryOperationData& in_entry_op,
                                        net::IOBuffer* in_buf,
                                        SimpleEntryStat* out_entry_stat,
                                        int* out_result) {
   DCHECK(initialized_);
   DCHECK_NE(0, in_entry_op.index);
   int index = in_entry_op.index;
   int file_index = GetFileIndexFromStreamIndex(index);
+  if (header_and_key_check_needed_[file_index] &&
+      !empty_file_omitted_[file_index] && !CheckHeaderAndKey(file_index)) {
+    *out_result = net::ERR_FAILED;
+    Doom();
+    return;
+  }
   int offset = in_entry_op.offset;
   int buf_len = in_entry_op.buf_len;
   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]) {
     // Don't create a new file if the entry has been doomed, to avoid it being
     // mixed up with a newly-created entry with the same key.
     if (doomed) {
       DLOG(WARNING) << "Rejecting write to lazily omitted stream "
                     << in_entry_op.index << " of doomed cache entry.";
       RecordWriteResult(cache_type_, WRITE_RESULT_LAZY_STREAM_ENTRY_DOOMED);
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
@@ skipping 12 matching lines @@
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
   }
   DCHECK(!empty_file_omitted_[file_index]);
 
   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();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
   }
   if (buf_len > 0) {
     if (files_[file_index].Write(file_offset, in_buf->data(), buf_len) !=
         buf_len) {
       RecordWriteResult(cache_type_, WRITE_RESULT_WRITE_FAILURE);
       Doom();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
   }
   if (!truncate && (buf_len > 0 || !extending_by_write)) {
     out_entry_stat->set_data_size(
         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();
       *out_result = net::ERR_CACHE_WRITE_FAILURE;
       return;
     }
   }
 
   RecordWriteResult(cache_type_, WRITE_RESULT_SUCCESS);
   base::Time modification_time = Time::Now();
@@ skipping 207 matching lines @@
   *out_result = static_cast<int>(std::min(avail_so_far, len_from_start));
 }
 
 void SimpleSynchronousEntry::CheckEOFRecord(int index,
                                             const SimpleEntryStat& entry_stat,
                                             uint32_t expected_crc32,
                                             int* out_result) const {
   DCHECK(initialized_);
   uint32_t crc32;
   bool has_crc32;
+  bool has_key_sha256;
   int stream_size;
-  *out_result =
-      GetEOFRecordData(index, entry_stat, &has_crc32, &crc32, &stream_size);
+  *out_result = GetEOFRecordData(index, entry_stat, &has_crc32, &has_key_sha256,
+                                 &crc32, &stream_size);
   if (*out_result != net::OK) {
     Doom();
     return;
   }
   if (has_crc32 && crc32 != expected_crc32) {
     DVLOG(1) << "EOF record had bad crc.";
     *out_result = net::ERR_CACHE_CHECKSUM_MISMATCH;
     RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH);
     Doom();
     return;
   }
   RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_SUCCESS);
 }
 
 void SimpleSynchronousEntry::Close(
     const SimpleEntryStat& entry_stat,
     std::unique_ptr<std::vector<CRCRecord>> crc32s_to_write,
     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)) {
     RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
     DVLOG(1) << "Could not write stream 0 data.";
     Doom();
   }
+  net::SHA256HashValue hash_value;
+  CalculateSHA256OfKey(key_, &hash_value);
+  if (files_[0].Write(stream_0_offset + entry_stat.data_size(0),
+                      reinterpret_cast<char*>(hash_value.data),
+                      sizeof(hash_value)) != sizeof(hash_value)) {
+    RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
+    DVLOG(1) << "Could not write stream 0 data.";
+    Doom();
+  }
 
   for (std::vector<CRCRecord>::const_iterator it = crc32s_to_write->begin();
        it != crc32s_to_write->end(); ++it) {
     const int stream_index = it->index;
     const int file_index = GetFileIndexFromStreamIndex(stream_index);
     if (empty_file_omitted_[file_index])
       continue;
 
     SimpleFileEOF eof_record;
     eof_record.stream_size = entry_stat.data_size(stream_index);
     eof_record.final_magic_number = kSimpleFinalMagicNumber;
     eof_record.flags = 0;
     if (it->has_crc32)
       eof_record.flags |= SimpleFileEOF::FLAG_HAS_CRC32;
+    if (stream_index == 0)
+      eof_record.flags |= SimpleFileEOF::FLAG_HAS_KEY_SHA256;
     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().
     if (stream_index == 0 &&
         !files_[file_index].SetLength(eof_offset)) {
       RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
       DVLOG(1) << "Could not truncate stream 0 file.";
       Doom();
       break;
     }
     if (files_[file_index].Write(eof_offset,
                                  reinterpret_cast<const char*>(&eof_record),
                                  sizeof(eof_record)) !=
         sizeof(eof_record)) {
       RecordCloseResult(cache_type_, CLOSE_RESULT_WRITE_FAILURE);
       DVLOG(1) << "Could not write eof record.";
       Doom();
       break;
     }
   }
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     if (empty_file_omitted_[i])
       continue;
 
+    if (header_and_key_check_needed_[i] && !CheckHeaderAndKey(i)) {
+      Doom();
+    }
     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);
     const int64_t cluster_loss = file_size % 4096 ? 4096 - file_size % 4096 : 0;
     SIMPLE_CACHE_UMA(PERCENTAGE,
                      "LastClusterLossPercent", cache_type_,
                      static_cast<base::HistogramBase::Sample>(
                          cluster_loss * 100 / (cluster_loss + file_size)));
   }
 
   if (sparse_file_open())
     sparse_file_.Close();
 
   if (files_created_) {
     const int stream2_file_index = GetFileIndexFromStreamIndex(2);
     SIMPLE_CACHE_UMA(BOOLEAN, "EntryCreatedAndStream2Omitted", cache_type_,
                      empty_file_omitted_[stream2_file_index]);
   }
   RecordCloseResult(cache_type_, CLOSE_RESULT_SUCCESS);
   have_open_files_ = false;
   delete this;
 }
 
 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) {
   for (int i = 0; i < kSimpleEntryFileCount; ++i)
     empty_file_omitted_[i] = false;
 }
 
 SimpleSynchronousEntry::~SimpleSynchronousEntry() {
   DCHECK(!(have_open_files_ && initialized_));
   if (have_open_files_)
@@ skipping 35 matching lines @@
   int flags = File::FLAG_CREATE | File::FLAG_READ | File::FLAG_WRITE |
               File::FLAG_SHARE_DELETE;
   files_[file_index].Initialize(filename, flags);
   *out_error = files_[file_index].error_details();
 
   empty_file_omitted_[file_index] = false;
 
   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);
       } else {
         SIMPLE_CACHE_UMA(ENUMERATION,
                          "SyncOpenPlatformFileError_WithoutIndex",
                          cache_type_,
                          -error, -base::File::FILE_ERROR_MAX);
       }
       while (--i >= 0)
@@ skipping 46 matching lines @@
   }
   SIMPLE_CACHE_UMA(CUSTOM_COUNTS,
                    "SyncOpenEntryAge", cache_type_,
                    entry_age.InHours(), 1, 1000, 50);
 
   files_created_ = false;
 
   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);
       } else {
         SIMPLE_CACHE_UMA(ENUMERATION,
                          "SyncCreatePlatformFileError_WithoutIndex",
                          cache_type_,
                          -error, -base::File::FILE_ERROR_MAX);
       }
       while (--i >= 0)
@@ skipping 25 matching lines @@
 
   if (sparse_file_open())
     CloseSparseFile();
 }
 
 void SimpleSynchronousEntry::CloseFiles() {
   for (int i = 0; i < kSimpleEntryFileCount; ++i)
     CloseFile(i);
 }
 
+bool SimpleSynchronousEntry::CheckHeaderAndKey(int file_index) {
+  // 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. In the case where we are opening an
+  // entry without a key, the kInitialHeaderRead setting means that we are
+  // actually already reading stream 1 data here, and tossing it out.
+  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 =
+      reinterpret_cast<const SimpleFileHeader*>(header_data.data());
+
+  if (bytes_read == -1 || static_cast<size_t>(bytes_read) < sizeof(*header)) {
+    RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_HEADER, had_index_);
+    return false;
+  }
+  // This resize will not invalidate iterators since it does not enlarge the
+  // header_data.
+  DCHECK_LE(static_cast<size_t>(bytes_read), header_data.size());
+  header_data.resize(bytes_read);
+
+  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);
+  if (header_data.size() < expected_header_size) {
+    size_t old_size = header_data.size();
+    int bytes_to_read = expected_header_size - old_size;
+    // This resize will invalidate iterators, since it is enlarging header_data.
+    header_data.resize(expected_header_size);
+    int bytes_read = files_[file_index].Read(
+        old_size, header_data.data() + old_size, bytes_to_read);
+    if (bytes_read != bytes_to_read) {
+      RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_KEY, had_index_);
+      return false;
+    }
+    header = reinterpret_cast<const SimpleFileHeader*>(header_data.data());
+  }
+
+  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_[file_index] = 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;
   }
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     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 (!key_.empty()) {
+      header_and_key_check_needed_[i] = true;
+    } else {
+      if (!CheckHeaderAndKey(i))
+        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;
-    }
-
-    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));
-      int ret_value_stream_0 = ReadAndValidateStream0(
-          total_data_size, out_entry_stat, stream_0_data, out_stream_0_crc32);
+      int ret_value_stream_0 =
+          ReadAndValidateStream0(out_entry_stat->data_size(1), 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);
 
   bool removed_stream2 = false;
   const int stream2_file_index = GetFileIndexFromStreamIndex(2);
   DCHECK(CanOmitEmptyFile(stream2_file_index));
   if (!empty_file_omitted_[stream2_file_index] &&
       out_entry_stat->data_size(2) == 0) {
     DVLOG(1) << "Removing empty stream 2 file.";
     CloseFile(stream2_file_index);
     DeleteFileForEntryHash(path_, entry_hash_, stream2_file_index);
     empty_file_omitted_[stream2_file_index] = true;
     removed_stream2 = true;
   }
 
   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;
 }
 
 bool SimpleSynchronousEntry::InitializeCreatedFile(
     int file_index,
     CreateEntryResult* out_result) {
   SimpleFileHeader header;
   header.initial_magic_number = kSimpleInitialMagicNumber;
   header.version = kSimpleEntryVersionOnDisk;
@@ skipping 12 matching lines @@
                                            key_.size());
   if (bytes_written != base::checked_cast<int>(key_.size())) {
     *out_result = CREATE_ENTRY_CANT_WRITE_KEY;
     return false;
   }
 
   return true;
 }
 
 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;
   }
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     if (empty_file_omitted_[i])
       continue;
 
     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;
 }
 
 int SimpleSynchronousEntry::ReadAndValidateStream0(
-    int total_data_size,
+    int file_size,
     SimpleEntryStat* out_entry_stat,
     scoped_refptr<net::GrowableIOBuffer>* stream_0_data,
-    uint32_t* out_stream_0_crc32) const {
+    uint32_t* out_stream_0_crc32) {
   // 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.
+  int total_data_size = GetDataSizeFromFileSize(key_.size(), file_size);
   out_entry_stat->set_data_size(0, 0);
-  out_entry_stat->set_data_size(1, total_data_size - sizeof(SimpleFileEOF));
+  out_entry_stat->set_data_size(
+      1,
+      total_data_size - sizeof(net::SHA256HashValue) - sizeof(SimpleFileEOF));

[Randy Smith (Not in Mondays), 2016/05/19 19:38:50]

So I looked and didn't find any problems for this possibility, but might this
value now be negative?  I.e. if the hash isn't there, and the total data for
streams 0 and 1 is, say, 0?  I think everything works out ok if it is, but the
thought scares me, so I wanted to run it by you in case you saw something I
didn't.

[gavinp, 2016/05/20 01:42:32]

It is negative occasionally, I believe -7 at most (least?). It's clean, some
unit tests exercise this path.

 
   bool has_crc32;
+  bool has_key_sha256;
   uint32_t read_crc32;
   int stream_0_size;
-  int ret_value_crc32 = GetEOFRecordData(
-      0, *out_entry_stat, &has_crc32, &read_crc32, &stream_0_size);
+  int ret_value_crc32 =
+      GetEOFRecordData(0, *out_entry_stat, &has_crc32, &has_key_sha256,
+                       &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))
+  // Calculate and set the real values for data size.
+  int stream_1_size = out_entry_stat->data_size(1) - stream_0_size;
+  if (!has_key_sha256)
+    stream_1_size += sizeof(net::SHA256HashValue);
+  if (stream_1_size < 0)
     return net::ERR_FAILED;
-
-  // These are the real values of data size.
   out_entry_stat->set_data_size(0, stream_0_size);
-  out_entry_stat->set_data_size(
-      1, out_entry_stat->data_size(1) - stream_0_size);
+  out_entry_stat->set_data_size(1, stream_1_size);
 
   // 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);
-  File* file = const_cast<File*>(&files_[0]);
-  int bytes_read =
-      file->Read(file_offset, (*stream_0_data)->data(), stream_0_size);
-  if (bytes_read != stream_0_size)
+  (*stream_0_data)->SetCapacity(stream_0_size + sizeof(net::SHA256HashValue));
+  int file_offset = out_entry_stat->GetOffsetInFile(key_.size(), 0, 0);
+  int read_size = stream_0_size;
+  if (has_key_sha256)
+    read_size += sizeof(net::SHA256HashValue);
+  if (files_[0].Read(file_offset, (*stream_0_data)->data(), read_size) !=
+      read_size)
     return net::ERR_FAILED;
 
   // Check the CRC32.
   uint32_t 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) {
     DVLOG(1) << "EOF record had bad crc.";
     RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_CRC_MISMATCH);
     return net::ERR_FAILED;
   }
   *out_stream_0_crc32 = expected_crc32;
+
+  // If present, check the key SHA256.
+  if (has_key_sha256) {
+    net::SHA256HashValue hash_value;
+    CalculateSHA256OfKey(key_, &hash_value);
+    bool matched =
+        std::memcmp(&hash_value, (*stream_0_data)->data() + stream_0_size,
+                    sizeof(hash_value)) == 0;
+    if (!matched) {
+      RecordKeySHA256Result(cache_type_, KeySHA256Result::NO_MATCH);
+      return net::ERR_FAILED;
+    }
+    RecordKeySHA256Result(cache_type_, KeySHA256Result::MATCHED);
+  } else {
+    RecordKeySHA256Result(cache_type_, KeySHA256Result::NOT_PRESENT);
+  }
+
+  // Ensure the key is validated before completion.
+  if (!has_key_sha256 && header_and_key_check_needed_[0])
+    CheckHeaderAndKey(0);
+
   RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_SUCCESS);
   return net::OK;
 }
 
 int SimpleSynchronousEntry::GetEOFRecordData(int index,
                                              const SimpleEntryStat& entry_stat,
                                              bool* out_has_crc32,
+                                             bool* out_has_key_sha256,
                                              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),
                  sizeof(eof_record)) !=
       sizeof(eof_record)) {
     RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_READ_FAILURE);
     return net::ERR_CACHE_CHECKSUM_READ_FAILURE;
   }
 
   if (eof_record.final_magic_number != kSimpleFinalMagicNumber) {
     RecordCheckEOFResult(cache_type_, CHECK_EOF_RESULT_MAGIC_NUMBER_MISMATCH);
     DVLOG(1) << "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_has_key_sha256 =
+      (eof_record.flags & SimpleFileEOF::FLAG_HAS_KEY_SHA256) ==
+      SimpleFileEOF::FLAG_HAS_KEY_SHA256;
   *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 {
   DeleteFilesForEntryHash(path_, entry_hash_);
 }
 
@@ skipping 307 matching lines @@
   range.offset = offset;
   range.length = len;
   range.data_crc32 = data_crc32;
   range.file_offset = data_file_offset;
   sparse_ranges_.insert(std::make_pair(offset, range));
 
   return true;
 }
 
 }  // namespace disk_cache