Refactor our SimpleIndex file format and serialization to use Pickle instead of the previously bugg…

net/disk_cache/simple/simple_index_file.h

+// 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.
+
+#ifndef NET_DISK_CACHE_SIMPLE_SIMPLE_INDEX_FILE_H_
+#define NET_DISK_CACHE_SIMPLE_SIMPLE_INDEX_FILE_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/hash_tables.h"
+#include "base/logging.h"

[Philippe, 2013/04/15 13:57:57]

ref_counted.h is missing.

[felipeg, 2013/04/15 14:39:07]

Done.

+#include "base/pickle.h"
+#include "net/disk_cache/simple/simple_index.h"
+
+namespace base {
+class TaskRunner;
+}
+
+namespace disk_cache {
+
+const uint64 kSimpleIndexInitialMagicNumber = GG_UINT64_C(0x656e74657220796f);
+
+// Simple Index File format is a pickle serialized data of IndexMetadata and

[pasko-google - do not use, 2013/04/15 14:23:55]

makes sense to expand this a little:
The file format is as follows: one instance of serialized |IndexMetadata|
followed serialized |EntryMetadata| entries repeated |number_of_entries| amount
of times.

Also, I would prefer the name IndexFileHeader, less confusing than "metadata"

[felipeg, 2013/04/15 15:37:04]

Done.

+// EntryMetadata objects. To know more about the pickle format, see

[pasko-google - do not use, 2013/04/15 14:23:55]

there is no need to reference what a pickled format is, just take the word
"pickle" in quotes on line 26 above

[felipeg, 2013/04/15 15:37:04]

Done.

+// SimpleIndexFile::Serialize() and SeeSimpleIndexFile::LoadFromDisk() methods.
+class SimpleIndexFile {
+ public:
+  class IndexMetadata {
+   public:
+    IndexMetadata();
+    IndexMetadata(uint64 number_of_entries, uint64 cache_size);
+
+    void Serialize(Pickle* pickle) const;
+    static bool DeSerialize(PickleIterator* it,
+                            IndexMetadata* out);

[Philippe, 2013/04/15 13:57:57]

Nit: I believe this fits on the line above.

[felipeg, 2013/04/15 14:39:07]

Done.

+
+    bool CheckIndexMetadata();
+
+    uint64 GetNumberOfEntries() { return number_of_entries_; }
+
+   private:
+    uint64 initial_magic_number_;
+    uint32 version_;

[Philippe, 2013/04/15 13:57:57]

FWIW, I believe you might be able to save 32 bits by making this field last. I
generally have struct fields ordered by descending size to avoid unnecessary
padding bytes. 

[felipeg, 2013/04/15 14:39:07]

early optimization

+    uint64 number_of_entries_;
+    uint64 cache_size_;  // Total cache storage size in bytes.
+  };
+
+  static void LoadFromDisk(
+      const base::FilePath& index_filename,
+      const scoped_refptr<base::TaskRunner>& callback_runner,
+      const IndexCompletionCallback& completion_callback);
+
+  // Enumerates all entries' files on disk and regenerates the index.
+  static void RestoreFromDisk(

[pasko-google - do not use, 2013/04/15 14:23:55]

why is this in SimpleIndexFile? Logically it belongs more to the SimpleIndex.

[felipeg, 2013/04/15 15:37:04]

Done.

+      const base::FilePath& index_filename,
+      const scoped_refptr<base::TaskRunner>& callback_runner,
+      const IndexCompletionCallback& completion_callback);
+
+  // Returns a scoped_ptr for a newly allocated Pickle containing the serialized
+  // data to be written to a file.
+  static scoped_ptr<Pickle> Serialize(
+      const SimpleIndexFile::IndexMetadata& index_metadata,
+      const EntrySet& entries);
+
+  // Write the serialized data from |pickle| into the index file.
+  static void WriteToDisk(const base::FilePath& index_filename,
+                          scoped_ptr<Pickle> pickle);
+
+ private:
+  struct PickleHeader : public Pickle::Header {
+    uint32 crc;
+  };
+
+  static uint32 CalculatePickleCRC(const Pickle& pickle);
+
+  SimpleIndexFile() {}
+  ~SimpleIndexFile() {}
+};
+
+
+}  // namespace disk_cache
+
+#endif  // NET_DISK_CACHE_SIMPLE_SIMPLE_INDEX_FILE_H_