Merge "Only check sibling config values to dedupe"

This commit is contained in:
Ryan Mitchell
2019-08-19 23:53:55 +00:00
committed by Android (Google) Code Review
2 changed files with 52 additions and 4 deletions

View File

@@ -63,13 +63,14 @@ class DominatedKeyValueRemover : public DominatorTree::BottomUpVisitor {
// Compare compatible configs for this entry and ensure the values are
// equivalent.
const ConfigDescription& node_configuration = node_value->config;
for (const auto& sibling : entry_->values) {
if (!sibling->value) {
for (const auto& sibling : parent->children()) {
ResourceConfigValue* sibling_value = sibling->value();
if (!sibling_value->value) {
// Sibling was already removed.
continue;
}
if (node_configuration.IsCompatibleWith(sibling->config) &&
!node_value->value->Equals(sibling->value.get())) {
if (node_configuration.IsCompatibleWith(sibling_value->config) &&
!node_value->value->Equals(sibling_value->value.get())) {
// The configurations are compatible, but the value is
// different, so we can't remove this value.
return;

View File

@@ -80,11 +80,58 @@ TEST(ResourceDeduperTest, DifferentValuesAreKept) {
.Build();
ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get()));
EXPECT_THAT(table, HasValue("android:string/keep", default_config));
EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_config));
EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_v21_config));
EXPECT_THAT(table, HasValue("android:string/keep", land_config));
}
TEST(ResourceDeduperTest, SameValuesAreDedupedIncompatibleSiblings) {
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
const ConfigDescription default_config = {};
const ConfigDescription ldrtl_config = test::ParseConfigOrDie("ldrtl");
const ConfigDescription ldrtl_night_config = test::ParseConfigOrDie("ldrtl-night");
// Chosen because this configuration is not compatible with ldrtl-night.
const ConfigDescription ldrtl_notnight_config = test::ParseConfigOrDie("ldrtl-notnight");
std::unique_ptr<ResourceTable> table =
test::ResourceTableBuilder()
.AddString("android:string/keep", ResourceId{}, default_config, "keep")
.AddString("android:string/keep", ResourceId{}, ldrtl_config, "dedupe")
.AddString("android:string/keep", ResourceId{}, ldrtl_night_config, "dedupe")
.AddString("android:string/keep", ResourceId{}, ldrtl_notnight_config, "keep2")
.Build();
ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get()));
EXPECT_THAT(table, HasValue("android:string/keep", default_config));
EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_config));
EXPECT_THAT(table, Not(HasValue("android:string/keep", ldrtl_night_config)));
EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_notnight_config));
}
TEST(ResourceDeduperTest, SameValuesAreDedupedCompatibleNonSiblings) {
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
const ConfigDescription default_config = {};
const ConfigDescription ldrtl_config = test::ParseConfigOrDie("ldrtl");
const ConfigDescription ldrtl_night_config = test::ParseConfigOrDie("ldrtl-night");
// Chosen because this configuration is compatible with ldrtl.
const ConfigDescription land_config = test::ParseConfigOrDie("land");
std::unique_ptr<ResourceTable> table =
test::ResourceTableBuilder()
.AddString("android:string/keep", ResourceId{}, default_config, "keep")
.AddString("android:string/keep", ResourceId{}, ldrtl_config, "dedupe")
.AddString("android:string/keep", ResourceId{}, ldrtl_night_config, "dedupe")
.AddString("android:string/keep", ResourceId{}, land_config, "keep2")
.Build();
ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get()));
EXPECT_THAT(table, HasValue("android:string/keep", default_config));
EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_config));
EXPECT_THAT(table, Not(HasValue("android:string/keep", ldrtl_night_config)));
EXPECT_THAT(table, HasValue("android:string/keep", land_config));
}
TEST(ResourceDeduperTest, LocalesValuesAreKept) {
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
const ConfigDescription default_config = {};