3 files changed, 225 insertions, 79 deletions

diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp
index 5ebf508807e8..9f6ec210a6a7 100644
--- a/tools/aapt2/java/ManifestClassGenerator_test.cpp
+++ b/tools/aapt2/java/ManifestClassGenerator_test.cpp
@@ -18,124 +18,115 @@
 
 #include "test/Test.h"
 
-namespace aapt {
+using ::testing::HasSubstr;
+using ::testing::Not;
 
-static ::testing::AssertionResult GetManifestClassText(IAaptContext* context,
-                                                       xml::XmlResource* res,
-                                                       std::string* out_str) {
-  std::unique_ptr<ClassDefinition> manifest_class =
-      GenerateManifestClass(context->GetDiagnostics(), res);
-  if (!manifest_class) {
-    return ::testing::AssertionFailure() << "manifest_class == nullptr";
-  }
-
-  std::stringstream out;
-  if (!manifest_class->WriteJavaFile(manifest_class.get(), "android", true,
-                                     &out)) {
-    return ::testing::AssertionFailure() << "failed to write java file";
-  }
+namespace aapt {
 
-  *out_str = out.str();
-  return ::testing::AssertionSuccess();
-}
+static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
+                                                       std::string* out_str);
 
 TEST(ManifestClassGeneratorTest, NameIsProperlyGeneratedFromSymbol) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
-  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"EOF(
-        <manifest xmlns:android="http://schemas.android.com/apk/res/android">
-          <permission android:name="android.permission.ACCESS_INTERNET" />
-          <permission android:name="android.DO_DANGEROUS_THINGS" />
-          <permission android:name="com.test.sample.permission.HUH" />
-          <permission-group android:name="foo.bar.PERMISSION" />
-        </manifest>)EOF");
+  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android">
+        <permission android:name="android.permission.ACCESS_INTERNET" />
+        <permission android:name="android.DO_DANGEROUS_THINGS" />
+        <permission android:name="com.test.sample.permission.HUH" />
+        <permission-group android:name="foo.bar.PERMISSION" />
+      </manifest>)");
 
   std::string actual;
   ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
 
-  const size_t permission_class_pos =
-      actual.find("public static final class permission {");
-  const size_t permission_croup_class_pos =
+  ASSERT_THAT(actual, HasSubstr("public static final class permission {"));
+  ASSERT_THAT(actual, HasSubstr("public static final class permission_group {"));
+
+  const size_t permission_start_pos = actual.find("public static final class permission {");
+  const size_t permission_group_start_pos =
       actual.find("public static final class permission_group {");
-  ASSERT_NE(std::string::npos, permission_class_pos);
-  ASSERT_NE(std::string::npos, permission_croup_class_pos);
 
   //
   // Make sure these permissions are in the permission class.
   //
-
-  size_t pos = actual.find(
-      "public static final String ACCESS_INTERNET="
-      "\"android.permission.ACCESS_INTERNET\";");
-  EXPECT_GT(pos, permission_class_pos);
-  EXPECT_LT(pos, permission_croup_class_pos);
-
-  pos = actual.find(
-      "public static final String DO_DANGEROUS_THINGS="
-      "\"android.DO_DANGEROUS_THINGS\";");
-  EXPECT_GT(pos, permission_class_pos);
-  EXPECT_LT(pos, permission_croup_class_pos);
-
-  pos = actual.find(
-      "public static final String HUH=\"com.test.sample.permission.HUH\";");
-  EXPECT_GT(pos, permission_class_pos);
-  EXPECT_LT(pos, permission_croup_class_pos);
+  const std::string permission_class =
+      actual.substr(permission_start_pos, permission_group_start_pos - permission_start_pos);
+
+  EXPECT_THAT(
+      permission_class,
+      HasSubstr(
+          "public static final String ACCESS_INTERNET=\"android.permission.ACCESS_INTERNET\";"));
+  EXPECT_THAT(
+      permission_class,
+      HasSubstr("public static final String DO_DANGEROUS_THINGS=\"android.DO_DANGEROUS_THINGS\";"));
+  EXPECT_THAT(permission_class,
+              HasSubstr("public static final String HUH=\"com.test.sample.permission.HUH\";"));
 
   //
   // Make sure these permissions are in the permission_group class
   //
