[Java.Interop] Add JniRuntime.JniValueManager.TryCreatePeer() (#1301)

Context: 78d5937
Context: be6cc8f
Context: dotnet/android@7a772f0
Context: dotnet/android#9716
Context: dotnet/android@694e975

dotnet/android@7a772f03 added the beginnings of a NativeAOT sample
to dotnet/android which built a ".NET for Android" app using
NativeAOT, which "rhymed with" the `Hello-NativeAOTFromAndroid`
sample in 78d5937.

Further work on the sample showed that it was lacking support for
`Android.App.Application` subclasses:

	[Application (Name = "my.MainApplication")]
	public class MainApplication : Application
	{
	    public MainApplication (IntPtr handle, JniHandleOwnership transfer)
	        : base (handle, transfer)
	    {
	    }

	    public override void OnCreate ()
	    {
	        base.OnCreate ();
	    }
	}


dotnet/android#9716 began fixing that oversight, but in the process
was triggering a stack overflow because when it needed to create a
"proxy" peer around the `my.MainApplication` Java type, which
subclassed `android.app.Application`.  Instead of creating an instance
of the expected `MainApplication` C# type, it instead created an
instance of `Android.App.Application`.  This was visible from the logs:

	Created PeerReference=0x2d06/G IdentityHashCode=0x8edcb07 Instance=0x957d2a Instance.Type=Android.App.Application, Java.Type=my/MainApplication

Note that `Instance.Type` is `Android.App.Application`, not the
in-sample `MainApplication` C# type.

Because the runtime type was `Android.App.Application`, when we later
attempted to dispatch the `Application.OnCreate()` override, this
resulted in a *virtual* invocation of the Java `Application.onCreate()`
method instead of a *non-virtual* invocation of
`Application.onCreate()`.  This virtual invocation was the root of a
recursive loop which eventually resulted in a stack overflow.

The fix in dotnet/android@694e975e was to fix
`NativeAotTypeManager.CreatePeer()` so that it properly checked for a
binding of the *runtime type* of the Java instance *before* using the
"fallback" type provided to `Object.GetObject<T>()` in the
`Application.n_OnCreate()` method:

	partial class Application {
	    static void n_OnCreate (IntPtr jnienv, IntPtr native__this)
	    {
	        // …
	        var __this = global::Java.Lang.Object.GetObject<
		    Android.App.Application     // This is the "fallback" NativeAotTypeManager
		> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
	        __this.OnCreate ();
	        // …
	    }
	}

All well and good.

The problem is that `NativeAotTypeManager` in dotnet/android needs to
support *both* dotnet/java-interop "activation constructors" with a
signature of `(ref JniObjectReference, JniObjectReferenceOptions)`,
*and* the .NET for Android signature of `(IntPtr, JniHandleOwnership)`.
Trying to support both constructors resulted in the need to copy
*all* of `JniRuntime.JniValueManager.CreatePeer()` *and dependencies*,
which felt a bit excessive.

Add a new `JniRuntime.JniValueManager.TryCreatePeer()` method, which
will invoke the activation constructor to create an `IJavaPeerable`:

	partial class JniRuntime {
	  partial class JniValueManager {
	    protected virtual IJavaPeerable? TryCreatePeer (
	        ref JniObjectReference reference,
	        JniObjectReferenceOptions options,
	        Type targetType);
	  }
	}

If the activation constructor is not found, then `TryCreatePeer()`
shall return `null`, allowing `CreatePeerInstance()` to try for
a base type or, ultimately, the fallback type.

This will allow a future dotnet/android to *remove*
`NativeAotTypeManager.CreatePeer()` and its dependencies entirely,
and instead do:

	partial class NativeAotTypeManager {
	    const   BindingFlags        ActivationConstructorBindingFlags   = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
	    static  readonly    Type[]  XAConstructorSignature  = new Type [] { typeof (IntPtr), typeof (JniHandleOwnership) };
	    protected override IJavaPeerable TryCreatePeer (ref JniObjectReference reference, JniObjectReferenceOptions options, Type type)
	    {
	        var c = type.GetConstructor (ActivationConstructorBindingFlags, null, XAConstructorSignature, null);
	        if (c != null) {
	            var args = new object[] {
	                reference.Handle,
	                JniHandleOwnership.DoNotTransfer,
	            };
	            var p       = (IJavaPeerable) c.Invoke (args);
	            JniObjectReference.Dispose (ref reference, options);
	            return p;
	        }
	        return base.TryCreatePeer (ref reference, options, type);
	    }
	}

vastly reducing the code it needs to care about.

Additionally, add `JniRuntime.JniTypeManager.GetInvokerType()`:

	partial class JniRuntime {
	  partial class JniTypeManager {
	    public Type? GetInvokerType (Type type);
	    protected virtual Type? GetInvokerTypeCore (Type type);
	  }
	}

`GetInvokerType()` calls `GetInvokerTypeCore()`, allowing flexibility
for subclasses to lookup the "string-based" the .NET for Android -style
Invoker types that JavaInterop1-style bindings used before commit
be6cc8f.

Author: jonpryor

src/Java.Interop/Documentation/Java.Interop/JniRuntime.JniTypeManager.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0"?>
+<docs>
+  <member name="T:JniTypeManager">
+    <summary>
+      Manages bound Java types.
+    </summary>
+    <remarks>
+    </remarks>
+  </member>
+  <member name="M:GetInvokerType">
+    <summary>Gets the <i>Invoker</i> type for <paramref name="type" /></summary>
+    <remarks>
+      <para>
+        An <i>Invoker type</i> is a concrete type which can be constructed,
+        which is used to invoke instances of abstract type that cannot be constructed.
+        For example, the interface type <c>Java.Lang.IRunnable</c> cannot be constructed,
+        but if a <c>java.lang.Runnable</c> instance enters managed code,
+        a Invoker must be constructed around the instance so that it may be used.
+      </para>
+    </remarks>
+    <returns>
+      If <paramref name="type" /> is an interface or abstract class, returns the
+      type which should be constructed around instances of <paramref name="type" />.
+      If no such type exists, or if <paramref name="type" /> is a concrete type,
+      then <see langword="null" /> is returned.
+    </returns>
+  </member>
+</docs>

src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs

@@ -79,8 +79,10 @@ public override string ToString ()
 		}
 #endif  // NET
 
