From 527ebbaa55f42e90a05f12f0154294d2e9d16903 Mon Sep 17 00:00:00 2001
From: Ryan Mitchell <rtmitchell@google.com>
Date: Fri, 30 Oct 2020 12:32:47 -0700
Subject: Fix DominatorTree for locale and mcc/mnc config

De-duping of configurations with locales was disabled previously since
there is not a good way to dedupe locales in a forwards compatible way
(change SHA: e38567480be67ac83a8f8f090704bb0d49e2eed2).

In b/171892595, since every locale is a root in the dominator tree,
configs that do not specify locale qualifiers are dominated by the
default config and their values are checked for compatiblity with the
locale config values.

b/171892595 took a while to detect because, this is only an issue at
runtime when a resource has one config containing mnc/mcc without a
locale, one config containing a locale, and the values for the configs
differ. This is because mcc/mnc is the only qualifier with a greater
precedence than locale.

Make configurations with mcc/mnc and mcc unable to be dominated until
locale deduping is fixed.

Bug: 171892595
Bug: 62409213
Test: aapt2-tests
Change-Id: Ia0a5e5d7a1650d070f5f2fcaf9a8469a8c7dabe6
---
 tools/aapt2/DominatorTree_test.cpp            | 28 +++++++++++++++++++++++++++
 tools/aapt2/optimize/ResourceDeduper_test.cpp | 22 +++++++++++++++++++++
 2 files changed, 50 insertions(+)

(limited to 'tools')

diff --git a/tools/aapt2/DominatorTree_test.cpp b/tools/aapt2/DominatorTree_test.cpp
index 3e49034310c3..52949da1b64f 100644
--- a/tools/aapt2/DominatorTree_test.cpp
+++ b/tools/aapt2/DominatorTree_test.cpp
@@ -198,5 +198,33 @@ TEST(DominatorTreeTest, NonZeroDensitiesMatch) {
   EXPECT_EQ(expected, printer.ToString(&tree));
 }
 
+TEST(DominatorTreeTest, MccMncIsPeertoLocale) {
+  const ConfigDescription default_config = {};
+  const ConfigDescription de_config = test::ParseConfigOrDie("de");
+  const ConfigDescription fr_config = test::ParseConfigOrDie("fr");
+  const ConfigDescription mcc_config = test::ParseConfigOrDie("mcc262");
+  const ConfigDescription mcc_fr_config = test::ParseConfigOrDie("mcc262-fr");
+  const ConfigDescription mnc_config = test::ParseConfigOrDie("mnc2");
+  const ConfigDescription mnc_fr_config = test::ParseConfigOrDie("mnc2-fr");
+  std::vector<std::unique_ptr<ResourceConfigValue>> configs;
+  configs.push_back(util::make_unique<ResourceConfigValue>(default_config, ""));
+  configs.push_back(util::make_unique<ResourceConfigValue>(de_config, ""));
+  configs.push_back(util::make_unique<ResourceConfigValue>(fr_config, ""));
+  configs.push_back(util::make_unique<ResourceConfigValue>(mcc_config, ""));
+  configs.push_back(util::make_unique<ResourceConfigValue>(mcc_fr_config, ""));
+  configs.push_back(util::make_unique<ResourceConfigValue>(mnc_config, ""));
+  configs.push_back(util::make_unique<ResourceConfigValue>(mnc_fr_config, ""));
+  DominatorTree tree(configs);
+  PrettyPrinter printer;
+  std::string expected =
+      "<default>\n"
+      "de\n"
+      "fr\n"
+      "mcc262\n"
+      "mcc262-fr\n"
+      "mnc2\n"
+      "mnc2-fr\n";
+  EXPECT_EQ(expected, printer.ToString(&tree));
+}
 
 }  // namespace aapt
diff --git a/tools/aapt2/optimize/ResourceDeduper_test.cpp b/tools/aapt2/optimize/ResourceDeduper_test.cpp
index 048e318d2802..888de40be268 100644
--- a/tools/aapt2/optimize/ResourceDeduper_test.cpp
+++ b/tools/aapt2/optimize/ResourceDeduper_test.cpp
@@ -52,9 +52,11 @@ TEST(ResourceDeduperTest, SameValuesAreDeduped) {
           .Build();
 
   ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get()));
+  EXPECT_THAT(table, HasValue("android:string/dedupe", default_config));
   EXPECT_THAT(table, Not(HasValue("android:string/dedupe", ldrtl_config)));
   EXPECT_THAT(table, Not(HasValue("android:string/dedupe", land_config)));
 
+  EXPECT_THAT(table, HasValue("android:string/dedupe2", default_config));
   EXPECT_THAT(table, HasValue("android:string/dedupe2", ldrtl_v21_config));
   EXPECT_THAT(table, Not(HasValue("android:string/dedupe2", ldrtl_config)));
 
@@ -151,4 +153,24 @@ TEST(ResourceDeduperTest, LocalesValuesAreKept) {
   EXPECT_THAT(table, HasValue("android:string/keep", fr_rCA_config));
 }
 
+TEST(ResourceDeduperTest, MccMncValuesAreKept) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+  const ConfigDescription default_config = {};
+  const ConfigDescription mcc_config = test::ParseConfigOrDie("mcc262");
+  const ConfigDescription mnc_config = test::ParseConfigOrDie("mnc2");
+
+  std::unique_ptr<ResourceTable> table =
+      test::ResourceTableBuilder()
+          .AddString("android:string/keep", ResourceId{}, default_config, "keep")
+          .AddString("android:string/keep", ResourceId{}, mcc_config, "keep")
+          .AddString("android:string/keep", ResourceId{}, mnc_config, "keep")
+          .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", mcc_config));
+  EXPECT_THAT(table, HasValue("android:string/keep", mnc_config));
+}
+
+
 }  // namespace aapt
-- 
cgit v1.2.3