Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 527ebbaa authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

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: e3856748).

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
parent 82cb76f9

libs/androidfw/ConfigDescription.cpp

+4 −1
Original line number Diff line number Diff line
@@ -887,13 +887,16 @@ bool ConfigDescription::Dominates(const ConfigDescription& o) const { 
  }



  // Locale de-duping is not-trivial, disable for now (b/62409213).

  if (diff(o) & CONFIG_LOCALE) {

  // We must also disable de-duping for all configuration qualifiers with precedence higher than

  // locale (b/171892595)

  if (diff(o) & (CONFIG_LOCALE | CONFIG_MCC | CONFIG_MNC)) {

    return false;

  }



  if (*this == DefaultConfig()) {

    return true;

  }



  return MatchWithDensity(o) && !o.MatchWithDensity(*this) &&

         !isMoreSpecificThan(o) && !o.HasHigherPrecedenceThan(*this);

}

tools/aapt2/DominatorTree_test.cpp

+28 −0
Original line number Diff line number Diff line
@@ -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

tools/aapt2/optimize/ResourceDeduper_test.cpp

+22 −0
Original line number Diff line number Diff line
@@ -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