diff options
| author | 2017-07-21 10:55:27 -0700 | |
|---|---|---|
| committer | 2017-10-20 16:25:18 -0700 | |
| commit | dc21dea9b8b1157a4a9347b68301da4307c51168 (patch) | |
| tree | e5a861fbec159776bebc37296753ca21c7916593 /tools/aapt2/java | |
| parent | 0d769d80a9a871cd4f0e5dc24e63c86d97fb3ad8 (diff) | |
AAPT2: Produce Conditional Proguard Keep Rules
Add the option to produce keep rules that conditional keep based on
usage of R identifiers. This allows Proguard to potentially shrink more
code if resources are not used.
Currently only produces conditional rules for classes referenced in
layout resources because they are the most common and has the easiest
transitive usage chain to analyze.
Bug: 63628451
Test: make aapt2_tests and manual testing
Change-Id: I6c1af7affd64af40c80e004d8506a9463444b2c3
Diffstat (limited to 'tools/aapt2/java')
| -rw-r--r-- | tools/aapt2/java/JavaClassGenerator.cpp | 6 | ||||
| -rw-r--r-- | tools/aapt2/java/JavaClassGenerator.h | 3 | ||||
| -rw-r--r-- | tools/aapt2/java/ProguardRules.cpp | 174 | ||||
| -rw-r--r-- | tools/aapt2/java/ProguardRules.h | 63 | ||||
| -rw-r--r-- | tools/aapt2/java/ProguardRules_test.cpp | 112 |
5 files changed, 313 insertions, 45 deletions
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 8da9106aa8d7..3ba4dd880607 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -61,7 +61,7 @@ static bool IsValidSymbol(const StringPiece& symbol) { // Java symbols can not contain . or -, but those are valid in a resource name. // Replace those with '_'. -static std::string TransformToFieldName(const StringPiece& symbol) { +std::string JavaClassGenerator::TransformToFieldName(const StringPiece& symbol) { std::string output = symbol.to_string(); for (char& c : output) { if (c == '.' || c == '-') { @@ -89,9 +89,9 @@ static std::string TransformNestedAttr(const ResourceNameRef& attr_name, // the package. if (!attr_name.package.empty() && package_name_to_generate != attr_name.package) { - output += "_" + TransformToFieldName(attr_name.package); + output += "_" + JavaClassGenerator::TransformToFieldName(attr_name.package); } - output += "_" + TransformToFieldName(attr_name.entry); + output += "_" + JavaClassGenerator::TransformToFieldName(attr_name.entry); return output; } diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h index 18746ffc5a0a..2541749750a6 100644 --- a/tools/aapt2/java/JavaClassGenerator.h +++ b/tools/aapt2/java/JavaClassGenerator.h @@ -24,6 +24,7 @@ #include "ResourceTable.h" #include "ResourceValues.h" +#include "androidfw/StringPiece.h" #include "process/IResourceTableConsumer.h" #include "process/SymbolTable.h" @@ -78,6 +79,8 @@ class JavaClassGenerator { const std::string& getError() const; + static std::string TransformToFieldName(const android::StringPiece& symbol); + private: bool SkipSymbol(SymbolState state); bool SkipSymbol(const Maybe<SymbolTable::Symbol>& symbol); diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp index 10c46101123c..b9ae654a0ba6 100644 --- a/tools/aapt2/java/ProguardRules.cpp +++ b/tools/aapt2/java/ProguardRules.cpp @@ -21,6 +21,10 @@ #include "android-base/macros.h" +#include "JavaClassGenerator.h" +#include "ResourceUtils.h" +#include "ValueVisitor.h" +#include "androidfw/StringPiece.h" #include "util/Util.h" #include "xml/XmlDom.h" @@ -31,7 +35,7 @@ class BaseVisitor : public xml::Visitor { public: using xml::Visitor::Visit; - BaseVisitor(const Source& source, KeepSet* keep_set) : source_(source), keep_set_(keep_set) { + BaseVisitor(const ResourceFile& file, KeepSet* keep_set) : file_(file), keep_set_(keep_set) { } void Visit(xml::Element* node) override { @@ -52,27 +56,47 @@ class BaseVisitor : public xml::Visitor { for (const auto& child : node->children) { child->Accept(this); } + + for (const auto& attr : node->attributes) { + if (attr.compiled_value) { + auto ref = ValueCast<Reference>(attr.compiled_value.get()); + if (ref) { + AddReference(node->line_number, ref); + } + } + } } protected: - void AddClass(size_t line_number, const std::string& class_name) { - keep_set_->AddClass(Source(source_.path, line_number), class_name); + ResourceFile file_; + KeepSet* keep_set_; + + virtual void AddClass(size_t line_number, const std::string& class_name) { + keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)}, class_name); } void AddMethod(size_t line_number, const std::string& method_name) { - keep_set_->AddMethod(Source(source_.path, line_number), method_name); + keep_set_->AddMethod({file_.name, file_.source.WithLine(line_number)}, method_name); + } + + void AddReference(size_t line_number, Reference* ref) { + if (ref && ref->name) { + ResourceName ref_name = ref->name.value(); + if (ref_name.package.empty()) { + ref_name = ResourceName(file_.name.package, ref_name.type, ref_name.entry); + } + keep_set_->AddReference({file_.name, file_.source.WithLine(line_number)}, ref_name); + } } private: DISALLOW_COPY_AND_ASSIGN(BaseVisitor); - Source source_; - KeepSet* keep_set_; }; class LayoutVisitor : public BaseVisitor { public: - LayoutVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + LayoutVisitor(const ResourceFile& file, KeepSet* keep_set) : BaseVisitor(file, keep_set) { } void Visit(xml::Element* node) override { @@ -110,7 +134,7 @@ class LayoutVisitor : public BaseVisitor { class MenuVisitor : public BaseVisitor { public: - MenuVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + MenuVisitor(const ResourceFile& file, KeepSet* keep_set) : BaseVisitor(file, keep_set) { } void Visit(xml::Element* node) override { @@ -136,7 +160,7 @@ class MenuVisitor : public BaseVisitor { class XmlResourceVisitor : public BaseVisitor { public: - XmlResourceVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + XmlResourceVisitor(const ResourceFile& file, KeepSet* keep_set) : BaseVisitor(file, keep_set) { } void Visit(xml::Element* node) override { @@ -163,7 +187,7 @@ class XmlResourceVisitor : public BaseVisitor { class TransitionVisitor : public BaseVisitor { public: - TransitionVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + TransitionVisitor(const ResourceFile& file, KeepSet* keep_set) : BaseVisitor(file, keep_set) { } void Visit(xml::Element* node) override { @@ -185,8 +209,9 @@ class TransitionVisitor : public BaseVisitor { class ManifestVisitor : public BaseVisitor { public: - ManifestVisitor(const Source& source, KeepSet* keep_set, bool main_dex_only) - : BaseVisitor(source, keep_set), main_dex_only_(main_dex_only) {} + ManifestVisitor(const ResourceFile& file, KeepSet* keep_set, bool main_dex_only) + : BaseVisitor(file, keep_set), main_dex_only_(main_dex_only) { + } void Visit(xml::Element* node) override { if (node->namespace_uri.empty()) { @@ -241,6 +266,10 @@ class ManifestVisitor : public BaseVisitor { BaseVisitor::Visit(node); } + virtual void AddClass(size_t line_number, const std::string& class_name) override { + keep_set_->AddManifestClass({file_.name, file_.source.WithLine(line_number)}, class_name); + } + private: DISALLOW_COPY_AND_ASSIGN(ManifestVisitor); @@ -249,9 +278,8 @@ class ManifestVisitor : public BaseVisitor { std::string default_process_; }; -bool CollectProguardRulesForManifest(const Source& source, xml::XmlResource* res, KeepSet* keep_set, - bool main_dex_only) { - ManifestVisitor visitor(source, keep_set, main_dex_only); +bool CollectProguardRulesForManifest(xml::XmlResource* res, KeepSet* keep_set, bool main_dex_only) { + ManifestVisitor visitor(res->file, keep_set, main_dex_only); if (res->root) { res->root->Accept(&visitor); return true; @@ -259,58 +287,150 @@ bool CollectProguardRulesForManifest(const Source& source, xml::XmlResource* res return false; } -bool CollectProguardRules(const Source& source, xml::XmlResource* res, KeepSet* keep_set) { +bool CollectProguardRules(xml::XmlResource* res, KeepSet* keep_set) { if (!res->root) { return false; } switch (res->file.name.type) { case ResourceType::kLayout: { - LayoutVisitor visitor(source, keep_set); + LayoutVisitor visitor(res->file, keep_set); res->root->Accept(&visitor); break; } case ResourceType::kXml: { - XmlResourceVisitor visitor(source, keep_set); + XmlResourceVisitor visitor(res->file, keep_set); res->root->Accept(&visitor); break; } case ResourceType::kTransition: { - TransitionVisitor visitor(source, keep_set); + TransitionVisitor visitor(res->file, keep_set); res->root->Accept(&visitor); break; } case ResourceType::kMenu: { - MenuVisitor visitor(source, keep_set); + MenuVisitor visitor(res->file, keep_set); res->root->Accept(&visitor); break; } - default: + default: { + BaseVisitor visitor(res->file, keep_set); + res->root->Accept(&visitor); break; + } } return true; } bool WriteKeepSet(std::ostream* out, const KeepSet& keep_set) { - for (const auto& entry : keep_set.keep_set_) { - for (const Source& source : entry.second) { - *out << "# Referenced at " << source << "\n"; + for (const auto& entry : keep_set.manifest_class_set_) { + for (const UsageLocation& location : entry.second) { + *out << "# Referenced at " << location.source << "\n"; } *out << "-keep class " << entry.first << " { <init>(...); }\n" << std::endl; } - for (const auto& entry : keep_set.keep_method_set_) { - for (const Source& source : entry.second) { - *out << "# Referenced at " << source << "\n"; + for (const auto& entry : keep_set.conditional_class_set_) { + std::set<UsageLocation> locations; + bool can_be_conditional = true; + for (const UsageLocation& location : entry.second) { + can_be_conditional &= CollectLocations(location, keep_set, &locations); + } + + for (const UsageLocation& location : entry.second) { + *out << "# Referenced at " << location.source << "\n"; + } + if (keep_set.conditional_keep_rules_ && can_be_conditional) { + *out << "-keep class " << entry.first << " {\n ifused class **.R$layout {\n"; + for (const UsageLocation& location : locations) { + auto transformed_name = JavaClassGenerator::TransformToFieldName(location.name.entry); + *out << " int " << transformed_name << ";\n"; + } + *out << " };\n <init>(...);\n}\n" << std::endl; + } else { + *out << "-keep class " << entry.first << " { <init>(...); }\n" << std::endl; + } + } + + for (const auto& entry : keep_set.method_set_) { + for (const UsageLocation& location : entry.second) { + *out << "# Referenced at " << location.source << "\n"; } *out << "-keepclassmembers class * { *** " << entry.first << "(...); }\n" << std::endl; } return true; } +bool CollectLocations(const UsageLocation& location, const KeepSet& keep_set, + std::set<UsageLocation>* locations) { + locations->insert(location); + + // TODO: allow for more reference types if we can determine its safe. + if (location.name.type != ResourceType::kLayout) { + return false; + } + + for (const auto& entry : keep_set.reference_set_) { + if (entry.first == location.name) { + for (auto& refLocation : entry.second) { + // Don't get stuck in loops + if (locations->find(refLocation) != locations->end()) { + return false; + } + if (!CollectLocations(refLocation, keep_set, locations)) { + return false; + } + } + } + } + + return true; +} + +class ReferenceVisitor : public ValueVisitor { + public: + using ValueVisitor::Visit; + + ReferenceVisitor(aapt::IAaptContext* context, ResourceName from, KeepSet* keep_set) + : context_(context), from_(from), keep_set_(keep_set) { + } + + void Visit(Reference* reference) override { + if (reference->name) { + ResourceName reference_name = reference->name.value(); + if (reference_name.package.empty()) { + reference_name = ResourceName(context_->GetCompilationPackage(), reference_name.type, + reference_name.entry); + } + keep_set_->AddReference({from_, reference->GetSource()}, reference_name); + } + } + + private: + aapt::IAaptContext* context_; + ResourceName from_; + KeepSet* keep_set_; +}; + +bool CollectResourceReferences(aapt::IAaptContext* context, ResourceTable* table, + KeepSet* keep_set) { + for (auto& pkg : table->packages) { + for (auto& type : pkg->types) { + for (auto& entry : type->entries) { + for (auto& config_value : entry->values) { + ResourceName from(pkg->name, type->type, entry->name); + ReferenceVisitor visitor(context, from, keep_set); + config_value->value->Accept(&visitor); + } + } + } + } + return true; +} + } // namespace proguard } // namespace aapt diff --git a/tools/aapt2/java/ProguardRules.h b/tools/aapt2/java/ProguardRules.h index 3c349bab1217..8dbe3c2bd1e8 100644 --- a/tools/aapt2/java/ProguardRules.h +++ b/tools/aapt2/java/ProguardRules.h @@ -23,37 +23,80 @@ #include <string> #include "Resource.h" +#include "ResourceTable.h" #include "Source.h" +#include "ValueVisitor.h" +#include "androidfw/StringPiece.h" +#include "process/IResourceTableConsumer.h" #include "xml/XmlDom.h" namespace aapt { namespace proguard { +struct UsageLocation { + ResourceName name; + Source source; +}; + class KeepSet { public: - inline void AddClass(const Source& source, const std::string& class_name) { - keep_set_[class_name].insert(source); + KeepSet() = default; + + KeepSet(bool conditional_keep_rules) : conditional_keep_rules_(conditional_keep_rules) { + } + + inline void AddManifestClass(const UsageLocation& file, const std::string& class_name) { + manifest_class_set_[class_name].insert(file); + } + + inline void AddConditionalClass(const UsageLocation& file, const std::string& class_name) { + conditional_class_set_[class_name].insert(file); } - inline void AddMethod(const Source& source, const std::string& method_name) { - keep_method_set_[method_name].insert(source); + inline void AddMethod(const UsageLocation& file, const std::string& method_name) { + method_set_[method_name].insert(file); + } + + inline void AddReference(const UsageLocation& file, const ResourceName& resource_name) { + reference_set_[resource_name].insert(file); } private: friend bool WriteKeepSet(std::ostream* out, const KeepSet& keep_set); - std::map<std::string, std::set<Source>> keep_set_; - std::map<std::string, std::set<Source>> keep_method_set_; + friend bool CollectLocations(const UsageLocation& location, const KeepSet& keep_set, + std::set<UsageLocation>* locations); + + bool conditional_keep_rules_ = false; + std::map<std::string, std::set<UsageLocation>> manifest_class_set_; + std::map<std::string, std::set<UsageLocation>> method_set_; + std::map<std::string, std::set<UsageLocation>> conditional_class_set_; + std::map<ResourceName, std::set<UsageLocation>> reference_set_; }; -bool CollectProguardRulesForManifest(const Source& source, - xml::XmlResource* res, KeepSet* keep_set, +bool CollectProguardRulesForManifest(xml::XmlResource* res, KeepSet* keep_set, bool main_dex_only = false); -bool CollectProguardRules(const Source& source, xml::XmlResource* res, - KeepSet* keep_set); +bool CollectProguardRules(xml::XmlResource* res, KeepSet* keep_set); +bool CollectResourceReferences(aapt::IAaptContext* context, ResourceTable* table, + KeepSet* keep_set); bool WriteKeepSet(std::ostream* out, const KeepSet& keep_set); +bool CollectLocations(const UsageLocation& location, const KeepSet& keep_set, + std::set<UsageLocation>* locations); + +// +// UsageLocation implementation. +// + +inline bool operator==(const UsageLocation& lhs, const UsageLocation& rhs) { + return lhs.name == rhs.name; +} + +inline int operator<(const UsageLocation& lhs, const UsageLocation& rhs) { + return lhs.name.compare(rhs.name); +} + } // namespace proguard } // namespace aapt diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp index 900b07339715..df3ac8b0809f 100644 --- a/tools/aapt2/java/ProguardRules_test.cpp +++ b/tools/aapt2/java/ProguardRules_test.cpp @@ -15,6 +15,7 @@ */ #include "java/ProguardRules.h" +#include "link/Linkers.h" #include "test/Test.h" @@ -31,7 +32,7 @@ TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules({}, layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); std::stringstream out; ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); @@ -47,7 +48,7 @@ TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules({}, layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); std::stringstream out; ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); @@ -65,7 +66,7 @@ TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules({}, layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); std::stringstream out; ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); @@ -75,6 +76,107 @@ TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) { EXPECT_THAT(actual, HasSubstr("com.foo.Baz")); } +TEST(ProguardRulesTest, CustomViewRulesAreEmitted) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"( + <View xmlns:android="http://schemas.android.com/apk/res/android"> + <com.foo.Bar /> + </View>)"); + layout->file.name = test::ParseNameOrDie("layout/foo"); + + proguard::KeepSet set; + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + + std::stringstream out; + ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); + + std::string actual = out.str(); + EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); +} + +TEST(ProguardRulesTest, IncludedLayoutRulesAreConditional) { + std::unique_ptr<xml::XmlResource> bar_layout = test::BuildXmlDom(R"( + <View xmlns:android="http://schemas.android.com/apk/res/android"> + <com.foo.Bar /> + </View>)"); + bar_layout->file.name = test::ParseNameOrDie("com.foo:layout/bar"); + + ResourceTable table; + StdErrDiagnostics errDiagnostics; + table.AddResource(bar_layout->file.name, ConfigDescription::DefaultConfig(), "", + util::make_unique<FileReference>(), &errDiagnostics); + + std::unique_ptr<IAaptContext> context = + test::ContextBuilder() + .SetCompilationPackage("com.foo") + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(&table)) + .Build(); + + std::unique_ptr<xml::XmlResource> foo_layout = test::BuildXmlDom(R"( + <View xmlns:android="http://schemas.android.com/apk/res/android"> + <include layout="@layout/bar" /> + </View>)"); + foo_layout->file.name = test::ParseNameOrDie("com.foo:layout/foo"); + + XmlReferenceLinker xml_linker; + ASSERT_TRUE(xml_linker.Consume(context.get(), bar_layout.get())); + ASSERT_TRUE(xml_linker.Consume(context.get(), foo_layout.get())); + + proguard::KeepSet set = proguard::KeepSet(true); + ASSERT_TRUE(proguard::CollectProguardRules(bar_layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(foo_layout.get(), &set)); + + std::stringstream out; + ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); + + std::string actual = out.str(); + EXPECT_THAT(actual, HasSubstr("ifused class **.R$layout")); + EXPECT_THAT(actual, HasSubstr("int foo")); + EXPECT_THAT(actual, HasSubstr("int bar")); + EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); +} + +TEST(ProguardRulesTest, AliasedLayoutRulesAreConditional) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"( + <View xmlns:android="http://schemas.android.com/apk/res/android"> + <com.foo.Bar /> + </View>)"); + layout->file.name = test::ParseNameOrDie("layout/foo"); + + proguard::KeepSet set = proguard::KeepSet(true); + set.AddReference({test::ParseNameOrDie("layout/bar"), {}}, layout->file.name); + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + + std::stringstream out; + ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); + + std::string actual = out.str(); + EXPECT_THAT(actual, HasSubstr("ifused class **.R$layout")); + EXPECT_THAT(actual, HasSubstr("int foo")); + EXPECT_THAT(actual, HasSubstr("int bar")); + EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); +} + +TEST(ProguardRulesTest, NonLayoutReferencesAreUnconditional) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"( + <View xmlns:android="http://schemas.android.com/apk/res/android"> + <com.foo.Bar /> + </View>)"); + layout->file.name = test::ParseNameOrDie("layout/foo"); + + proguard::KeepSet set = proguard::KeepSet(true); + set.AddReference({test::ParseNameOrDie("style/MyStyle"), {}}, layout->file.name); + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + + std::stringstream out; + ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); + + std::string actual = out.str(); + EXPECT_THAT(actual, Not(HasSubstr("ifused"))); +} + TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"( @@ -83,7 +185,7 @@ TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules({}, layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); std::stringstream out; ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); @@ -104,7 +206,7 @@ TEST(ProguardRulesTest, MenuRulesAreEmitted) { menu->file.name = test::ParseNameOrDie("menu/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules({}, menu.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(menu.get(), &set)); std::stringstream out; ASSERT_TRUE(proguard::WriteKeepSet(&out, set)); |