[Mono.Android] JavaCast<T>() should throw on unsupported types (#9145)

Context: dotnet/java-interop#910
Context: dotnet/java-interop@1adb796
Context: 8928f11
Context: 35f41dc
Context: ab412a5

The `.JavaCast<T>()` extension method is for checking type casts
across the JNI boundary.  As such, it can be expected to fail.
Failure is indicated by throwing an exception.

	var n = new Java.Lang.Integer(42);
	var a = n.JavaCast<Java.Lang.IAppendable>();    // this should throw, right?

The last line should throw because, at this point anyway,
[`java.lang.Integer`][0] doesn't implement the
[`java.lang.Appendable`][1] interface.

What *actually* happens as of ab412a5 is that
`n.JavaCast<IAppendable>()` ***does not throw***;
instead, it returns `null`!

This is a ***BREAK*** from .NET 8, in which an exception *is* thrown:

	System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
	 ---> System.InvalidCastException: Unable to convert instance of type 'crc64382c01b66da5f22f/MainActivity' to type 'java.lang.Appendable'.
	   at Java.Lang.IAppendableInvoker.Validate(IntPtr handle)
	   at Java.Lang.IAppendableInvoker..ctor(IntPtr handle, JniHandleOwnership transfer)
	   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Constructor(Object obj, IntPtr* args)
	   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
	   --- End of inner exception stack trace ---
	   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
	   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
	   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
	   at System.Reflection.ConstructorInfo.Invoke(Object[] parameters)
	   at Java.Interop.TypeManager.CreateProxy(Type type, IntPtr handle, JniHandleOwnership transfer)
	   at Java.Interop.TypeManager.CreateInstance(IntPtr handle, JniHandleOwnership transfer, Type targetType)
	   at Java.Lang.Object.GetObject(IntPtr handle, JniHandleOwnership transfer, Type type)
	   at Java.Interop.JavaObjectExtensions._JavaCast[IAppendable](IJavaObject instance)

What happened™?!

What happened are (at least) two things:

First is dotnet/java-interop@1adb7964 and 8928f11, which optimized
interface invokers (dotnet/java-interop#910).  Part of this
optimization involved *removing* the `Validate(IntPtr)` method from
the interface invoker classes.  (Was this part of the optimization?
No.  It's just that @jonpryor didn't think that `Validate()` was
needed anymore, and that the checks it performed was already done
elsewhere.  Turns Out™, that wasn't true!)

The result of 8928f11 is that `.JavaCast<T>()` would *always* return
a non-`null` value for the requested type.  Worse, any method
invocations on that value would ***CRASH THE PROCESS***:

	var n = new Java.Lang.Integer(42);
	var a = n.JavaCast<Java.Lang.IAppendable>();    // does NOT throw
	a.Append('a');
	// app dies

`adb logcat` contains:

	JNI DETECTED ERROR IN APPLICATION: can't call java.lang.Appendable java.lang.Appendable.append(char) on instance of java.lang.Integer
	    in call to CallObjectMethodA

Oops.

Secondly, commit 35f41dc actually added the checks that @jonpryor
thought were already there, but as part of these checks
`TypeManager.CreateInstance()` now returns `null` if the Java-side
type checks fail.  This means that `.JavaCast<T>()` now returns
`null`, which prevents the above `JNI DETECTED ERROR IN APPLICATION`
crash (good), but means `.JavaCast<T>()` returns `null` which will
likely result in `NullReferenceException`s if the value is used.

Fix `.JavaCast<T>()` by updating `JavaObjectExtensions.JavaCast<T>()`
to throw an `InvalidCastException` if `TypeManager.CreateInstance()`
returns `null`.

Additionally, massively simplify the `.JavaCast<T>()` code paths by
removing the semantic distinction between interface and class types.
This in turn introduces *new* (intentional!) semantic changes.

Suppose we have an `ArrayList` instance:

	var list          = new Java.Util.ArrayList();

and to ensure "appropriate magic" happens, *alias* `list`:

	var alias         = new Java.Lang.Object(list.Handle, JniHandleOwnership.DoNotTransfer);

We now have two managed instances referring to the same Java instance.
Next, we want to use `alias` as an `AbstractList`:

	var abstractList  = alias.JavaCast<Java.Util.AbstractList>();
	var newAlias      = !object.ReferenceEquals(list, abstractList);

As `java.util.ArrayList` inherits `java.util.AbstractList`, this will
return a non-`null` value.  *However*, what *type and instance* will
`abstractList` be?

In .NET 8 and .NET 9 *prior to this commit*, `newAlias` will be true
and `abstractList` will be an `AbstractListInvoker`, meaning there
will be *three* managed instances referring to the same Java-side
`ArrayList`.

Starting with this commit, `abstractList` be the same value as `list`,
have a runtime type of `ArrayList`, and `newAlias` will be false.

[0]: https://developer.android.com/reference/java/lang/Integer
[1]: https://developer.android.com/reference/java/lang/Appendable

src/Mono.Android/Java.Interop/JavaObjectExtensions.cs

@@ -81,41 +81,9 @@ internal static TResult? _JavaCast<
 			if (instance.Handle == IntPtr.Zero)
 				throw new ObjectDisposedException (instance.GetType ().FullName);
 
-			Type resultType = typeof (TResult);
-			if (resultType.IsClass) {
-				return (TResult) CastClass (instance, resultType);
-			}
-			else if (resultType.IsInterface) {
-				return (TResult?) Java.Lang.Object.GetObject (instance.Handle, JniHandleOwnership.DoNotTransfer, resultType);
-			}
-			else
-				throw new NotSupportedException (FormattableString.Invariant ($"Unable to convert type '{instance.GetType ().FullName}' to '{resultType.FullName}'."));
-		}
-
-		static IJavaObject CastClass (
-				IJavaObject instance,
-				[DynamicallyAccessedMembers (Constructors)]
-				Type resultType)
-		{
-			var klass = JNIEnv.FindClass (resultType);
-			try {
-				if (klass == IntPtr.Zero)
-					throw new ArgumentException ("Unable to determine JNI class for '" + resultType.FullName + "'.", "TResult");
-				if (!JNIEnv.IsInstanceOf (instance.Handle, klass))
-					throw new InvalidCastException (
-							FormattableString.Invariant ($"Unable to convert instance of type '{instance.GetType ().FullName}' to type '{resultType.FullName}'."));
-			} finally {
-				JNIEnv.DeleteGlobalRef (klass);
-			}
-
-			if (resultType.IsAbstract) {
-				// TODO: keep in sync with TypeManager.CreateInstance() algorithm
-				var invokerType = GetInvokerType (resultType);
-				if (invokerType == null)
-					throw new ArgumentException ("Unable to get Invoker for abstract type '" + resultType.FullName + "'.", "TResult");
-				resultType = invokerType;
-			}
-			return (IJavaObject) TypeManager.CreateProxy (resultType, instance.Handle, JniHandleOwnership.DoNotTransfer);
+			return (TResult) Java.Lang.Object.GetObject (instance.Handle, JniHandleOwnership.DoNotTransfer, typeof (TResult)) ??
+				throw new InvalidCastException (
+					FormattableString.Invariant ($"Unable to convert instance of type '{instance.GetType ().FullName}' to type '{typeof (TResult).FullName}'."));
 		}
 
 		internal static IJavaObject? JavaCast (
@@ -132,14 +100,9 @@ static IJavaObject CastClass (
 			if (resultType.IsAssignableFrom (instance.GetType ()))
 				return instance;
 
-			if (resultType.IsClass) {
-				return CastClass (instance, resultType);
-			}
-			else if (resultType.IsInterface) {
-				return (IJavaObject?) Java.Lang.Object.GetObject (instance.Handle, JniHandleOwnership.DoNotTransfer, resultType);
-			}
-			else
-				throw new NotSupportedException (FormattableString.Invariant ($"Unable to convert type '{instance.GetType ().FullName}' to '{resultType.FullName}'."));
+			return (IJavaObject?) Java.Lang.Object.GetObject (instance.Handle, JniHandleOwnership.DoNotTransfer, resultType) ??
+				throw new InvalidCastException (
+					FormattableString.Invariant ($"Unable to convert instance of type '{instance.GetType ().FullName}' to type '{resultType.FullName}'."));
 		}
 
 		// typeof(Foo) -> FooInvoker

src/Mono.Android/Java.Interop/TypeManager.cs

@@ -336,7 +336,6 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership
 
 				handleClass = JniEnvironment.Types.GetObjectClass (new JniObjectReference (handle));
 				if (!JniEnvironment.Types.IsAssignableFrom (handleClass, typeClass)) {
-					Logger.Log (LogLevel.Info, "*jonp*", $"# jonp: can't assign `{GetClassName(handleClass.Handle)}` to `{GetClassName(typeClass.Handle)}`");
 					return null;
 				}
 			} finally {

tests/Mono.Android-Tests/Java.Interop/JavaObjectExtensionsTests.cs

@@ -40,6 +40,32 @@ public void JavaCast_InterfaceCast ()
 			}
 		}
 
+		[Test]
+		public void JavaCast_BadInterfaceCast ()
+		{
+			using var n = new Java.Lang.Integer (42);
+			var e = Assert.Catch<Exception>(() => JavaObjectExtensions.JavaCast<Java.Lang.IAppendable> (n));
+			if (e is System.Reflection.TargetInvocationException tie) {
+				// .NET 8 behavior
+				Assert.IsTrue (tie.InnerException is InvalidCastException);
+			} else if (e is InvalidCastException ice) {
+				// Yay
+			} else {
+				Assert.Fail ($"Unexpected exception type: {e.GetType ()}: {e.ToString ()}");
+			}
+		}
+
+		[Test]
+		public void JavaCast_ObtainOriginalInstance ()
+		{
+			using var list = new Java.Util.ArrayList ();
+			using var copy = new Java.Lang.Object (list.Handle, JniHandleOwnership.DoNotTransfer);
+			using var al   = JavaObjectExtensions.JavaCast<Java.Util.AbstractList> (copy);
+
+			// Semantic change: in .NET 8, `al` is a new `AbstractListInvoker` instance, not `list`
+			Assert.AreSame (list, al);
+		}
+
 		[Test]
 		public void JavaCast_InvalidTypeCastThrows ()
 		{