Avoid fragmenting the heap too much while reconstructing the index.

net/disk_cache/simple/simple_index_file.cc

Index: net/disk_cache/simple/simple_index_file.cc
diff --git a/net/disk_cache/simple/simple_index_file.cc b/net/disk_cache/simple/simple_index_file.cc
index 7bcea7cdfa86255360a7416d4eff5731ca24ae7c..8a3c8d472ae50fb8691d294bd902267c353cf2b3 100644
--- a/net/disk_cache/simple/simple_index_file.cc
+++ b/net/disk_cache/simple/simple_index_file.cc
@@ -7,7 +7,6 @@
 #include <vector>
 
 #include "base/file_util.h"
-#include "base/files/file_enumerator.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/hash.h"
 #include "base/logging.h"
@@ -22,11 +21,18 @@
 #include "net/disk_cache/simple/simple_util.h"
 #include "third_party/zlib/zlib.h"
 
-
+namespace disk_cache {
 namespace {
 
+const int kEntryFilesHashLength = 16;
+const int kEntryFilesSuffixLength = 2;
+
 const uint64 kMaxEntiresInIndex = 100000000;
 
+const char kFakeIndexFileName[] = "index";
+const char kIndexFileName[] = "the-real-index";
+const char kTempIndexFileName[] = "temp-index";
+
 uint32 CalculatePickleCRC(const Pickle& pickle) {
   return crc32(crc32(0, Z_NULL, 0),
                reinterpret_cast<const Bytef*>(pickle.payload()),
@@ -67,9 +73,60 @@ void WriteToDiskInternal(const base::FilePath& index_filename,
   }
 }
 
-}  // namespace
+// Called for each cache directory traversal iteration.
+void ProcessEntryFile(SimpleIndex::EntrySet* entries,
+                      const base::FilePath& file_path) {
+  static const size_t kEntryFilesLength =
+      kEntryFilesHashLength + kEntryFilesSuffixLength;
+  const base::FilePath::StringType file_name_string =
+      file_path.BaseName().value();
+  if (file_name_string.size() != kEntryFilesLength) {
+    if (file_name_string != kFakeIndexFileName &&
+            file_name_string != kIndexFileName) {
+      LOG(ERROR) << "unexpected file " << file_name_string;

[pasko, 2013/08/21 16:03:17]

it would be easy to forget to update this check when updating the structure of
the cache directory with something new, say new directory. So expect firing this
"useful" error message a bit later.

I'd just remove it.

[Philippe, 2013/08/21 16:20:58]

Done.

+    }
+    return;
+  }
 
-namespace disk_cache {
+  const base::StringPiece hash_string(
+      file_name_string.begin(),
+      file_name_string.begin() + kEntryFilesHashLength);
+  uint64 hash_key = 0;
+  if (!simple_util::GetEntryHashKeyFromHexString(hash_string, &hash_key)) {
+    LOG(WARNING) << "Invalid entry hash key filename while restoring index from"
+                 << " disk: " << file_name_string;
+    // TODO(felipeg): Should we delete the invalid file here ?

[pasko, 2013/08/21 16:03:17]

I am absolutely sure we must *not* delete the invalid file. Please remove the
TODO.

[Philippe, 2013/08/21 16:20:58]

Done.

+    return;
+  }
+
+  base::PlatformFileInfo file_info;
+  if (!file_util::GetFileInfo(file_path, &file_info)) {
+    LOG(ERROR) << "Could not get file info for " << file_path.value();
+    return;
+  }
+  base::Time last_used_time;
+#if defined(OS_POSIX)
+  // For POSIX systems, a last access time is available. However, it's not
+  // guaranteed to be more accurate than mtime. It is no worse though.
+  last_used_time = file_info.last_accessed;
+#endif
+  if (last_used_time.is_null())
+    last_used_time = file_info.last_modified;
+
+  int64 file_size = file_info.size;
+  SimpleIndex::EntrySet::iterator it = entries->find(hash_key);
+  if (it == entries->end()) {
+    SimpleIndex::InsertInEntrySet(
+        hash_key,
+        EntryMetadata(last_used_time, file_size),
+        entries);
+  } else {
+    // Summing up the total size of the entry through all the *_[0-2] files
+    it->second.SetEntrySize(it->second.GetEntrySize() + file_size);
+  }
+}
+
+}  // namespace
 
 SimpleIndexLoadResult::SimpleIndexLoadResult() : did_load(false),
                                                  flush_required(false) {
@@ -84,11 +141,6 @@ void SimpleIndexLoadResult::Reset() {
   entries.clear();
 }
 
-// static
-const char SimpleIndexFile::kIndexFileName[] = "the-real-index";
-// static
-const char SimpleIndexFile::kTempIndexFileName[] = "temp-index";
-
 SimpleIndexFile::IndexMetadata::IndexMetadata() :
     magic_number_(kSimpleIndexMagicNumber),
     version_(kSimpleVersion),
@@ -350,7 +402,6 @@ void SimpleIndexFile::SyncRestoreFromDisk(
     const base::FilePath& index_file_path,
     SimpleIndexLoadResult* out_result) {
   LOG(INFO) << "Simple Cache Index is being restored from disk.";
-
   base::DeleteFile(index_file_path, /* recursive = */ false);
   out_result->Reset();
   SimpleIndex::EntrySet* entries = &out_result->entries;
@@ -359,53 +410,13 @@ void SimpleIndexFile::SyncRestoreFromDisk(
   COMPILE_ASSERT(kSimpleEntryFileCount == 3,
                  file_pattern_must_match_file_count);
 
-  const int kFileSuffixLength = sizeof("_0") - 1;
-  const base::FilePath::StringType file_pattern = FILE_PATH_LITERAL("*_[0-2]");
-  base::FileEnumerator enumerator(cache_directory,
-                                  false /* recursive */,
-                                  base::FileEnumerator::FILES,
-                                  file_pattern);
-  for (base::FilePath file_path = enumerator.Next(); !file_path.empty();
-       file_path = enumerator.Next()) {
-    const base::FilePath::StringType base_name = file_path.BaseName().value();
-    // Converting to std::string is OK since we never use UTF8 wide chars in our
-    // file names.
-    const std::string hash_key_string(base_name.begin(),
-                                      base_name.end() - kFileSuffixLength);
-    uint64 hash_key = 0;
-    if (!simple_util::GetEntryHashKeyFromHexString(
-            hash_key_string, &hash_key)) {
-      LOG(WARNING) << "Invalid Entry Hash Key filename while restoring "
-                   << "Simple Index from disk: " << base_name;
-      // TODO(felipeg): Should we delete the invalid file here ?
-      continue;
-    }
-
-    base::FileEnumerator::FileInfo info = enumerator.GetInfo();
-    base::Time last_used_time;
-#if defined(OS_POSIX)
-    // For POSIX systems, a last access time is available. However, it's not
-    // guaranteed to be more accurate than mtime. It is no worse though.
-    last_used_time = base::Time::FromTimeT(info.stat().st_atime);
-#endif
-    if (last_used_time.is_null())
-      last_used_time = info.GetLastModifiedTime();
-
-    int64 file_size = info.GetSize();
-    SimpleIndex::EntrySet::iterator it = entries->find(hash_key);
-    if (it == entries->end()) {
-      SimpleIndex::InsertInEntrySet(
-          hash_key,
-          EntryMetadata(last_used_time, file_size),
-          entries);
-    } else {
-      // Summing up the total size of the entry through all the *_[0-2] files
-      it->second.SetEntrySize(it->second.GetEntrySize() + file_size);
-    }
+  const bool did_succeed = TraverseCacheDirectory(
+      cache_directory, base::Bind(&ProcessEntryFile, entries));
+  if (!did_succeed) {
+    LOG(ERROR) << "Could not reconstruct index from disk";
+    return;
   }
-
   out_result->did_load = true;
-
   // When we restore from disk we write the merged index file to disk right
   // away, this might save us from having to restore again next time.
   out_result->flush_required = true;