+		/// <include file="../Documentation/Java.Interop/JniRuntime.JniTypeManager.xml" path="/docs/member[@name='T:JniTypeManager']/*" />
 		public partial class JniTypeManager : IDisposable, ISetRuntime {
 
+			internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;
 			internal const DynamicallyAccessedMemberTypes Methods = DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods;
 			internal const DynamicallyAccessedMemberTypes MethodsAndPrivateNested = Methods | DynamicallyAccessedMemberTypes.NonPublicNestedTypes;
 			internal const DynamicallyAccessedMemberTypes MethodsConstructors = MethodsAndPrivateNested | DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;
@@ -385,6 +387,49 @@ IEnumerable<Type> CreateGetTypesForSimpleReferenceEnumerator (string jniSimpleRe
 				yield break;
 			}
 
+			/// <include file="../Documentation/Java.Interop/JniRuntime.JniTypeManager.xml" path="/docs/member[@name='M:GetInvokerType']/*" />
+			[return: DynamicallyAccessedMembers (Constructors)]
+			public Type? GetInvokerType (
+					[DynamicallyAccessedMembers (Constructors)]
+					Type type)
+			{
+				if (type.IsAbstract || type.IsInterface) {
+					return GetInvokerTypeCore (type);
+				}
+				return null;
+			}
+
+			[return: DynamicallyAccessedMembers (Constructors)]
+			protected virtual Type? GetInvokerTypeCore (
+					[DynamicallyAccessedMembers (Constructors)]
+					Type type)
+			{
+				// https://github.com/xamarin/xamarin-android/blob/5472eec991cc075e4b0c09cd98a2331fb93aa0f3/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L176-L186
+				const string makeGenericTypeMessage = "Generic 'Invoker' types are preserved by the MarkJavaObjects trimmer step.";
+
+				[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = makeGenericTypeMessage)]
+				[return: DynamicallyAccessedMembers (Constructors)]
+				static Type MakeGenericType (
+						[DynamicallyAccessedMembers (Constructors)]
+						Type type,
+						Type [] arguments) =>
+					// FIXME: https://github.com/dotnet/java-interop/issues/1192
+					#pragma warning disable IL3050
+					type.MakeGenericType (arguments);
+					#pragma warning restore IL3050
+
+				var signature   = type.GetCustomAttribute<JniTypeSignatureAttribute> ();
+				if (signature == null || signature.InvokerType == null) {
+					return null;
+				}
+
+				Type[] arguments = type.GetGenericArguments ();
+				if (arguments.Length == 0)
+					return signature.InvokerType;
+
+				return MakeGenericType (signature.InvokerType, arguments);
+			}
+
 #if NET
 
 			public IReadOnlyList<string>? GetStaticMethodFallbackTypes (string jniSimpleReference)

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

