[generator] generator --lang-features=emit-legacy-interface-invokers

Fixes: #910

Context: bc5bcf4
Context: #858

Consider the Java `java.lang.Runnable` interface:

	package java.lang;
	public interface Runnable {
	    void run ();
	}

This is bound as:

	package Java.Lang;
	public interface IRunnable : IJavaPeerable {
	    void Run ();
	}

with some slight differences depending on whether we're dealing with
.NET Android (`generator --codegen-target=xajavainterop1`) or
`src/Java.Base` (`generator --codegen-target=javainterop1`).

Now, assume a Java API + corresponding binding which returns a
`Runnable` instance:

	package example;
	public class Whatever {
	    public static Runnable createRunnable();
	}

You can invoke `IRunnable.Run()` on the return value:

	IRunnable r = Whatever.CreateRunnable();
	r.Run();

but how does that work?

This works via an "interface Invoker", which is a class emitted by
`generator` which implements the interface and invokes the interface
methods through JNI:

	internal partial class IRunnableInvoker : Java.Lang.Object, IRunnable {
	    public void Run() => …
	}

Once Upon A Time™, the interface invoker implementation mirrored that
of classes: a static `IntPtr` field held the `jmethodID` value, which
would be looked up on first-use and cached for subsequent invocations:

	partial class IRunnableInvoker {
	    static IntPtr id_run;
	    public unsafe void Run() {
	        if (id_run == IntPtr.Zero)
	            id_run = JNIEnv.GetMethodID (class_ref, "run", "()V");
	        JNIEnv.CallVoidMethod (Handle, id_run, …);
	    }
	}

This approach works until you have interface inheritance and methods
which come from different interfaces:

	package android.view;
	public /* partial */ interface ViewManager {
	    void addView(View view, ViewGroup.LayoutParams params);
	}
	public /* partial */ interface WindowManager extends ViewManager {
	    void removeViewImmediate(View view);
	}

This would be bound as:

	namespace Android.Views;
	public partial interface IViewManager : IJavaPeerable {
	    void AddView (View view, ViewGroup.LayoutParams @params);
	}
	public partial IWindowManager : IViewManager {
	    void RemoveViewImmediate (View view);
	}
	internal partial class IWindowManagerInvoker : Java.Lang.Object, IWindowManager {
	    static IntPtr id_addView;
	    public void AddView(View view, ViewGroup.LayoutParams @params)
	    {
	        if (id_addView == IntPtr.Zero)
	            id_run = JNIEnv.GetMethodID (class_ref, "addView", "…");
	        JNIEnv.CallVoidMethod (Handle, id_addView, …);
	    }
	}

Unfortunately, *invoking* `IViewManager.AddView()` through an
`IWindowManagerInvoker` would crash!

	D/dalvikvm( 6645): GetMethodID: method not found: Landroid/view/WindowManager;.addView:(Landroid/view/View;Landroid/view/ViewGroup$LayoutParams;)V
	I/MonoDroid( 6645): UNHANDLED EXCEPTION: Java.Lang.NoSuchMethodError: Exception of type 'Java.Lang.NoSuchMethodError' was thrown.
	I/MonoDroid( 6645): at Android.Runtime.JNIEnv.GetMethodID (intptr,string,string)
	I/MonoDroid( 6645): at Android.Views.IWindowManagerInvoker.AddView (Android.Views.View,Android.Views.ViewGroup/LayoutParams)
	I/MonoDroid( 6645): at Mono.Samples.Hello.HelloActivity.OnCreate (Android.OS.Bundle)
	I/MonoDroid( 6645): at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (intptr,intptr,intptr)
	I/MonoDroid( 6645): at (wrapper dynamic-method) object.ecadbe0b-9124-445e-a498-f351075f6c89 (intptr,intptr,intptr)

Interfaces are not classes, and this is one of the places that this
is most apparent.  Because of this crash, we had to use *instance*
`jmethodID` caches:

	internal partial class IWindowManagerInvoker : Java.Lang.Object, IWindowManager {
	    IntPtr id_addView;
	    public void AddView(View view, ViewGroup.LayoutParams @params)
	    {
	        if (id_addView == IntPtr.Zero)
	            id_run = JNIEnv.GetMethodID (class_ref, "addView", "…");
	        JNIEnv.CallVoidMethod (Handle, id_addView, …);
	    }
	}

