Revert "Completely redo how themes are stored on disk and processed at install

time," as it fails valgrind tests.

This reverts commit 86faccd1028937a69ccc718718fd48c06c0cd471 (r34379).

Review URL: http://codereview.chromium.org/490025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34385 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 28 insertions, 135 deletions

diff --git a/base/base.gyp b/base/base.gyp
index 28cf048..e0a0fe9 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -185,6 +185,7 @@
             '../third_party/icu/icu.gyp:icudata',
           ],
           'sources!': [
+            'data_pack_unittest.cc',
             'file_descriptor_shuffle_unittest.cc',
           ],
         }, {  # OS != "win"
diff --git a/base/base.gypi b/base/base.gypi
index 77a919e..c19b343 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -331,6 +331,7 @@
                 '../chrome/third_party/wtl/include',
               ],
               'sources!': [
+                'data_pack.cc',
                 'event_recorder_stubs.cc',
                 'file_descriptor_shuffle.cc',
                 'message_pump_libevent.cc',
diff --git a/base/data_pack.cc b/base/data_pack.cc
index 1078d0a..27054d3 100644
--- a/base/data_pack.cc
+++ b/base/data_pack.cc
@@ -14,22 +14,17 @@
 // http://dev.chromium.org/developers/design-documents/linuxresourcesandlocalizedstrings
 
 namespace {
-
-// A word is four bytes.
-static const size_t kWord = 4;
-
-static const uint32 kFileFormatVersion = 1;
+static const uint32_t kFileFormatVersion = 1;
 // Length of file header: version and entry count.
-static const size_t kHeaderLength = 2 * sizeof(uint32);
+static const size_t kHeaderLength = 2 * sizeof(uint32_t);
 
-#pragma pack(push,1)
 struct DataPackEntry {
-  uint32 resource_id;
-  uint32 file_offset;
-  uint32 length;
+  uint32_t resource_id;
+  uint32_t file_offset;
+  uint32_t length;
 
   static int CompareById(const void* void_key, const void* void_entry) {
-    uint32 key = *reinterpret_cast<const uint32*>(void_key);
+    uint32_t key = *reinterpret_cast<const uint32_t*>(void_key);
     const DataPackEntry* entry =
         reinterpret_cast<const DataPackEntry*>(void_entry);
     if (key < entry->resource_id) {
@@ -40,10 +35,7 @@ struct DataPackEntry {
       return 0;
     }
   }
-};
-#pragma pack(pop)
-
-COMPILE_ASSERT(sizeof(DataPackEntry) == 12, size_of_header_must_be_twelve);
+}  __attribute((packed));
 
 }  // anonymous namespace
 
@@ -58,13 +50,12 @@ DataPack::~DataPack() {
 bool DataPack::Load(const FilePath& path) {
   mmap_.reset(new file_util::MemoryMappedFile);
   if (!mmap_->Initialize(path)) {
-    DLOG(ERROR) << "Failed to mmap datapack";
-    return false;
+    PCHECK(false) << "Failed to mmap " << path.value();
   }
 
   // Parse the header of the file.
-  // First uint32: version; second: resource count.
-  const uint32* ptr = reinterpret_cast<const uint32*>(mmap_->data());
+  // First uint32_t: version; second: resource count.
+  const uint32* ptr = reinterpret_cast<const uint32_t*>(mmap_->data());
   uint32 version = ptr[0];
   if (version != kFileFormatVersion) {
     LOG(ERROR) << "Bad data pack version: got " << version << ", expected "
@@ -98,7 +89,7 @@ bool DataPack::Load(const FilePath& path) {
   return true;
 }
 
-bool DataPack::GetStringPiece(uint32 resource_id, StringPiece* data) {
+bool DataPack::GetStringPiece(uint32_t resource_id, StringPiece* data) {
   // It won't be hard to make this endian-agnostic, but it's not worth
   // bothering to do right now.
 #if defined(__BYTE_ORDER)
@@ -114,6 +105,7 @@ bool DataPack::GetStringPiece(uint32 resource_id, StringPiece* data) {
       bsearch(&resource_id, mmap_->data() + kHeaderLength, resource_count_,
               sizeof(DataPackEntry), DataPackEntry::CompareById));
   if (!target) {
+    LOG(ERROR) << "No resource found with id: " << resource_id;
     return false;
   }
 
@@ -121,7 +113,7 @@ bool DataPack::GetStringPiece(uint32 resource_id, StringPiece* data) {
   return true;
 }
 
-RefCountedStaticMemory* DataPack::GetStaticMemory(uint32 resource_id) {
+RefCountedStaticMemory* DataPack::GetStaticMemory(uint32_t resource_id) {
   base::StringPiece piece;
   if (!GetStringPiece(resource_id, &piece))
     return NULL;
@@ -130,67 +122,4 @@ RefCountedStaticMemory* DataPack::GetStaticMemory(uint32 resource_id) {
       reinterpret_cast<const unsigned char*>(piece.data()), piece.length());
 }
 
-// static
-bool DataPack::WritePack(const FilePath& path,
-                         const std::map<uint32, StringPiece>& resources) {
-  FILE* file = file_util::OpenFile(path, "wb");
-  if (!file)
-    return false;
-
-  if (fwrite(&kFileFormatVersion, 1, kWord, file) != kWord) {
-    LOG(ERROR) << "Failed to write file version";
-    file_util::CloseFile(file);
-    return false;
-  }
-
-  // Note: the python version of this function explicitly sorted keys, but
-  // std::map is a sorted associative container, we shouldn't have to do that.
-  uint32 entry_count = resources.size();
-  if (fwrite(&entry_count, 1, kWord, file) != kWord) {
-    LOG(ERROR) << "Failed to write entry count";
-    file_util::CloseFile(file);
-    return false;
-  }
-
-  // Each entry is 3 uint32s.
-  uint32 index_length = entry_count * 3 * kWord;
-  uint32 data_offset = kHeaderLength + index_length;
-  for (std::map<uint32, StringPiece>::const_iterator it = resources.begin();
-       it != resources.end(); ++it) {
-    if (fwrite(&it->first, 1, kWord, file) != kWord) {
-      LOG(ERROR) << "Failed to write id for " << it->first;
-      file_util::CloseFile(file);
-      return false;
-    }
-
-    if (fwrite(&data_offset, 1, kWord, file) != kWord) {
-      LOG(ERROR) << "Failed to write offset for " << it->first;
-      file_util::CloseFile(file);
-      return false;
-    }
-
-    uint32 len = it->second.length();
-    if (fwrite(&len, 1, kWord, file) != kWord) {
-      LOG(ERROR) << "Failed to write length for " << it->first;
-      file_util::CloseFile(file);
-      return false;
-    }
-
-    data_offset += len;
-  }
-
-  for (std::map<uint32, StringPiece>::const_iterator it = resources.begin();
-       it != resources.end(); ++it) {
-    if (fwrite(it->second.data(), it->second.length(), 1, file) != 1) {
-      LOG(ERROR) << "Failed to write data for " << it->first;
-      file_util::CloseFile(file);
-      return false;
-    }
-  }
-
-  file_util::CloseFile(file);
-
-  return true;
-}
-
 }  // namespace base
diff --git a/base/data_pack.h b/base/data_pack.h
index 8938491..7c9abde 100644
--- a/base/data_pack.h
+++ b/base/data_pack.h
@@ -9,8 +9,6 @@
 #ifndef BASE_DATA_PACK_H_
 #define BASE_DATA_PACK_H_
 
-#include <map>
-
 #include "base/basictypes.h"
 #include "base/ref_counted_memory.h"
 #include "base/scoped_ptr.h"
@@ -35,16 +33,12 @@ class DataPack {
   // Get resource by id |resource_id|, filling in |data|.
   // The data is owned by the DataPack object and should not be modified.
   // Returns false if the resource id isn't found.
-  bool GetStringPiece(uint32 resource_id, StringPiece* data);
+  bool GetStringPiece(uint32_t resource_id, StringPiece* data);
 
   // Like GetStringPiece(), but returns a reference to memory. This interface
   // is used for image data, while the StringPiece interface is usually used
   // for localization strings.
-  RefCountedStaticMemory* GetStaticMemory(uint32 resource_id);
-
-  // Writes a pack file containing |resources| to |path|.
-  static bool WritePack(const FilePath& path,
-                        const std::map<uint32, StringPiece>& resources);
+  RefCountedStaticMemory* GetStaticMemory(uint32_t resource_id);
 
  private:
   // The memory-mapped data.
diff --git a/base/data_pack_unittest.cc b/base/data_pack_unittest.cc
index d089b28..a62acf0 100644
--- a/base/data_pack_unittest.cc
+++ b/base/data_pack_unittest.cc
@@ -5,20 +5,24 @@
 #include "base/data_pack.h"
 
 #include "base/file_path.h"
-#include "base/file_util.h"
 #include "base/path_service.h"
-#include "base/scoped_temp_dir.h"
 #include "base/string_piece.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(DataPackTest, Load) {
-  FilePath data_path;
-  PathService::Get(base::DIR_SOURCE_ROOT, &data_path);
-  data_path = data_path.Append(
+class DataPackTest : public testing::Test {
+ public:
+  DataPackTest() {
+    PathService::Get(base::DIR_SOURCE_ROOT, &data_path_);
+    data_path_ = data_path_.Append(
         FILE_PATH_LITERAL("base/data/data_pack_unittest/sample.pak"));
+  }
 
+  FilePath data_path_;
+};
+
+TEST_F(DataPackTest, Load) {
   base::DataPack pack;
-  ASSERT_TRUE(pack.Load(data_path));
+  ASSERT_TRUE(pack.Load(data_path_));
 
   base::StringPiece data;
   ASSERT_TRUE(pack.GetStringPiece(4, &data));
@@ -35,39 +39,3 @@ TEST(DataPackTest, Load) {
   // Try looking up an invalid key.
   ASSERT_FALSE(pack.GetStringPiece(140, &data));
 }
-
-TEST(DataPackTest, Write) {
-  ScopedTempDir dir;
-  ASSERT_TRUE(dir.CreateUniqueTempDir());
-  FilePath file = dir.path().Append(FILE_PATH_LITERAL("data.pak"));
-
-  std::string one("one");
-  std::string two("two");
-  std::string three("three");
-  std::string four("four");
-  std::string fifteen("fifteen");
-
-  std::map<uint32, base::StringPiece> resources;
-  resources.insert(std::make_pair(1, base::StringPiece(one)));
-  resources.insert(std::make_pair(2, base::StringPiece(two)));
-  resources.insert(std::make_pair(15, base::StringPiece(fifteen)));
-  resources.insert(std::make_pair(3, base::StringPiece(three)));
-  resources.insert(std::make_pair(4, base::StringPiece(four)));
-  ASSERT_TRUE(base::DataPack::WritePack(file, resources));
-
-  // Now try to read the data back in.
-  base::DataPack pack;
-  ASSERT_TRUE(pack.Load(file));
-
-  base::StringPiece data;
-  ASSERT_TRUE(pack.GetStringPiece(1, &data));
-  EXPECT_EQ(one, data);
-  ASSERT_TRUE(pack.GetStringPiece(2, &data));
-  EXPECT_EQ(two, data);
-  ASSERT_TRUE(pack.GetStringPiece(3, &data));
-  EXPECT_EQ(three, data);
-  ASSERT_TRUE(pack.GetStringPiece(4, &data));
-  EXPECT_EQ(four, data);
-  ASSERT_TRUE(pack.GetStringPiece(15, &data));
-  EXPECT_EQ(fifteen, data);
-}