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

tools/aapt2/java/ProguardRules.cpp

@@ -85,7 +85,11 @@ class LayoutVisitor : public BaseVisitor {
     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 @@ class LayoutVisitor : public BaseVisitor {
   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 @@ bool CollectProguardRules(const Source& source, xml::XmlResource* res,
       break;
     }
 
+    case ResourceType::kMenu: {
+      MenuVisitor visitor(source, keep_set);
+      res->root->Accept(&visitor);
+      break;
+    }
+
     default:
       break;
   }

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

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 ...`