[Java.Interop] CreatePeer() must satisfy targetType (#1308)

Context: dotnet/android#9747
Context: https://discord.com/channels/732297728826277939/732297837953679412/1339638847864176640
Context: https://discord.com/channels/732297728826277939/732297837953679412/1340011105510101063

While trying to get a MAUI sample running atop NativeAOT, we're
observing the following crash:

	E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidCastException: Arg_InvalidCastException
	E AndroidRuntime:    at Java.Lang.Object._GetObject[T](IntPtr, JniHandleOwnership) + 0x64
	E AndroidRuntime:    at Microsoft.Maui.WindowOverlay.Initialize() + 0x168

Further investigation shows that the crash is from accessing the
`Activity.WindowManager` property:

	namespace Android.App;
	partial class Activity {
	  public virtual unsafe Android.Views.IWindowManager? WindowManager {
	    // Metadata.xml XPath method reference: path="/api/package[@name='android.app']/class[@name='Activity']/method[@name='getWindowManager' and count(parameter)=0]"
	    [Register ("getWindowManager", "()Landroid/view/WindowManager;", "GetGetWindowManagerHandler")]
	    get {
	      const string __id = "getWindowManager.()Landroid/view/WindowManager;";
	      try {
	        var __rm = _members.InstanceMethods.InvokeVirtualObjectMethod (__id, this, null);
	        return global::Java.Lang.Object.GetObject<Android.Views.IWindowManager> (__rm.Handle, JniHandleOwnership.TransferLocalRef);
	      } finally {
	      }
	    }
	  }
	}

`Object.GetObject<T>()` is now a wrapper around
`JniRuntime.JniValueManager.GetPeer()`, so the problem, rephrased,
is that in this:

	var peer = JniEnvironment.Runtime.ValueManager.GetPeer (
	    ref h,
	    JniObjectReferenceOptions.CopyAndDispose,
	    targetType:typeof (IWindowManager));
	var wm   = (IWindowManager) peer;

`peer` is a value which *does not implement* `IWindowManager`.
It was, in fact, returning a `Java.Lang.Object` instance (!).
Consequently, the cast to `IWindowManager` throws.

The cause of the bug is that `JniRuntime.JniValueManager.CreatePeer()`
was not checking that the `Type` returned form
`Runtime.TypeManager.GetType(JniTypeSignature)` was compatible with
`targetType`; instead, it returned the first type in the inheritance
chain that had an activation constructor.  This was `Java.Lang.Object`.

Later, when `_GetObject<T>()` tried to cast the return value of
`JniRuntime.JniValueManager.GetPeer()` to `IWindowManager`, it failed.

Fix this by updating `CreatePeer()` to check that the `Type` from
`JniRuntime.JniTypeManager.GetType(JniTypeSignature)` is assignable
to `targetType`.  This ensures that we *don't* return a
`Java.Lang.Object` instance, allowing the cast to succeed.

Update `JniRuntimeJniValueManagerContract` to test these new semantics.

Author: jonpryor

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

@@ -348,7 +348,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 			IJavaPeerable? CreatePeerInstance (
 					ref JniObjectReference klass,
 					[DynamicallyAccessedMembers (Constructors)]
-					Type fallbackType,
+					Type targetType,
 					ref JniObjectReference reference,
 					JniObjectReferenceOptions transfer)
 			{
@@ -362,7 +362,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 
 					type    = Runtime.TypeManager.GetType (sig);
 
-					if (type != null) {
+					if (type != null && targetType.IsAssignableFrom (type)) {
 						var peer = TryCreatePeerInstance (ref reference, transfer, type);
 
 						if (peer != null) {
@@ -381,7 +381,7 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 				}
 				JniObjectReference.Dispose (ref klass, JniObjectReferenceOptions.CopyAndDispose);
 
-				return TryCreatePeerInstance (ref reference, transfer, fallbackType);
+				return TryCreatePeerInstance (ref reference, transfer, targetType);
 			}
 
 			IJavaPeerable? TryCreatePeerInstance (

tests/Java.Interop-Tests/Java.Interop-Tests.csproj

@@ -36,6 +36,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\AnotherJavaInterfaceImpl.java" />
     <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CrossReferenceBridge.java" />
     <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CallNonvirtualBase.java" />
     <JavaInteropTestJar Include="$(MSBuildThisFileDirectory)java\net\dot\jni\test\CallNonvirtualDerived.java" />

tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs

@@ -37,6 +37,7 @@ class JavaVMFixtureTypeManager : JniRuntime.JniTypeManager {
 			[GenericHolder<int>.JniTypeName]    = typeof (GenericHolder<>),
 			[RenameClassBase.JniTypeName]       = typeof (RenameClassBase),
 			[RenameClassDerived.JniTypeName]    = typeof (RenameClassDerived),
+			[AnotherJavaInterfaceImpl.JniTypeName]          = typeof (AnotherJavaInterfaceImpl),
 			[CallVirtualFromConstructorBase.JniTypeName]    = typeof (CallVirtualFromConstructorBase),
 			[CallVirtualFromConstructorDerived.JniTypeName] = typeof (CallVirtualFromConstructorDerived),
 			[CrossReferenceBridge.JniTypeName]              = typeof (CrossReferenceBridge),

tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs

@@ -133,6 +133,30 @@ public void CollectPeers ()
 			// TODO
 		}
 
+		[Test]
+		public void CreatePeer_InvalidHandleReturnsNull ()
+		{
+			var r = new JniObjectReference ();
+			var o = valueManager.CreatePeer (ref r, JniObjectReferenceOptions.Copy, null);
+			Assert.IsNull (o);
+		}
+
+		[Test]
+		public unsafe void CreatePeer_UsesFallbackType ()
+		{
+			using var t = new JniType (AnotherJavaInterfaceImpl.JniTypeName);
+
+			var ctor    = t.GetConstructor ("()V");
+			var lref    = t.NewObject (ctor, null);
+
+			using var p = valueManager.CreatePeer (ref lref, JniObjectReferenceOptions.CopyAndDispose, typeof (IJavaInterface));
+
+			Assert.IsFalse (lref.IsValid);  // .CopyAndDispose disposes
+
+			Assert.IsNotNull (p);
+			Assert.AreSame (typeof (IJavaInterfaceInvoker), p!.GetType ());
+		}
+
 		[Test]
 		public void CreateValue ()
 		{
@@ -294,4 +318,38 @@ public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniVa
 		protected override Type ValueManagerType => ManagedValueManagerType;
 	}
 #endif  // !__ANDROID__
+
+	// Note: Java side implements JavaInterface, while managed binding DOES NOT.
+	// This is so that `CreatePeer(…, typeof(IJavaInterface))` tests don't use an existing AnotherJavaInterfaceImpl instance.
+	//
+	// This is mostly identical to MyJavaInterfaceImpl; the important difference is that
+	// it contains an activation constructor, while MyJavaInterfaceImpl does not.
+	// MyJavaInterfaceImpl can't have one, as that's what provokes the NotSupportedException in the JavaAs() tests.
+	//
+	// We want one here so that in "bad" `CreatePeer()` implementations, we'll find this peer and construct it
+	// before verifying that it satisfies the targetType requirement.
+	[JniTypeSignature (JniTypeName, GenerateJavaPeer=false)]
+	public class AnotherJavaInterfaceImpl : JavaObject {
+		internal            const       string          JniTypeName    = "net/dot/jni/test/AnotherJavaInterfaceImpl";
+
+		internal    static  readonly    JniPeerMembers  _members    = new JniPeerMembers (JniTypeName, typeof (AnotherJavaInterfaceImpl));
+
+		public override JniPeerMembers JniPeerMembers {
+			get {return _members;}
+		}
+
+		AnotherJavaInterfaceImpl (ref JniObjectReference reference, JniObjectReferenceOptions options)
+			: base (ref reference, options)
+		{
+		}
+
+		public unsafe AnotherJavaInterfaceImpl ()
+			: base (ref *InvalidJniObjectReference, JniObjectReferenceOptions.None)
+		{
+			const   string  id  = "()V";
+			var peer = _members.InstanceMethods.StartCreateInstance (id, GetType (), null);
+			Construct (ref peer, JniObjectReferenceOptions.CopyAndDispose);
+			_members.InstanceMethods.FinishCreateInstance (id, this, null);
+		}
+	}
 }

tests/Java.Interop-Tests/java/net/dot/jni/test/AnotherJavaInterfaceImpl.java

@@ -0,0 +1,13 @@
+package net.dot.jni.test;
+
+public class AnotherJavaInterfaceImpl
+	implements JavaInterface, Cloneable
+{
+	public String getValue() {
+		return "Another hello from Java!";
+	}
+
+	public Object clone() {
+		return this;
+	}
+}