diff --git a/tools/aapt2/link/NoDefaultResourceRemover.cpp b/tools/aapt2/link/NoDefaultResourceRemover.cpp index cfb4b26aba02b..5173b8541943b 100644 --- a/tools/aapt2/link/NoDefaultResourceRemover.cpp +++ b/tools/aapt2/link/NoDefaultResourceRemover.cpp @@ -22,17 +22,7 @@ namespace aapt { -static bool IsDefaultConfigRequired(const ConfigDescription& config) { - // We don't want to be overzealous with resource removal, so have strict requirements. - // If a resource defines a value for a locale-only configuration, the default configuration is - // required. - if (ConfigDescription::DefaultConfig().diff(config) == ConfigDescription::CONFIG_LOCALE) { - return true; - } - return false; -} - -static bool KeepResource(const std::unique_ptr& entry) { +static bool KeepResource(const std::unique_ptr& entry, int minSdk) { if (entry->visibility.level == Visibility::Level::kPublic) { // Removing a public API without the developer knowing is bad, so just leave this here for now. return true; @@ -44,22 +34,44 @@ static bool KeepResource(const std::unique_ptr& entry) { } // There is no default value defined, check if removal is required. + bool defaultRequired = false; for (const auto& config_value : entry->values) { - if (IsDefaultConfigRequired(config_value->config)) { - return false; + const int config = ConfigDescription::DefaultConfig().diff(config_value->config); + // If a resource defines a value for a locale-only configuration, the default configuration is + // required. + if (config == ConfigDescription::CONFIG_LOCALE) { + defaultRequired = true; + } + // If a resource defines a version-only config, the config value can be used as a default if + // the version is at most the minimum sdk version + else if (config == ConfigDescription::CONFIG_VERSION + && config_value->config.sdkVersion <= minSdk) { + return true; + } + // If a resource defines a value for a density only configuration, then that value could be used + // as a default and the entry should not be removed + else if (config == ConfigDescription::CONFIG_DENSITY + || (config == (ConfigDescription::CONFIG_DENSITY | ConfigDescription::CONFIG_VERSION) + && config_value->config.sdkVersion <= minSdk)) { + return true; } } - return true; + + return !defaultRequired; } bool NoDefaultResourceRemover::Consume(IAaptContext* context, ResourceTable* table) { - const ConfigDescription default_config = ConfigDescription::DefaultConfig(); for (auto& pkg : table->packages) { for (auto& type : pkg->types) { + // Gather the entries without defaults that must be removed + const int minSdk = context->GetMinSdkVersion(); const auto end_iter = type->entries.end(); - const auto new_end_iter = - std::stable_partition(type->entries.begin(), end_iter, KeepResource); - for (auto iter = new_end_iter; iter != end_iter; ++iter) { + const auto remove_iter = std::stable_partition(type->entries.begin(), end_iter, + [&minSdk](const std::unique_ptr& entry) -> bool { + return KeepResource(entry, minSdk); + }); + + for (auto iter = remove_iter; iter != end_iter; ++iter) { const ResourceName name(pkg->name, type->type, (*iter)->name); IDiagnostics* diag = context->GetDiagnostics(); diag->Warn(DiagMessage() << "removing resource " << name @@ -74,7 +86,7 @@ bool NoDefaultResourceRemover::Consume(IAaptContext* context, ResourceTable* tab } } - type->entries.erase(new_end_iter, type->entries.end()); + type->entries.erase(remove_iter, end_iter); } } return true; diff --git a/tools/aapt2/link/NoDefaultResourceRemover_test.cpp b/tools/aapt2/link/NoDefaultResourceRemover_test.cpp index 943709a2af125..d129c9ac8db7b 100644 --- a/tools/aapt2/link/NoDefaultResourceRemover_test.cpp +++ b/tools/aapt2/link/NoDefaultResourceRemover_test.cpp @@ -46,4 +46,64 @@ TEST(NoDefaultResourceRemoverTest, RemoveEntryWithNoDefaultAndOnlyLocales) { EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:string/baz"))); } +TEST(NoDefaultResourceRemoverTest, KeepEntryWithLocalesAndDensities) { + std::unique_ptr context = test::ContextBuilder().SetMinSdkVersion(26).Build(); + std::unique_ptr table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("mdpi")) // v4 + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/keep2", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/keep2", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/keep2", test::ParseConfigOrDie("xxxhdpi")) // v4 + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("w600dp-xhdpi")) // v13 + .Build(); + + NoDefaultResourceRemover remover; + ASSERT_TRUE(remover.Consume(context.get(), table.get())); + + EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:drawable/keep1"))); + EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:drawable/keep2"))); + EXPECT_FALSE(table->FindResource(test::ParseNameOrDie("android:drawable/remove1"))); +} + +TEST(NoDefaultResourceRemoverTest, RemoveEntryWithLocalesAndDensitiesLowVersion) { + std::unique_ptr context = test::ContextBuilder().SetMinSdkVersion(3).Build(); + std::unique_ptr table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("mdpi")) // v4 + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) + .Build(); + + NoDefaultResourceRemover remover; + ASSERT_TRUE(remover.Consume(context.get(), table.get())); + + EXPECT_FALSE(table->FindResource(test::ParseNameOrDie("android:drawable/remove1"))); +} + +TEST(NoDefaultResourceRemoverTest, KeepEntryWithVersion) { + std::unique_ptr context = test::ContextBuilder().SetMinSdkVersion(8).Build(); + std::unique_ptr table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("v8")) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("v9")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) + .Build(); + + NoDefaultResourceRemover remover; + ASSERT_TRUE(remover.Consume(context.get(), table.get())); + + EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:drawable/keep1"))); + EXPECT_FALSE(table->FindResource(test::ParseNameOrDie("android:drawable/remove1"))); +} + } // namespace aapt