AAPT2: Emit more proguard keep rules for layouts and menus
<fragment> tags in layouts use the class attribute to determine which
Fragment subclass to load, and fallback on android:name if class is not
set.
AAPT2 only emitted a proguard rule for the class attribute for <fragment>,
when it should emit a proguard rule for the android:name attribute as
well.
AAPT2 didn't handle menu XML, so support for actionViewClass,
actionProviderClass and onClick is added.
Bug: 62216174
Test: make aapt2_tests
Change-Id: Ie8675c2bd899a5b51f3661eb0901ab8c9a16fd70
diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp
index 624a559..5f61fae 100644
--- a/tools/aapt2/java/ProguardRules.cpp
+++ b/tools/aapt2/java/ProguardRules.cpp
@@ -85,7 +85,11 @@
bool check_class = false;
bool check_name = false;
if (node->namespace_uri.empty()) {
- check_class = node->name == "view" || node->name == "fragment";
+ if (node->name == "view") {
+ check_class = true;
+ } else if (node->name == "fragment") {
+ check_class = check_name = true;
+ }
} else if (node->namespace_uri == xml::kSchemaAndroid) {
check_name = node->name == "fragment";
}
@@ -110,6 +114,32 @@
DISALLOW_COPY_AND_ASSIGN(LayoutVisitor);
};
+class MenuVisitor : public BaseVisitor {
+ public:
+ MenuVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) {
+ }
+
+ virtual void Visit(xml::Element* node) override {
+ if (node->namespace_uri.empty() && node->name == "item") {
+ for (const auto& attr : node->attributes) {
+ if (attr.namespace_uri == xml::kSchemaAndroid) {
+ if ((attr.name == "actionViewClass" || attr.name == "actionProviderClass") &&
+ util::IsJavaClassName(attr.value)) {
+ AddClass(node->line_number, attr.value);
+ } else if (attr.name == "onClick") {
+ AddMethod(node->line_number, attr.value);
+ }
+ }
+ }
+ }
+
+ BaseVisitor::Visit(node);
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MenuVisitor);
+};
+
class XmlResourceVisitor : public BaseVisitor {
public:
XmlResourceVisitor(const Source& source, KeepSet* keep_set)
@@ -267,6 +297,12 @@
break;
}
+ case ResourceType::kMenu: {
+ MenuVisitor visitor(source, keep_set);
+ res->root->Accept(&visitor);
+ break;
+ }
+
default:
break;
}
diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp
new file mode 100644
index 0000000..900b073
--- /dev/null
+++ b/tools/aapt2/java/ProguardRules_test.cpp
@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "java/ProguardRules.h"
+
+#include "test/Test.h"
+
+using ::testing::HasSubstr;
+using ::testing::Not;
+
+namespace aapt {
+
+TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) {
+ std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+ std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"(
+ <fragment xmlns:android="http://schemas.android.com/apk/res/android"
+ android:name="com.foo.Bar"/>)");
+ 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, FragmentClassRuleIsEmitted) {
+ std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+ std::unique_ptr<xml::XmlResource> layout =
+ test::BuildXmlDom(R"(<fragment class="com.foo.Bar"/>)");
+ 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, FragmentNameAndClassRulesAreEmitted) {
+ std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+ std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"(
+ <fragment xmlns:android="http://schemas.android.com/apk/res/android"
+ android:name="com.foo.Baz"
+ class="com.foo.Bar"/>)");
+ 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"));
+ EXPECT_THAT(actual, HasSubstr("com.foo.Baz"));
+}
+
+TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) {
+ 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"
+ android:onClick="bar_method" />)");
+ 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("bar_method"));
+}
+
+TEST(ProguardRulesTest, MenuRulesAreEmitted) {
+ std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+ std::unique_ptr<xml::XmlResource> menu = test::BuildXmlDom(R"(
+ <menu xmlns:android="http://schemas.android.com/apk/res/android">
+ <item android:onClick="on_click"
+ android:actionViewClass="com.foo.Bar"
+ android:actionProviderClass="com.foo.Baz"
+ android:name="com.foo.Bat" />
+ </menu>)");
+ menu->file.name = test::ParseNameOrDie("menu/foo");
+
+ proguard::KeepSet set;
+ ASSERT_TRUE(proguard::CollectProguardRules({}, menu.get(), &set));
+
+ std::stringstream out;
+ ASSERT_TRUE(proguard::WriteKeepSet(&out, set));
+
+ std::string actual = out.str();
+ EXPECT_THAT(actual, HasSubstr("on_click"));
+ EXPECT_THAT(actual, HasSubstr("com.foo.Bar"));
+ EXPECT_THAT(actual, HasSubstr("com.foo.Baz"));
+ EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat")));
+}
+
+} // namespace aapt
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index 139d8d9..ebcd469 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -7,6 +7,8 @@
- Fixed issue where `--custom-package` was not honored when writing Manifest.java. (bug 62826426)
- Add `<key-sets>` and its nested tags to the allowed set of XML tags in AndroidManifest.xml.
(bug 62839863)
+- Fixed issue where Java classes referenced from fragments and menus were not added to
+ the set of Proguard keep rules. (bug 62216174)
## Version 2.17
### `aapt2 ...`