+  const std::string permission_group_class = actual.substr(permission_group_start_pos);
 
-  pos = actual.find(
-      "public static final String PERMISSION="
-      "\"foo.bar.PERMISSION\";");
-  EXPECT_GT(pos, permission_croup_class_pos);
-  EXPECT_LT(pos, std::string::npos);
+  EXPECT_THAT(permission_group_class,
+              HasSubstr("public static final String PERMISSION=\"foo.bar.PERMISSION\";"));
 }
 
 TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
-  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"EOF(
-        <manifest xmlns:android="http://schemas.android.com/apk/res/android">
-          <!-- Required to access the internet.
-               Added in API 1. -->
-          <permission android:name="android.permission.ACCESS_INTERNET" />
-          <!-- @deprecated This permission is for playing outside. -->
-          <permission android:name="android.permission.PLAY_OUTSIDE" />
-          <!-- This is a private permission for system only!
-               @hide
-               @SystemApi -->
-          <permission android:name="android.permission.SECRET" />
-        </manifest>)EOF");
+  std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android">
+        <!-- Required to access the internet.
+             Added in API 1. -->
+        <permission android:name="android.permission.ACCESS_INTERNET" />
+        <!-- @deprecated This permission is for playing outside. -->
+        <permission android:name="android.permission.PLAY_OUTSIDE" />
+        <!-- This is a private permission for system only!
+             @hide
+             @SystemApi -->
+        <permission android:name="android.permission.SECRET" />
+      </manifest>)");
 
   std::string actual;
   ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
 
-  const char* expected_access_internet =
-      R"EOF(    /**
+  const char* expected_access_internet = R"(    /**
      * Required to access the internet.
      * Added in API 1.
      */
-    public static final String ACCESS_INTERNET="android.permission.ACCESS_INTERNET";)EOF";
-
-  EXPECT_NE(std::string::npos, actual.find(expected_access_internet));
+    public static final String ACCESS_INTERNET="android.permission.ACCESS_INTERNET";)";
+  EXPECT_THAT(actual, HasSubstr(expected_access_internet));
 
-  const char* expected_play_outside =
-      R"EOF(    /**
+  const char* expected_play_outside = R"(    /**
      * @deprecated This permission is for playing outside.
      */
     @Deprecated
-    public static final String PLAY_OUTSIDE="android.permission.PLAY_OUTSIDE";)EOF";
-
-  EXPECT_NE(std::string::npos, actual.find(expected_play_outside));
+    public static final String PLAY_OUTSIDE="android.permission.PLAY_OUTSIDE";)";
+  EXPECT_THAT(actual, HasSubstr(expected_play_outside));
 
-  const char* expected_secret =
-      R"EOF(    /**
+  const char* expected_secret = R"(    /**
      * This is a private permission for system only!
      * @hide
      */
     @android.annotation.SystemApi
-    public static final String SECRET="android.permission.SECRET";)EOF";
+    public static final String SECRET="android.permission.SECRET";)";
+  EXPECT_THAT(actual, HasSubstr(expected_secret));
+}
 
-  EXPECT_NE(std::string::npos, actual.find(expected_secret));
+static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
+                                                       std::string* out_str) {
+  std::unique_ptr<ClassDefinition> manifest_class =
+      GenerateManifestClass(context->GetDiagnostics(), res);
+  if (!manifest_class) {
+    return ::testing::AssertionFailure() << "manifest_class == nullptr";
+  }
+
+  std::stringstream out;
+  if (!manifest_class->WriteJavaFile(manifest_class.get(), "android", true, &out)) {
+    return ::testing::AssertionFailure() << "failed to write java file";
+  }
+
+  *out_str = out.str();
+  return ::testing::AssertionSuccess();
 }
 
 }  // namespace aapt
diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp
index 624a559c4dae..5f61faeeebe7 100644
--- a/tools/aapt2/java/ProguardRules.cpp
+++ b/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;
   }
diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp
new file mode 100644
index 000000000000..900b07339715
--- /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