From 7c7573040ade65f70560bfdb508cdb29d41b1254 Mon Sep 17 00:00:00 2001
From: Mohamed Heikal <mheikal@google.com>
Date: Thu, 25 Apr 2019 17:39:43 -0400
Subject: [PATCH] Fix bug where path shortening breaks ColorStateLists

Android resource loader uses the file path to check if a resource is a
ColorStateList. Path shortening removed that part of the path and thus
broke the resource loader of APKs optimized with path shortening. This
cl skips path shortening for resources under "res/color/"

Test: make aapt2_tests
Bug: b/75965637

Change-Id: If94dfa398efd81522d4faed157afd35f6dabe856
---
 .../aapt2/optimize/ResourcePathShortener.cpp  |  4 ++
 .../optimize/ResourcePathShortener_test.cpp   | 46 +++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/tools/aapt2/optimize/ResourcePathShortener.cpp b/tools/aapt2/optimize/ResourcePathShortener.cpp
index c5df3dd00db96..845262bcb0d96 100644
--- a/tools/aapt2/optimize/ResourcePathShortener.cpp
+++ b/tools/aapt2/optimize/ResourcePathShortener.cpp
@@ -95,6 +95,10 @@ bool ResourcePathShortener::Consume(IAaptContext* context, ResourceTable* table)
     android::StringPiece res_subdir, actual_filename, extension;
     util::ExtractResFilePathParts(*file_ref->path, &res_subdir, &actual_filename, &extension);
 
+    // Android detects ColorStateLists via pathname, skip res/color/*
+    if (res_subdir == android::StringPiece("res/color/"))
+      continue;
+
     std::string shortened_filename = ShortenFileName(*file_ref->path, num_chars);
     int collision_count = 0;
     std::string shortened_path = GetShortenedPath(shortened_filename, extension, collision_count);
diff --git a/tools/aapt2/optimize/ResourcePathShortener_test.cpp b/tools/aapt2/optimize/ResourcePathShortener_test.cpp
index 88cadc76c336e..efbef8f407221 100644
--- a/tools/aapt2/optimize/ResourcePathShortener_test.cpp
+++ b/tools/aapt2/optimize/ResourcePathShortener_test.cpp
@@ -24,6 +24,11 @@ using ::testing::Not;
 using ::testing::NotNull;
 using ::testing::Eq;
 
+android::StringPiece GetExtension(android::StringPiece path) {
+  auto iter = std::find(path.begin(), path.end(), '.');
+  return android::StringPiece(iter, path.end() - iter);
+}
+
 namespace aapt {
 
 TEST(ResourcePathShortenerTest, FileRefPathsChangedInResourceTable) {
@@ -64,4 +69,45 @@ TEST(ResourcePathShortenerTest, FileRefPathsChangedInResourceTable) {
   EXPECT_THAT(path_map.find("res/should/still/be/the/same.png"), Eq(path_map.end()));
 }
 
+TEST(ResourcePathShortenerTest, SkipColorFileRefPaths) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+
+  std::unique_ptr<ResourceTable> table =
+      test::ResourceTableBuilder()
+          .AddFileReference("android:color/colorlist", "res/color/colorlist.xml")
+          .Build();
+
+  std::map<std::string, std::string> path_map;
+  ASSERT_TRUE(ResourcePathShortener(path_map).Consume(context.get(), table.get()));
+
+  // Expect that the path map to not contain the ColorStateList
+  ASSERT_THAT(path_map.find("res/color/colorlist.xml"), Eq(path_map.end()));
+}
+
+TEST(ResourcePathShortenerTest, KeepExtensions) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+
+  std::string original_xml_path = "res/drawable/xmlfile.xml";
+  std::string original_png_path = "res/drawable/pngfile.png";
+
+  std::unique_ptr<ResourceTable> table =
+      test::ResourceTableBuilder()
+          .AddFileReference("android:color/xmlfile", original_xml_path)
+          .AddFileReference("android:color/pngfile", original_png_path)
+          .Build();
+
+  std::map<std::string, std::string> path_map;
+  ASSERT_TRUE(ResourcePathShortener(path_map).Consume(context.get(), table.get()));
+
+  // Expect that the path map is populated
+  ASSERT_THAT(path_map.find("res/drawable/xmlfile.xml"), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find("res/drawable/pngfile.png"), Not(Eq(path_map.end())));
+
+  auto shortend_xml_path = path_map[original_xml_path];
+  auto shortend_png_path = path_map[original_png_path];
+
+  EXPECT_THAT(GetExtension(path_map[original_xml_path]), Eq(android::StringPiece(".xml")));
+  EXPECT_THAT(GetExtension(path_map[original_png_path]), Eq(android::StringPiece(".png")));
+}
+
 }   // namespace aapt