[Java.Interop] Add JniAddNativeMethodRegistrationAttribute (#206)

Instead of hard-coding the name of a method to register native 
members in a given type, introduce new attribute to mark appropriate
methods within that type.

```csharp
// Old-and-busted:
static void __RegisterNativeMembers (JniType type, string members)
{
	// ...
	type.RegisterNativeMethods (...)
}

// New-Hawtness:
[JniAddNativeMethodRegistrationAttribute]
static void RegisterNativeMembers (JniNativeMethodRegistrationArguments args)
{
	args.AddRegistrations (...);
}

// Supporting cast:
public sealed class JniAddNativeMethodRegistrationAttribute : Attribute {
}
public struct JniNativeMethodRegistrationArguments {
	public JniNativeMethodRegistrationArguments (ICollection<JniNativeMethodRegistration> registrations, string methods);

	public ICollection<JniNativeMethodRegistration> Registrations   {get;}
	public string                                   Methods         {get;}

	public void AddRegistrations (IEnumerable<JniNativeMethodRegistration> registrations);		
}
```

Responsibility for invoking `JniType.RegisterNativeMembers()` has
been *moved* from `__RegisterNativeMembers` into internal code, and
`JniType.RegisterNativeMembers()` has been made `internal` so that
it can't be called externally.

Rationale: The `__RegisterNativeMembers()` method was "too magical":
it's hard to document, but it has a large responsibility in ensuring
that the Java `native` methods on a type are properly registered.

Introducing a new `[JniAddNativeMethodRegistrationAttribute]` custom
attribute simplifies this, as there is now an in-language construct
which can contain suitable documentation.

Additionally, `JNIEnv::RegisterNativeMethods()` can only be invoked
*once* on a given type. (Not *strictly* true -- you *can* call it
more than once for a given `jclass`! -- but the second and later
invocations will *replace* the contents of the original invocation.)
This complicates *documenting* what
`[JniAddNativeMethodRegistrationAttribute]` can do. There are only
two plausible usage scenarios:

 1. A type can contain *only **one*** such method.
 2. A type can contain *multiple* such methods.

Absent a truly compelling reason to go with (1), the only way to
support (2) is to prevent application code from calling
`JniType.RegisterNativeMethods()`. This allows
`JniRuntime.JniTypeManager.TryRegisterNativeMembers()` to invoke
all `[JniAddNativeMethodRegistrationAttribute]` methods and collect
the values, then call `JNIEnv::RegisterNativeMethods()` *once*.

`TestType.cs` has been updated to demonstrate using multiple
`[JniAddNativeMethodRegistrationAttribute]` methods.

Author: radekdoulik

gendarme-ignore.txt

@@ -577,3 +577,7 @@ T: Java.Interop.JavaObject
 R: Gendarme.Rules.Design.Generic.DoNotExposeGenericListsRule
 # We don't care here as we don't hold the list and create it on request. So to avoid performace penalty, we keep the list as return type
 M: System.Collections.Generic.List`1<Java.Interop.JniSurfacedPeerInfo> Java.Interop.JniRuntime/JniValueManager::GetSurfacedPeers()
+
+R: Gendarme.Rules.Concurrency.DoNotUseLockedRegionOutsideMethodRule
+# Looks like Gendarme issue, as there are both Monitor.TryEnter and Monitor.Exit used in this method
+M: System.Boolean Java.Interop.JniRuntime/JniTypeManager::TryRegisterNativeMembers(Java.Interop.JniType,System.Type,System.String)

src/Java.Interop.Export/Tests/Java.Interop/ExportTest.cs

@@ -1,7 +1,7 @@
 using System;
-using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using System.Collections.Generic;
 
 using Java.Interop;
 using Java.Interop.Expressions;
@@ -11,11 +11,10 @@ namespace Java.InteropTests
 	[JniTypeSignature ("com/xamarin/interop/export/ExportType")]
 	public class ExportTest : JavaObject
 	{
-		static void __RegisterNativeMembers (JniType type, string members)
+		[JniAddNativeMethodRegistrationAttribute]
+		static void RegisterNativeMembers (JniNativeMethodRegistrationArguments args)
 		{
-			var methods = JniEnvironment.Runtime.MarshalMemberBuilder
-				.GetExportedMemberRegistrations (typeof (ExportTest));
-			type.RegisterNativeMethods (methods.ToArray ());
+			args.AddRegistrations (JniEnvironment.Runtime.MarshalMemberBuilder.GetExportedMemberRegistrations (typeof (ExportTest)));
 		}
 
 		public ExportTest (ref JniObjectReference reference, JniObjectReferenceOptions transfer)

src/Java.Interop/Java.Interop.csproj

@@ -101,6 +101,8 @@
       <DependentUpon>JniPeerMembers.JniFields.tt</DependentUpon>
     </Compile>
     <Compile Include="Java.Interop\JniPeerMembers.JniStaticFields.cs" />
+    <Compile Include="Java.Interop\JniAddNativeMethodRegistrationAttribute.cs" />
+    <Compile Include="Java.Interop\JniNativeMethodRegistrationArguments.cs" />
   </ItemGroup>
   <ItemGroup Condition=" !$(Configuration.StartsWith('XAIntegration')) ">
     <Compile Include="Java.Interop\JavaArray.cs" />

src/Java.Interop/Java.Interop/JavaProxyObject.cs

@@ -11,12 +11,12 @@ sealed class JavaProxyObject : JavaObject
 		static  readonly    JniPeerMembers                                  _members        = new JniPeerMembers (JniTypeName, typeof (JavaProxyObject));
 		static  readonly    ConditionalWeakTable<object, JavaProxyObject>   CachedValues    = new ConditionalWeakTable<object, JavaProxyObject> ();
 
-		static void __RegisterNativeMembers (JniType type, string members)
+		[JniAddNativeMethodRegistrationAttribute]
+		static void RegisterNativeMembers (JniNativeMethodRegistrationArguments args)
 		{
-			_members.JniPeerType.RegisterNativeMethods (
-					new JniNativeMethodRegistration ("equals",      "(Ljava/lang/Object;)Z",    (Func<IntPtr, IntPtr, IntPtr, bool>)    _Equals),
-					new JniNativeMethodRegistration ("hashCode",    "()I",                      (Func<IntPtr, IntPtr, int>)             _GetHashCode),
-					new JniNativeMethodRegistration ("toString",    "()Ljava/lang/String;",     (Func<IntPtr, IntPtr, IntPtr>)          _ToString));
+			args.Registrations.Add (new JniNativeMethodRegistration ("equals",   "(Ljava/lang/Object;)Z", (Func<IntPtr, IntPtr, IntPtr, bool>)_Equals));
+			args.Registrations.Add (new JniNativeMethodRegistration ("hashCode", "()I",                   (Func<IntPtr, IntPtr, int>)_GetHashCode));
+			args.Registrations.Add (new JniNativeMethodRegistration ("toString", "()Ljava/lang/String;",  (Func<IntPtr, IntPtr, IntPtr>)_ToString));
 		}
 
 		public override JniPeerMembers JniPeerMembers {

src/Java.Interop/Java.Interop/JavaProxyThrowable.cs

@@ -7,10 +7,6 @@ sealed class JavaProxyThrowable : JavaException
 	{
 		new internal    const   string  JniTypeName = "com/xamarin/java_interop/internal/JavaProxyThrowable";
 
-		static void __RegisterNativeMembers (JniType type, string members)
-		{
-		}
-
 		public  Exception   Exception {get; private set;}
 
 		public JavaProxyThrowable (Exception exception)

src/Java.Interop/Java.Interop/JniAddNativeMethodRegistrationAttribute.cs

@@ -0,0 +1,9 @@
+using System;
+
+namespace Java.Interop
+{
+	[AttributeUsage (AttributeTargets.Method)]
+	public sealed class JniAddNativeMethodRegistrationAttribute : Attribute
+	{
+	}
+}

src/Java.Interop/Java.Interop/JniNativeMethodRegistrationArguments.cs

@@ -0,0 +1,33 @@
+using System;
+using System.Collections.Generic;
+
+namespace Java.Interop
+{
+	public struct JniNativeMethodRegistrationArguments
+	{
+		public ICollection<JniNativeMethodRegistration> Registrations {
+			get { return _registrations; }
+		}
+		public string Methods { get; }
+		ICollection<JniNativeMethodRegistration> _registrations;
+
+		public JniNativeMethodRegistrationArguments (ICollection<JniNativeMethodRegistration> registrations, string methods)
+		{
+			_registrations = registrations ?? throw new ArgumentNullException (nameof (registrations));
+			Methods = methods ?? throw new ArgumentNullException (nameof (methods));
+		}
+
+		public void AddRegistrations (IEnumerable<JniNativeMethodRegistration> registrations)
+		{
+			if (_registrations == null)
+				throw new InvalidOperationException ($"{nameof(JniNativeMethodRegistrationArguments)} state is invalid. Please use constructor with parameters.");
+
+			if (registrations is List<JniNativeMethodRegistration> list) {
+				list.AddRange (registrations);
+			} else {
+				foreach (var registration in registrations)
+					_registrations.Add (registration);
+			}
+		}
+	}
+}

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

@@ -3,6 +3,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
+using System.Threading;
 
 namespace Java.Interop {
 
@@ -243,15 +244,41 @@ static Assembly TryLoadAssembly (AssemblyName name)
 				}
 			}
 
+			static List<JniNativeMethodRegistration> sharedRegistrations = new List<JniNativeMethodRegistration> ();
+
 			static bool TryRegisterNativeMembers (JniType nativeClass, Type marshalType, string methods)
 			{
-				var registerMethod  = marshalType.GetTypeInfo ().GetDeclaredMethod ("__RegisterNativeMembers");
-				if (registerMethod == null) {
-					return false;
+				bool lockTaken = false;
+
+				try {
+					Monitor.TryEnter (sharedRegistrations, ref lockTaken);
+					List<JniNativeMethodRegistration> registrations;
+					if (lockTaken) {
+						sharedRegistrations.Clear ();
+						registrations = sharedRegistrations;
+					} else {
+						registrations = new List<JniNativeMethodRegistration> ();
+					}
+					JniNativeMethodRegistrationArguments arguments = new JniNativeMethodRegistrationArguments (registrations, methods);
+					foreach (var methodInfo in marshalType.GetRuntimeMethods ()) {
+						if (methodInfo.GetCustomAttribute (typeof (JniAddNativeMethodRegistrationAttribute)) == null) {
+							continue;
+						}
+
+						if ((methodInfo.Attributes & MethodAttributes.Static) != MethodAttributes.Static) {
+							throw new InvalidOperationException ($"The method {methodInfo} marked with {nameof (JniAddNativeMethodRegistrationAttribute)} must be static");
+						}
+
+						var register = (Action<JniNativeMethodRegistrationArguments>)methodInfo.CreateDelegate (typeof (Action<JniNativeMethodRegistrationArguments>));
+						register (arguments);
+					}
+					nativeClass.RegisterNativeMethods (registrations.ToArray ());
+				} finally {
+					if (lockTaken) {
+						Monitor.Exit (sharedRegistrations);
+					}
 				}
 
-				var register    = (Action<JniType, string>) registerMethod.CreateDelegate (typeof(Action<JniType, string>));
-				register (nativeClass, methods);
 				return true;
 			}
 		}

src/Java.Interop/Java.Interop/JniType.cs

@@ -135,7 +135,7 @@ public bool IsInstanceOfType (JniObjectReference value)
 		JniNativeMethodRegistration[] methods;
 #pragma warning restore 0414
 
-		public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods)
+		internal void RegisterNativeMethods (params JniNativeMethodRegistration[] methods)
 		{
 			AssertValid ();
 

src/Java.Interop/Properties/AssemblyInfo.cs

@@ -27,3 +27,10 @@
 	"814f144e5d817efc4c6502cc012df310783348304e3ae38573c6d658c234025821fda87a0be8a0" +
 	"d504df564e2c93b2b878925f42503e9d54dfef9f9586d9e6f38a305769587b1de01f6c0410328b" +
 	"2c9733db")]
+[assembly: InternalsVisibleTo (
+	"Java.Interop-Tests, PublicKey=" +
+	"0024000004800000940000000602000000240000525341310004000011000000438ac2a5acfbf1" +
+	"6cbd2b2b47a62762f273df9cb2795ceccdf77d10bf508e69e7a362ea7a45455bbf3ac955e1f2e2" +
+	"814f144e5d817efc4c6502cc012df310783348304e3ae38573c6d658c234025821fda87a0be8a0" +
+	"d504df564e2c93b2b878925f42503e9d54dfef9f9586d9e6f38a305769587b1de01f6c0410328b" +
+	"2c9733db")]