Pro: no more crash!

Con: *every different instance* of `IWindowManagerInvoker` needs to
separately lookup whatever methods are required.  There is *some*
caching, so repeated calls to `AddView()` on the same instance will
hit the cache, but if you obtain a different `IWindowManager`
instance, `jmethodID` values will need to be looked up again.

This was "fine", until #858 enters the picture:
interface invokers were full of Android-isms --
`Android.Runtime.JNIEnv.GetMethodID()`! -- and thus ***not*** APIs
that @jonpryor wished to expose within desktop Java.Base bindings.

Enter `generator --lang-features=emit-legacy-interface-invokers`:
when *not* specified, interface invokers will now use
`JniPeerMembers` for method lookup and invocation, allowing
`jmethodID` values to be cached *across* instances.  In order to
prevent the runtime crash, an interface may have *multiple*
`JniPeerMembers` values, one per inherited interface, which is used
to invoke methods from that interface.

`IWindowManagerInvoker` now becomes:

	internal partial class IWindowManagerInvoker : Java.Lang.Object, IWindowManager {
	    static readonly JniPeerMembers _members_ViewManager = …;
	    static readonly JniPeerMembers _members_WindowManager = …;

	    public void AddView(View view, ViewGroup.LayoutParams @params)
	    {
	        const string __id = "addView.…";
	        _members_ViewManager.InstanceMethods.InvokeAbstractVoidMethod (__id, this, …);
	    }

	    public void RemoveViewImmediate(View view)
	    {
	        const string __id = "removeViewImmediate.…";
	        _members_WindowManager.InstanceMethods.InvokeAbstractVoidMethod (__id, this, …);
	    }
	}

This has two advantages:

 1. More caching!
 2. Desktop `Java.Base` binding can now have interface invokers.

TODO: Performance?  How much better is this?
Update tests/invocation-overhead.

Update `tests/generator-Tests` expected output.
Note: to keep this patch small, only JavaInterop1 output uses the
new pattern.  XAJavaInterop1 output is unchanged, *even though*
it will use the new output *by default*.

Added [CS0114][0] to `$(NoWarn)` in `Java.Base.csproj` to ignore
warnings such as:

	…/src/Java.Base/obj/Debug-net7.0/mcw/Java.Lang.ICharSequence.cs(195,25): warning CS0114:
	'ICharSequenceInvoker.ToString()' hides inherited member 'Object.ToString()'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Ignoring CS0114 is also done in `Mono.Android.dll` as well, so this
is not a new or unique requirement.

Update `Java.Interop.dll` so that
`JniRuntime.JniValueManager.GetActivationConstructor()` now knows
about and looks for `*Invoker` types, then uses the activation
constructor from the `*Invoker` type when the source type is an
abstract `class` or `interface`.

Update `tests/Java.Base-Tests` to test for implicit `*Invoker` lookup
and invocation support.

[0]: https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0114

src/Java.Base/Java.Base.csproj

@@ -4,7 +4,7 @@
     <TargetFramework>$(DotNetTargetFramework)</TargetFramework>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <Nullable>enable</Nullable>
-    <NoWarn>$(NoWarn);8764</NoWarn>
+    <NoWarn>$(NoWarn);8764;0114</NoWarn>
   </PropertyGroup>
 
   <Import Project="..\..\TargetFrameworkDependentValues.props" />

src/Java.Base/Java.Lang/ICharSequence.cs

