[Xamarin.Android.Build.Tasks] fix proguard rules for R8 compat (dotnet#2735)

Fixes: dotnet#2684

Context: https://www.guardsquare.com/en/products/proguard/manual/usage#keepoptionmodifiers
Context: https://www.guardsquare.com/en/products/proguard/manual/examples

Using R8 on a pre-Android 8.0 device was crashing with:

	Unhandled Exception:
	Java.Lang.LinkageError: no non-static method "Landroid/runtime/UncaughtExceptionHandler;.<init>()V"
	  at Java.Interop.JniEnvironment+InstanceMethods.GetMethodID (Java.Interop.JniObjectReference type, System.String name, System.String signature) [0x0005b] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
	  at Java.Interop.JniType.GetConstructor (System.String signature) [0x0000c] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.GetConstructor (System.String signature) [0x00035] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.FinishCreateInstance (System.String constructorSignature, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) [0x00036] in <7d7bcc9ee9cc460db8abcdb9a9622733>:0
	  at Java.Lang.Object..ctor () [0x00054] in <d77389624c8c4948a12589c4bd4500eb>:0
	  at Android.Runtime.UncaughtExceptionHandler..ctor (Java.Lang.Thread+IUncaughtExceptionHandler defaultHandler) [0x00000] in <d77389624c8c4948a12589c4bd4500eb>:0
	  at Android.Runtime.JNIEnv.Initialize (Android.Runtime.JnienvInitializeArgs* args) [0x00202] in <d77389624c8c4948a12589c4bd4500eb>:0
	  --- End of managed Java.Lang.LinkageError stack trace ---
	java.lang.NoSuchMethodError: no non-static method "Landroid/runtime/UncaughtExceptionHandler;.<init>()V"
	  at mono.android.Runtime.init(Native Method)
	  at mono.MonoPackageManager.LoadApplication(:21)
	  at mono.MonoRuntimeProvider.attachInfo(:1)
	  at android.app.ActivityThread.installProvider(ActivityThread.java:5853)
	  at android.app.ActivityThread.installContentProviders(ActivityThread.java:5445)
	  at android.app.ActivityThread.handleBindApplication(ActivityThread.java:5384)
	  at android.app.ActivityThread.-wrap2(ActivityThread.java)
	  at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1545)
	  at android.os.Handler.dispatchMessage(Handler.java:102)
	  at android.os.Looper.loop(Looper.java:154)
	  at android.app.ActivityThread.main(ActivityThread.java:6119)
	  at java.lang.reflect.Method.invoke(Native Method)
	  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
	  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

Looking into it, the following method was present when using
ProGuard, but not R8:

	#1              : (in Landroid/runtime/UncaughtExceptionHandler;)
	  name          : '<init>'
	  type          : '()V'
	  access        : 0x10001 (PUBLIC CONSTRUCTOR)
	  code          -
	  registers     : 4
	  ins           : 1
	  outs          : 4
	  insns size    : 22 16-bit code units
	  catches       : (none)
	  positions     :
	    0x0000 line=22
	    0x0003 line=23
	    0x0010 line=24
	  locals        :
	    0x0000 - 0x0016 reg=3 this Landroid/runtime/UncaughtExceptionHandler;

I looked at `proguard_xamarin.cfg`, and noticed something strange:

	-keep class android.runtime.** { <init>(***); }

It seemed this rule was being ignored?  All the other *working* rules
were using `<init>(...);` to refer to the instance constructor.