@@ -336,31 +336,21 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 
 				JniObjectReference.Dispose (ref targetClass);
 
-				var ctor = GetPeerConstructor (ref refClass, targetType);
-				if (ctor == null) {
+				var peer = CreatePeerInstance (ref refClass, targetType, ref reference, transfer);
+				if (peer == null) {
 					throw new NotSupportedException (string.Format ("Could not find an appropriate constructable wrapper type for Java type '{0}', targetType='{1}'.",
 							JniEnvironment.Types.GetJniTypeNameFromInstance (reference), targetType));
 				}
-
-				var acts = new object[] {
-					reference,
-					transfer,
-				};
-				try {
-					var peer    = (IJavaPeerable) ctor.Invoke (acts);
-					peer.SetJniManagedPeerState (peer.JniManagedPeerState | JniManagedPeerStates.Replaceable);
-					return peer;
-				} finally {
-					reference   = (JniObjectReference) acts [0];
-				}
+				peer.SetJniManagedPeerState (peer.JniManagedPeerState | JniManagedPeerStates.Replaceable);
+				return peer;
 			}
 
-			static  readonly    Type    ByRefJniObjectReference = typeof (JniObjectReference).MakeByRefType ();
-
-			ConstructorInfo? GetPeerConstructor (
+			IJavaPeerable? CreatePeerInstance (
 					ref JniObjectReference klass,
 					[DynamicallyAccessedMembers (Constructors)]
-					Type fallbackType)
+					Type fallbackType,
+					ref JniObjectReference reference,
+					JniObjectReferenceOptions transfer)
 			{
 				var jniTypeName = JniEnvironment.Types.GetJniTypeNameFromClass (klass);
 
@@ -373,11 +363,11 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 					type    = Runtime.TypeManager.GetType (sig);
 
 					if (type != null) {
-						var ctor = GetActivationConstructor (type);
+						var peer = TryCreatePeerInstance (ref reference, transfer, type);
 
-						if (ctor != null) {
+						if (peer != null) {
 							JniObjectReference.Dispose (ref klass);
-							return ctor;
+							return peer;
 						}
 					}
 
@@ -391,51 +381,41 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 				}
 				JniObjectReference.Dispose (ref klass, JniObjectReferenceOptions.CopyAndDispose);
 
-				return GetActivationConstructor (fallbackType);
+				return TryCreatePeerInstance (ref reference, transfer, fallbackType);
 			}
 