@@ -1,5 +1,10 @@
+using System.Collections;
+
 namespace Java.Lang {
 
+	partial class ICharSequenceInvoker : IEnumerable {
+	}
+
 	public static partial class ICharSequenceExtensions {
 
 		public static ICharSequence[]? ToCharSequenceArray (this string?[]? values)

src/Java.Base/Transforms/Metadata.xml

@@ -1,7 +1,10 @@
 <metadata>
-  <!-- For now, just bind java.lang.* -->
-  <remove-node path="//api/package[not(starts-with(@name, 'java.lang')
-      or starts-with(@name, 'java.io')
+  <!-- For now, just bind a few packages  -->
+  <remove-node path="//api/package[
+      not(
+        starts-with(@name, 'java.lang')
+        or starts-with(@name, 'java.io')
+        or starts-with(@name, 'java.util.function')
       )]" />
 
   <!-- Type / Namespace conflicts -->

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

@@ -335,14 +335,33 @@ static Type GetPeerType (Type type)
 
 			static ConstructorInfo? GetActivationConstructor (Type type)
 			{
-				return
-					(from c in type.GetConstructors (BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
-					 let p = c.GetParameters ()
-					 where p.Length == 2 && p [0].ParameterType == ByRefJniObjectReference && p [1].ParameterType == typeof (JniObjectReferenceOptions)
-					 select c)
-				.FirstOrDefault ();
+				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;
 			}
 
+			static Type? GetInvokerType (Type type)
+			{
+				const string suffix = "Invoker";
+				Type[] arguments = type.GetGenericArguments ();
+				if (arguments.Length == 0)
+					return type.Assembly.GetType (type + suffix);
+				Type definition = type.GetGenericTypeDefinition ();
+				int bt = definition.FullName!.IndexOf ("`", StringComparison.Ordinal);
+				if (bt == -1)
+					throw new NotSupportedException ("Generic type doesn't follow generic type naming convention! " + type.FullName);
+				Type? suffixDefinition = definition.Assembly.GetType (
+						definition.FullName.Substring (0, bt) + suffix + definition.FullName.Substring (bt));
+				if (suffixDefinition == null)
+					return null;
+				return suffixDefinition.MakeGenericType (arguments);
+			}
 
 			public object? CreateValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType = null)
 			{

tests/Java.Base-Tests/Java.Base/JavaToManagedTests.cs

@@ -18,6 +18,18 @@ public void InterfaceMethod ()
 			Assert.IsTrue (invoked);
 			r.Dispose ();
 		}
+
+		[Test]
+		public void InterfaceInvokerMethod ()
+		{
+			int value   = 0;
+			using var c = new MyIntConsumer (v => value = v);
+			using var r = JavaInvoker.CreateRunnable (c);
+			r.Run ();
+			Assert.AreEqual (0, value);
+			r.Run ();
+			Assert.AreEqual (1, value);
+		}
 	}
 
 	class JavaInvoker : JavaObject {
@@ -31,6 +43,14 @@ public static unsafe void Run (Java.Lang.IRunnable r)
 			args [0] = new JniArgumentValue (r);
 			_members.StaticMethods.InvokeVoidMethod ("run.(Ljava/lang/Runnable;)V", args);
 		}
+
+		public static unsafe Java.Lang.IRunnable? CreateRunnable (Java.Util.Function.IIntConsumer c)
+		{
+			JniArgumentValue* args = stackalloc JniArgumentValue [1];
+			args [0] = new JniArgumentValue (c);
+			var _rm  = _members.StaticMethods.InvokeObjectMethod ("createRunnable.(Ljava/util/function/IntConsumer;)Ljava/lang/Runnable;", args);
+			return Java.Interop.JniEnvironment.Runtime.ValueManager.GetValue<Java.Lang.IRunnable> (ref _rm, JniObjectReferenceOptions.CopyAndDispose);
+		}
 	}
 
 	[JniTypeSignature ("example/MyRunnable")]
@@ -48,4 +68,20 @@ public void Run ()
 			action ();
 		}
 	}
+
+	[JniTypeSignature ("example/MyIntConsumer")]
+	class MyIntConsumer : Java.Lang.Object, Java.Util.Function.IIntConsumer {
+
+		Action<int> action;
+
+		public MyIntConsumer (Action<int> action)
+		{
+			this.action = action;
+		}
+
+		public void Accept (int value)
+		{
+			action (value);
+		}
+	}
 }

tests/Java.Base-Tests/java/com/microsoft/java_base_tests/Invoker.java

@@ -1,8 +1,19 @@
 package com.microsoft.java_base_tests;
 