I reviewed [ProGuard's documented syntax/grammar][0]:

	[@annotationtype] [[!]public|final|abstract|@ ...] [!]interface|class|enum classname
	    [extends|implements [@annotationtype] classname]
	[{
	    [@annotationtype] [[!]public|private|protected|static|volatile|transient ...]
	        <fields> |
	        (fieldtype fieldname);
	    [@annotationtype] [[!]public|private|protected|static|synchronized|native|abstract|strictfp ...]
	        <methods> |
	        <init>(argumenttype,...) |
	        classname(argumenttype,...) |
	        (returntype methodname(argumenttype,...));
	    [@annotationtype] [[!]public|private|protected|static ... ] *;
	    ...
	}]

It seems `(...)` is the proper way to specify a match against any set
of arguments.  Looking through ProGuard's examples, I can't find any
usage of `(***)`.  It must *happened* to work, but is undocumented.

R8 does not appear to accept the `(***)` syntax at all.

Update `proguard_xamarin.cfg` so that it uses `(...)`, not `(***)`,
allowing R8 to properly process `proguard_xamarin.cfg`.

~~ Other changes ~~

I expanded upon the `DexUtils` class so it can look for methods within
classes. I also adjusted one test to verify this problem is solved.

[0]: https://www.guardsquare.com/en/products/proguard/manual/usage#classspecification

Author: jonathanpeppers

src/Xamarin.Android.Build.Tasks/Resources/proguard_xamarin.cfg

@@ -14,15 +14,15 @@
 -keep class opentk_1_0.GameViewBase { *; <init>(...); }
 -keep class com.xamarin.java_interop.ManagedPeer { *; <init>(...); }
 
--keep class android.runtime.** { <init>(***); }
--keep class assembly_mono_android.android.runtime.** { <init>(***); }
+-keep class android.runtime.** { <init>(...); }
+-keep class assembly_mono_android.android.runtime.** { <init>(...); }
 # hash for android.runtime and assembly_mono_android.android.runtime.
 -keep class md52ce486a14f4bcd95899665e9d932190b.** { *; <init>(...); }
 -keepclassmembers class md52ce486a14f4bcd95899665e9d932190b.** { *; <init>(...); }
 
 # Android's template misses fluent setters...
 -keepclassmembers class * extends android.view.View {
-   *** set*(***);
+   *** set*(...);
 }
 
 # also misses those inflated custom layout stuff from xml...

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

@@ -774,10 +774,15 @@ public void BuildProguardEnabledProject ([Values (true, false)] bool isRelease,
 					Assert.IsTrue (StringAssertEx.ContainsText (File.ReadAllLines (proguardProjectPrimary), "-keep class md52d9cf6333b8e95e8683a477bc589eda5.MainActivity"), "`md52d9cf6333b8e95e8683a477bc589eda5.MainActivity` should exist in `proguard_project_primary.cfg`!");
 				}
 
-				var className = "Lmono/MonoRuntimeProvider;";
 				var dexFile = b.Output.GetIntermediaryPath (Path.Combine ("android", "bin", "classes.dex"));
 				FileAssert.Exists (dexFile);
-				Assert.IsTrue (DexUtils.ContainsClass (className, dexFile, b.AndroidSdkDirectory), $"`{dexFile}` should include `{className}`!");
+				var classes = new [] {
+					"Lmono/MonoRuntimeProvider;",
+					"Landroid/runtime/UncaughtExceptionHandler;",
+				};
+				foreach (var className in classes) {
+					Assert.IsTrue (DexUtils.ContainsClassWithMethod (className, "<init>", "()V", dexFile, b.AndroidSdkDirectory), $"`{dexFile}` should include `{className}`!");
+				}
 			}
 		}
 

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/DexUtils.cs

