commit 8d4ee973fa631c13ad458a3b906d8c1319fe0913
author Ryan Mitchell <rtmitchell@google.com> Mon Aug 27 11:24:04 2018 -0700
committer Ryan Mitchell <rtmitchell@google.com> Mon Aug 27 22:35:34 2018 +0000
tree 52bdaa2fda135ea05a60237c281981012bcbc0c7
parent 342df6ddd178d55aa94e01ff94e5be00457f3440

AAPT2: Fix resource table load time regression

A previous change (deee395) caused duplicate entries to be created for
entries eith entry ids greater than 0x0ff. This is because the wrong
data type was used (uint8_t instead of uint16_t). This made loading in
resources slower as well since more entries had to be iterated over.

Bug: 36051266
Test: Dumping all resources in 700 apks found in the android tree took 1
minute instead of 5 minutes. Created a test in aapt2_tests.

Change-Id: I1c3d830da517a56ac3496221dbe605c72e0c6014

tools/aapt2/ResourceTable_test.cpp

diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp
index 7fa8ea2..1aa9751 100644
--- a/tools/aapt2/ResourceTable_test.cpp
+++ b/tools/aapt2/ResourceTable_test.cpp
@@ -258,4 +258,44 @@
   ASSERT_FALSE(table.SetOverlayable(name, overlayable, test::GetDiagnostics()));
 }
 
+TEST(ResourceTableTest, AllowDuplictaeResourcesNames) {
+  ResourceTable table(/* validate_resources */ false);
+
+  const ResourceName foo_name = test::ParseNameOrDie("android:bool/foo");
+  ASSERT_TRUE(table.AddResourceWithId(foo_name, ResourceId(0x7f0100ff), ConfigDescription{} , "",
+                                      test::BuildPrimitive(android::Res_value::TYPE_INT_BOOLEAN, 0),
+                                      test::GetDiagnostics()));
+  ASSERT_TRUE(table.AddResourceWithId(foo_name, ResourceId(0x7f010100), ConfigDescription{} , "",
+                                      test::BuildPrimitive(android::Res_value::TYPE_INT_BOOLEAN, 1),
+                                      test::GetDiagnostics()));
+
+  ASSERT_TRUE(table.SetVisibilityWithId(foo_name, Visibility{Visibility::Level::kPublic},
+                                        ResourceId(0x7f0100ff), test::GetDiagnostics()));
+  ASSERT_TRUE(table.SetVisibilityWithId(foo_name, Visibility{Visibility::Level::kPrivate},
+                                        ResourceId(0x7f010100), test::GetDiagnostics()));
+
+  auto package = table.FindPackageById(0x7f);
+  ASSERT_THAT(package, NotNull());
+  auto type = package->FindType(ResourceType::kBool);
+  ASSERT_THAT(type, NotNull());
+
+  auto entry1 = type->FindEntry("foo", 0x00ff);
+  ASSERT_THAT(entry1, NotNull());
+  ASSERT_THAT(entry1->id, Eq(0x00ff));
+  ASSERT_THAT(entry1->values[0], NotNull());
+  ASSERT_THAT(entry1->values[0]->value, NotNull());
+  ASSERT_THAT(ValueCast<BinaryPrimitive>(entry1->values[0]->value.get()), NotNull());
+  ASSERT_THAT(ValueCast<BinaryPrimitive>(entry1->values[0]->value.get())->value.data, Eq(0u));
+  ASSERT_THAT(entry1->visibility.level, Visibility::Level::kPublic);
+
+  auto entry2 = type->FindEntry("foo", 0x0100);
+  ASSERT_THAT(entry2, NotNull());
+  ASSERT_THAT(entry2->id, Eq(0x0100));
+  ASSERT_THAT(entry2->values[0], NotNull());
+  ASSERT_THAT(entry1->values[0]->value, NotNull());
+  ASSERT_THAT(ValueCast<BinaryPrimitive>(entry2->values[0]->value.get()), NotNull());
+  ASSERT_THAT(ValueCast<BinaryPrimitive>(entry2->values[0]->value.get())->value.data, Eq(1u));
+  ASSERT_THAT(entry2->visibility.level, Visibility::Level::kPrivate);
+}
+
 }  // namespace aapt