Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

AAPT2: Accept density config as default

Browse Source 
When a resource does not have a default config value defined, and the
resource only has config values defined for language translations, that
resource should be removed; however, if the resource also contains a
density specific config value, the resource should not be removed.

This is because a density specific value should have the same content as
what the default value would have, just higher or lower quality.

Bug: 76413048
Test: aapt2_tests
Change-Id: I52fe37a7aa3aefb694d3af6c8adfd6269fa26f54
This commit is contained in:
Ryan Mitchell
2018-05-23 13:31:28 -07:00
parent 5cfe061ed5
commit a593605dfa
2 changed files with 91 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
tools/aapt2/link/NoDefaultResourceRemover.cpp
View File

@@ -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<ResourceEntry>& entry) {
static bool KeepResource(const std::unique_ptr<ResourceEntry>& 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<ResourceEntry>& 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<ResourceEntry>& 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;

60
tools/aapt2/link/NoDefaultResourceRemover_test.cpp
View File

@@ -46,4 +46,64 @@ TEST(NoDefaultResourceRemoverTest, RemoveEntryWithNoDefaultAndOnlyLocales) {
EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:string/baz")));
}
TEST(NoDefaultResourceRemoverTest, KeepEntryWithLocalesAndDensities) {
std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(26).Build();
std::unique_ptr<ResourceTable> 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<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(3).Build();
std::unique_ptr<ResourceTable> 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<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(8).Build();
std::unique_ptr<ResourceTable> 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