Merge "Only keep methods with correct signature for view/menu click"

tools/aapt2/java/ProguardRules.cpp

@@ -79,8 +79,10 @@ class BaseVisitor : public xml::Visitor {
     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({file_.name, file_.source.WithLine(line_number)}, method_name);
+  void AddMethod(size_t line_number, const std::string& method_name,
+                 const std::string& method_signature) {
+    keep_set_->AddMethod({file_.name, file_.source.WithLine(line_number)},
+        {method_name, method_signature});
   }
 
   void AddReference(size_t line_number, Reference* ref) {
@@ -125,7 +127,7 @@ class LayoutVisitor : public BaseVisitor {
         AddClass(node->line_number, attr.value);
       } else if (attr.namespace_uri == xml::kSchemaAndroid &&
                  attr.name == "onClick") {
-        AddMethod(node->line_number, attr.value);
+        AddMethod(node->line_number, attr.value, "android.view.View");
       }
     }
 
@@ -149,7 +151,7 @@ class MenuVisitor : public BaseVisitor {
               util::IsJavaClassName(attr.value)) {
             AddClass(node->line_number, attr.value);
           } else if (attr.name == "onClick") {
-            AddMethod(node->line_number, attr.value);
+            AddMethod(node->line_number, attr.value, "android.view.MenuItem");
           }
         }
       }
@@ -396,7 +398,8 @@ void WriteKeepSet(const KeepSet& keep_set, OutputStream* out) {
     for (const UsageLocation& location : entry.second) {
       printer.Print("# Referenced at ").Println(location.source.to_string());
     }
-    printer.Print("-keepclassmembers class * { *** ").Print(entry.first).Println("(...); }");
+    printer.Print("-keepclassmembers class * { *** ").Print(entry.first.name)
+        .Print("(").Print(entry.first.signature).Println("); }");
     printer.Println();
   }
 }

tools/aapt2/java/ProguardRules.h

@@ -40,6 +40,11 @@ struct UsageLocation {
   Source source;
 };
 
+struct NameAndSignature {
+  std::string name;
+  std::string signature;
+};
+
 class KeepSet {
  public:
   KeepSet() = default;
@@ -55,8 +60,8 @@ class KeepSet {
     conditional_class_set_[class_name].insert(file);
   }
 
-  inline void AddMethod(const UsageLocation& file, const std::string& method_name) {
-    method_set_[method_name].insert(file);
+  inline void AddMethod(const UsageLocation& file, const NameAndSignature& name_and_signature) {
+    method_set_[name_and_signature].insert(file);
   }
 
   inline void AddReference(const UsageLocation& file, const ResourceName& resource_name) {
@@ -71,7 +76,7 @@ class KeepSet {
 
   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<NameAndSignature, std::set<UsageLocation>> method_set_;
   std::map<std::string, std::set<UsageLocation>> conditional_class_set_;
   std::map<ResourceName, std::set<UsageLocation>> reference_set_;
 };
@@ -100,6 +105,20 @@ inline int operator<(const UsageLocation& lhs, const UsageLocation& rhs) {
   return lhs.name.compare(rhs.name);
 }
 
+//
+// NameAndSignature implementation.
+//
+
+inline bool operator<(const NameAndSignature& lhs, const NameAndSignature& rhs) {
+  if (lhs.name < rhs.name) {
+    return true;
+  }
+  if (lhs.name == rhs.name) {
+    return lhs.signature < rhs.signature;
+  }
+  return false;
+}
+
 }  // namespace proguard
 }  // namespace aapt
 

tools/aapt2/java/ProguardRules_test.cpp

@@ -239,7 +239,8 @@ TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keepclassmembers class * { *** bar_method(...); }"));
+  EXPECT_THAT(actual, HasSubstr(
+      "-keepclassmembers class * { *** bar_method(android.view.View); }"));
 }
 
 TEST(ProguardRulesTest, MenuRulesAreEmitted) {
@@ -258,7 +259,8 @@ TEST(ProguardRulesTest, MenuRulesAreEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keepclassmembers class * { *** on_click(...); }"));
+  EXPECT_THAT(actual, HasSubstr(
+      "-keepclassmembers class * { *** on_click(android.view.MenuItem); }"));
   EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
   EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }"));
   EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat")));