-public class Invoker {
+import java.util.function.IntConsumer;
+
+public final class Invoker {
 
 	public static void run(Runnable r) {
 		r.run();
 	}
+
+	public static Runnable createRunnable(final IntConsumer consumer) {
+		return new Runnable() {
+			int value;
+			public void run() {
+				consumer.accept(value++);
+			}
+		};
+	}
 }

tests/generator-Tests/Integration-Tests/BaseGeneratorTest.cs

@@ -143,6 +143,7 @@ protected void Run (CodeGenerationTarget target, string outputPath, string apiDe
 			AdditionalSourceDirectories.Clear ();
 
 			Options.CodeGenerationTarget                        = target;
+			Options.EmitLegacyInterfaceInvokers                 = target == CodeGenerationTarget.XAJavaInterop1;
 			Options.ApiDescriptionFile                          = FullPath (apiDescriptionFile);
 			Options.ManagedCallableWrapperSourceOutputDirectory = FullPath (outputPath);
 

tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteDuplicateInterfaceEventArgs.txt

@@ -11,6 +11,47 @@ public partial interface AnimatorListener : IJavaPeerable {
 
 }
 
+[global::Java.Interop.JniTypeSignature ("java/code/AnimatorListener", GenerateJavaPeer=false)]
+internal partial class AnimatorListenerInvoker : global::Java.Lang.Object, AnimatorListener {
+	[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
+	[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
+	public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+		get { return _members_AnimatorListener; }
+	}
+
+	static readonly JniPeerMembers _members_AnimatorListener = new JniPeerMembers ("java/code/AnimatorListener", typeof (AnimatorListenerInvoker));
+
+	public AnimatorListenerInvoker (ref JniObjectReference reference, JniObjectReferenceOptions options) : base (ref reference, options)
+	{
+	}
+
+	public unsafe bool OnAnimationEnd (int param1)
+	{
+		const string __id = "OnAnimationEnd.(I)Z";
+		try {
+			JniArgumentValue* __args = stackalloc JniArgumentValue [1];
+			__args [0] = new JniArgumentValue (param1);
+			var __rm = _members_AnimatorListener.InstanceMethods.InvokeAbstractBooleanMethod (__id, this, __args);
+			return __rm;
+		} finally {
+		}
+	}
+
+	public unsafe bool OnAnimationEnd (int param1, int param2)
+	{
+		const string __id = "OnAnimationEnd.(II)Z";
+		try {
+			JniArgumentValue* __args = stackalloc JniArgumentValue [2];
+			__args [0] = new JniArgumentValue (param1);
+			__args [1] = new JniArgumentValue (param2);
+			var __rm = _members_AnimatorListener.InstanceMethods.InvokeAbstractBooleanMethod (__id, this, __args);
+			return __rm;
+		} finally {
+		}
+	}
+
+}
+
 // event args for java.code.AnimatorListener.OnAnimationEnd
 public partial class AnimationEndEventArgs : global::System.EventArgs {
 	bool handled;

tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterface.txt

@@ -79,3 +79,114 @@ public partial interface IMyInterface : IJavaPeerable {
 	}
 
 }
+
+[global::Java.Interop.JniTypeSignature ("java/code/IMyInterface", GenerateJavaPeer=false)]
+internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterface {
+	[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
+	[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
+	public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+		get { return _members_IMyInterface; }
+	}
+
+	static readonly JniPeerMembers _members_IMyInterface = new JniPeerMembers ("java/code/IMyInterface", typeof (IMyInterfaceInvoker));
+
+	public IMyInterfaceInvoker (ref JniObjectReference reference, JniObjectReferenceOptions options) : base (ref reference, options)
+	{
+	}
+
+	public unsafe int Count {
+		get {
+			const string __id = "get_Count.()I";
+			try {
+				var __rm = _members_IMyInterface.InstanceMethods.InvokeVirtualInt32Method (__id, this, null);
+				return __rm;
+			} finally {
+			}
+		}
+		set {
+			const string __id = "set_Count.(I)V";
+			try {
+				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
+				__args [0] = new JniArgumentValue (value);
+				_members_IMyInterface.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
+			} finally {
+			}
+		}
+	}
+
+	public unsafe string? Key {
+		get {
+			const string __id = "get_Key.()Ljava/lang/String;";
+			try {
+				var __rm = _members_IMyInterface.InstanceMethods.InvokeVirtualObjectMethod (__id, this, null);
+				return global::Java.Interop.JniEnvironment.Strings.ToString (ref __rm, JniObjectReferenceOptions.CopyAndDispose);
+			} finally {
+			}
+		}
+		set {
+			const string __id = "set_Key.(Ljava/lang/String;)V";
+			var native_value = global::Java.Interop.JniEnvironment.Strings.NewString (value);
+			try {
+				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
+				__args [0] = new JniArgumentValue (native_value);
+				_members_IMyInterface.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
+			} finally {
+				global::Java.Interop.JniObjectReference.Dispose (ref native_value);
+			}
+		}
+	}
+
+	public unsafe int AbstractCount {
+		get {
+			const string __id = "get_AbstractCount.()I";
+			try {
+				var __rm = _members_IMyInterface.InstanceMethods.InvokeAbstractInt32Method (__id, this, null);
+				return __rm;
+			} finally {
+			}
+		}
+		set {
+			const string __id = "set_AbstractCount.(I)V";
+			try {
+				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
+				__args [0] = new JniArgumentValue (value);
+				_members_IMyInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, __args);
+			} finally {
+			}
+		}
+	}
+
+	public unsafe int GetCountForKey (string? key)
+	{
+		const string __id = "GetCountForKey.(Ljava/lang/String;)I";
+		var native_key = global::Java.Interop.JniEnvironment.Strings.NewString (key);
+		try {
+			JniArgumentValue* __args = stackalloc JniArgumentValue [1];
+			__args [0] = new JniArgumentValue (native_key);
+			var __rm = _members_IMyInterface.InstanceMethods.InvokeVirtualInt32Method (__id, this, __args);
+			return __rm;
+		} finally {
+			global::Java.Interop.JniObjectReference.Dispose (ref native_key);
+		}
+	}
+
+	public unsafe string? Key ()
+	{
+		const string __id = "Key.()Ljava/lang/String;";
+		try {
+			var __rm = _members_IMyInterface.InstanceMethods.InvokeVirtualObjectMethod (__id, this, null);
+			return global::Java.Interop.JniEnvironment.Strings.ToString (ref __rm, JniObjectReferenceOptions.CopyAndDispose);
+		} finally {
+		}
+	}
+
+	public unsafe void AbstractMethod ()
+	{
+		const string __id = "AbstractMethod.()V";
+		try {
+			_members_IMyInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null);
+		} finally {
+		}
+	}
+
+}

tests/generator-Tests/Unit-Tests/CodeGeneratorTestBase.cs

@@ -37,6 +37,7 @@ protected virtual CodeGenerationOptions CreateOptions ()
 		{
 			return new CodeGenerationOptions {
 				CodeGenerationTarget = Target,
+				EmitLegacyInterfaceInvokers = Target == Xamarin.Android.Binder.CodeGenerationTarget.XAJavaInterop1,
 			};
 		}
 

tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs

@@ -1193,6 +1193,7 @@ protected override CodeGenerationOptions CreateOptions ()
 				SupportInterfaceConstants       = true,
 				SupportNestedInterfaceTypes     = true,
 				SupportNullableReferenceTypes   = true,
+				EmitLegacyInterfaceInvokers     = Target == CodeGenerationTarget.XAJavaInterop1,
 			};
 			return options;
 		}

tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs

@@ -16,4 +16,40 @@ public partial interface IExtendedInterface : IJavaPeerable {
 		void BaseMethod ();
 
 	}
+
+	[global::Java.Interop.JniTypeSignature ("xamarin/test/ExtendedInterface", GenerateJavaPeer=false)]
+	internal partial class IExtendedInterfaceInvoker : global::Java.Lang.Object, IExtendedInterface {
+		[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
+		[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
+		public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+			get { return _members_IExtendedInterface; }
+		}
+
+		static readonly JniPeerMembers _members_IExtendedInterface = new JniPeerMembers ("xamarin/test/ExtendedInterface", typeof (IExtendedInterfaceInvoker));
+
+		static readonly JniPeerMembers _members_IBaseInterface = new JniPeerMembers ("xamarin/test/BaseInterface", typeof (IExtendedInterfaceInvoker));
+
+		public IExtendedInterfaceInvoker (ref JniObjectReference reference, JniObjectReferenceOptions options) : base (ref reference, options)
+		{
+		}
+
+		public unsafe void ExtendedMethod ()
+		{
+			const string __id = "extendedMethod.()V";
+			try {
+				_members_IExtendedInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null);
+			} finally {
+			}
+		}
+
+		public unsafe void BaseMethod ()
+		{
+			const string __id = "baseMethod.()V";
+			try {
+				_members_IBaseInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null);
+			} finally {
+			}
+		}
+
+	}
 }