Fix C++ data pack writer.

Most data pak files are generated at build time using a
Python script (see tools/grit/grit/format/data_pak.py),
but the C++ data pack writer can also be used at runtime
by the ThemeService (see ThemeService::SetTheme()).

The Python pak writer was recently changed to support
version 5 of the format that introduces a resource
alias table to reduce the size of the final pak files,
in the case of resource data duplication. This is mostly
important for localized strings paks.

While the C++ data pack reader and writer were updated
to support this new format, the WritePack() method didn't
try to find resource duplicates and build an appropriate
alias table.

This CL fixes the C++ writer, to detect resource data
duplication and build an appropriate alias table when
it is detected. After this patch, both the Python and
C++ code should behave identically.

+ Fix slightly misleading comment documenting the header
  formats (using uint instead of int values in the
  description, the code is already correct).

+ Use ScopedFileWriter convenience class to simplify
  error handling and cleanup.

+ Use for '(const auto& item : collection)' to simplify
  scanning over maps.

+ Add a sanity check to fail if there are more than 2^16
  resources to write (since the format only supports up
  to 65536 resources per pak file).

BUG=None
R=agrieve@chromium.org,asvitkine@chromium.org,sadrul@chromium.org

Change-Id: Ic04eb68558026ff48835ed0e9e44ba539b3a52a8
Reviewed-on: https://chromium-review.googlesource.com/737636Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516316}

ui/base/resource/data_pack.h

@@ -74,6 +74,13 @@ class UI_DATA_PACK_EXPORT DataPack : public ResourceHandle {
       const std::vector<std::unique_ptr<ResourceHandle>>& packs);
 #endif
 
+  // Return the size of the resource and alias tables. Should only be used for
+  // unit-testing (more specifically checking that alias table generation
+  // removes entries for the resources table), as this is an implementation
+  // detail.
+  size_t GetResourceTableSizeForTesting() const { return resource_count_; }
+  size_t GetAliasTableSizeForTesting() const { return alias_count_; }
+
  private:
   struct Entry;
   struct Alias;

ui/base/resource/data_pack_unittest.cc

@@ -239,6 +239,64 @@ TEST_P(DataPackTest, Write) {
   EXPECT_EQ(four, data);
   ASSERT_TRUE(pack.GetStringPiece(15, &data));
   EXPECT_EQ(fifteen, data);
+
+  EXPECT_EQ(5U, pack.GetResourceTableSizeForTesting());
+  EXPECT_EQ(0U, pack.GetAliasTableSizeForTesting());
+}
+
+TEST_P(DataPackTest, WriteWithAliases) {
+  base::ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  base::FilePath file = dir.GetPath().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<uint16_t, 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)));
+  resources.insert(std::make_pair(10, base::StringPiece(one)));
+  resources.insert(std::make_pair(11, base::StringPiece(three)));
+  ASSERT_TRUE(DataPack::WritePack(file, resources, GetParam()));
+
+  // Now try to read the data back in.
+  DataPack pack(SCALE_FACTOR_100P);
+  ASSERT_TRUE(pack.LoadFromPath(file));
+  EXPECT_EQ(pack.GetTextEncodingType(), GetParam());
+
+  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);
+  ASSERT_TRUE(pack.GetStringPiece(10, &data));
+  EXPECT_EQ(one, data);
+  ASSERT_TRUE(pack.GetStringPiece(11, &data));
+  EXPECT_EQ(three, data);
+
+  base::StringPiece data2;
+  ASSERT_TRUE(pack.GetStringPiece(1, &data));
+  ASSERT_TRUE(pack.GetStringPiece(10, &data2));
+  EXPECT_EQ(data.data(), data2.data());
+
+  ASSERT_TRUE(pack.GetStringPiece(3, &data));
+  ASSERT_TRUE(pack.GetStringPiece(11, &data2));
+  EXPECT_EQ(data.data(), data2.data());
+
+  EXPECT_EQ(5U, pack.GetResourceTableSizeForTesting());
+  EXPECT_EQ(2U, pack.GetAliasTableSizeForTesting());
 }
 
 #if defined(OS_POSIX)