-			static ConstructorInfo? GetActivationConstructor (
+			IJavaPeerable? TryCreatePeerInstance (
+					ref JniObjectReference reference,
+					JniObjectReferenceOptions options,
 					[DynamicallyAccessedMembers (Constructors)]
 					Type type)
 			{
-				if (type.IsAbstract || type.IsInterface) {
-					type = GetInvokerType (type) ?? type;
-				}
-				foreach (var c in type.GetConstructors (BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) {
-					var p = c.GetParameters ();
-					if (p.Length == 2 && p [0].ParameterType == ByRefJniObjectReference && p [1].ParameterType == typeof (JniObjectReferenceOptions))
-						return c;
-				}
-				return null;
+				type    = Runtime.TypeManager.GetInvokerType (type) ?? type;
+				return TryCreatePeer (ref reference, options, type);
 			}
 
-			[return: DynamicallyAccessedMembers (Constructors)]
-			static Type? GetInvokerType (Type type)
-			{
-				// https://github.com/xamarin/xamarin-android/blob/5472eec991cc075e4b0c09cd98a2331fb93aa0f3/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L176-L186
-				const string makeGenericTypeMessage = "Generic 'Invoker' types are preserved by the MarkJavaObjects trimmer step.";
-
-				[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = makeGenericTypeMessage)]
-				[return: DynamicallyAccessedMembers (Constructors)]
-				static Type MakeGenericType (
-						[DynamicallyAccessedMembers (Constructors)]
-						Type type,
-						Type [] arguments) =>
-					// FIXME: https://github.com/dotnet/java-interop/issues/1192
-					#pragma warning disable IL3050
-					type.MakeGenericType (arguments);
-					#pragma warning restore IL3050
-
-				var signature   = type.GetCustomAttribute<JniTypeSignatureAttribute> ();
-				if (signature == null || signature.InvokerType == null) {
-					return null;
-				}
+			const               BindingFlags    ActivationConstructorBindingFlags   = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
+			static  readonly    Type            ByRefJniObjectReference = typeof (JniObjectReference).MakeByRefType ();
+			static  readonly    Type[]          JIConstructorSignature  = new Type [] { ByRefJniObjectReference, typeof (JniObjectReferenceOptions) };
 
-				Type[] arguments = type.GetGenericArguments ();
-				if (arguments.Length == 0)
-					return signature.InvokerType;
 
-				return MakeGenericType (signature.InvokerType, arguments);
+			protected virtual IJavaPeerable? TryCreatePeer (
+					ref JniObjectReference reference,
+					JniObjectReferenceOptions options,
+					[DynamicallyAccessedMembers (Constructors)]
+					Type type)
+			{
+				var c = type.GetConstructor (ActivationConstructorBindingFlags, null, JIConstructorSignature, null);
+				if (c != null) {
+					var args = new object[] {
+						reference,
+						options,
+					};
+					var p       = (IJavaPeerable) c.Invoke (args);
+					reference   = (JniObjectReference) args [0];
+					return p;
+				}
+				return null;
 			}
 
 			public object? CreateValue (

src/Java.Interop/PublicAPI.Unshipped.txt

@@ -3,8 +3,11 @@ static Java.Interop.JniEnvironment.BeginMarshalMethod(nint jnienv, out Java.Inte
 static Java.Interop.JniEnvironment.EndMarshalMethod(ref Java.Interop.JniTransition transition) -> void
 virtual Java.Interop.JniRuntime.OnEnterMarshalMethod() -> void
 virtual Java.Interop.JniRuntime.OnUserUnhandledException(ref Java.Interop.JniTransition transition, System.Exception! e) -> void
+virtual Java.Interop.JniRuntime.JniTypeManager.GetInvokerTypeCore(System.Type! type) -> System.Type?
+virtual Java.Interop.JniRuntime.JniValueManager.TryCreatePeer(ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions options, System.Type! type) -> Java.Interop.IJavaPeerable?
 Java.Interop.JavaException.JavaException(ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions transfer, Java.Interop.JniObjectReference throwableOverride) -> void
 Java.Interop.JavaException.SetJavaStackTrace(Java.Interop.JniObjectReference peerReferenceOverride = default(Java.Interop.JniObjectReference)) -> void
+Java.Interop.JniRuntime.JniTypeManager.GetInvokerType(System.Type! type) -> System.Type?
 Java.Interop.JniRuntime.JniValueManager.GetPeer(Java.Interop.JniObjectReference reference, System.Type? targetType = null) -> Java.Interop.IJavaPeerable?
 Java.Interop.JniTypeSignatureAttribute.InvokerType.get -> System.Type?
 Java.Interop.JniTypeSignatureAttribute.InvokerType.set -> void

tests/Java.Interop-Tests/Java.Interop/JniRuntime.JniTypeManagerTests.cs

@@ -0,0 +1,33 @@
+using System;
+using System.Reflection;
+using System.Collections.Generic;
+
+using Java.Interop;
+
+using NUnit.Framework;
+
+namespace Java.InteropTests {
+
+	[TestFixture]
+	public class JniRuntimeJniTypeManagerTests : JavaVMFixture {
+
+		[Test]
+		public void GetInvokerType ()
+		{
+			using (var vm  = new MyTypeManager ()) {
+				// Concrete type; no invoker
+				Assert.IsNull (vm.GetInvokerType (typeof (JavaObject)));
+
+				// Not a bound abstract Java type; no invoker
+				Assert.IsNull (vm.GetInvokerType (typeof (System.ICloneable)));
+
+				// Bound abstract Java type; has an invoker
+				Assert.AreSame (typeof (IJavaInterfaceInvoker), vm.GetInvokerType (typeof (IJavaInterface)));
+			}
+		}
+
+		class MyTypeManager : JniRuntime.JniTypeManager {
+		}
+    }
+}
+