@@ -9,10 +9,60 @@ namespace Xamarin.ProjectTools
 {
 	public static class DexUtils
 	{
+		/*
+		 Example dexdump output:
+		 
+			Class #12            -
+			  Class descriptor  : 'Landroid/runtime/UncaughtExceptionHandler;'
+			  Access flags      : 0x0001 (PUBLIC)
+			  Superclass        : 'Ljava/lang/Object;'
+			  Interfaces        -
+				#0              : 'Ljava/lang/Thread$UncaughtExceptionHandler;'
+				#1              : 'Lmono/android/IGCUserPeer;'
+			  Static fields     -
+				#0              : (in Landroid/runtime/UncaughtExceptionHandler;)
+				  name          : '__md_methods'
+				  type          : 'Ljava/lang/String;'
+				  access        : 0x0019 (PUBLIC STATIC FINAL)
+			  Instance fields   -
+				#0              : (in Landroid/runtime/UncaughtExceptionHandler;)
+				  name          : 'refList'
+				  type          : 'Ljava/util/ArrayList;'
+				  access        : 0x0002 (PRIVATE)
+			  Direct methods    -
+				#0              : (in Landroid/runtime/UncaughtExceptionHandler;)
+				  name          : '<clinit>'
+				  type          : '()V'
+				  access        : 0x10008 (STATIC CONSTRUCTOR)
+				  code          -
+				  registers     : 3
+				  ins           : 0
+				  outs          : 3
+				  insns size    : 10 16-bit code units
+				  catches       : (none)
+				  positions     : 
+					0x0002 line=16
+				  locals        : 
+				#1              : (in Landroid/runtime/UncaughtExceptionHandler;)
+				  name          : '<init>'
+				  type          : '()V'
+				  access        : 0x10001 (PUBLIC CONSTRUCTOR)
+				  code          -
+				  registers     : 4
+				  ins           : 1
+				  outs          : 4
+				  insns size    : 22 16-bit code units
+				  catches       : (none)
+				  positions     : 
+					0x0000 line=22
+					0x0003 line=23
+					0x0010 line=24
+				  locals        : 
+					0x0000 - 0x0016 reg=3 this Landroid/runtime/UncaughtExceptionHandler; 
+		 */
+
 		/// <summary>
 		/// Runs the dexdump command to see if a class exists in a dex file
-		///     dexdump returns data of the form:
-		///     Class descriptor  : 'Landroid/app/ActivityTracker;'
 		/// </summary>
 		/// <param name="className">A Java class name of the form 'Landroid/app/ActivityTracker;'</param>
 		public static bool ContainsClass (string className, string dexFile, string androidSdkDirectory)
@@ -22,13 +72,45 @@ public static bool ContainsClass (string className, string dexFile, string andro
 				if (e.Data != null && e.Data.Contains ("Class descriptor") && e.Data.Contains (className))
 					containsClass = true;
 			};
+			DexDump (handler, dexFile, androidSdkDirectory);			
+			return containsClass;
+		}
 
-			var androidSdk = new AndroidSdkInfo ((l, m) => {
-					Console.WriteLine ($"{l}: {m}");
-					if (l == TraceLevel.Error) {
-						throw new Exception (m);
+		/// <summary>
+		/// Runs the dexdump command to see if a class exists in a dex file *and* has a public constructor
+		/// </summary>
+		/// <param name="className">A Java class name of the form 'Landroid/app/ActivityTracker;'</param>
+		/// <param name="method">A Java method name of the form 'foo'</param>
+		/// <param name="type">A Java method signature of the form '()V'</param>
+		public static bool ContainsClassWithMethod (string className, string method, string type, string dexFile, string androidSdkDirectory)
+		{
+			bool inClass = false;
+			bool hasName = false;
+			bool hasType = false;
+			DataReceivedEventHandler handler = (s, e) => {
+				if (e.Data != null) {
+					if (e.Data.Contains ("Class descriptor")) {
+						inClass = e.Data.Contains (className);
+						hasName = false;
+					} else if (inClass && e.Data.Contains ("name") && e.Data.Contains (method)) {
+						hasName = true;
+					} else if (hasName && e.Data.Contains ("type") && e.Data.Contains (type)) {
+						hasType = true;
 					}
-				}, androidSdkDirectory);
+				}
+			};
+			DexDump (handler, dexFile, androidSdkDirectory);
+			return hasType;
+		}
+
+		static void DexDump (DataReceivedEventHandler handler, string dexFile, string androidSdkDirectory)
+		{
+			var androidSdk = new AndroidSdkInfo ((l, m) => {
+				Console.WriteLine ($"{l}: {m}");
+				if (l == TraceLevel.Error) {
+					throw new Exception (m);
+				}
+			}, androidSdkDirectory);
 			var buildToolsPath = androidSdk.GetBuildToolsPaths ().FirstOrDefault ();
 			if (string.IsNullOrEmpty (buildToolsPath)) {
 				throw new Exception ($"Unable to find build-tools in `{androidSdkDirectory}`!");
@@ -53,8 +135,6 @@ public static bool ContainsClass (string className, string dexFile, string andro
 				p.BeginOutputReadLine ();
 				p.WaitForExit ();
 			}
-
-			return containsClass;
 		}
 	}
 }