[Java.Interop] Replace JniMarshalInfo with JniValueMarshaler.

Context: dotnet#2
Context: dotnet#8

From Issue dotnet#8:

> I don't like JniMarshalInfo; it's too convoluted, makes the
> simple cases hard.

What do we mean by "hard"?  We mean that it's use is error prone,
because what you need to do depends on the values of the
JniMarshalInfo members, which is best characterized by what
JniArgumentMarshalInfo had to do to create a JNI Local refererence:

	JniObjectReference lref;

	var marshaler = JniRuntime.CurrentRuntime.ValueManager.GetJniMarshalInfoForType (type);
	if (info.CreateMarshalCollection != null) {
		var obj = info.CreateMarshalCollection (value);
		lref    = obj.PeerReference;
	} else if (info.CreateLocalRef != null) {
		lref    = info.CreateLocalRef (value);
	} else
		throw new NotSupportedException ("Don't know how to get a JNI Local Reference!");

	// can now use `lref`...

Need to pass as a method argument? Similar-yet-different.  Then
there's the cleanup code!

The eventual intention is for tools/jnimarshalmethod-gen to
post-process assemblies and generate Java > Managed marshal methods
for all resolvable methods, and do this "dynamically" based on the
actual types involved.  This will allow "fixing" binding assemblies
when involved types change, because binding assemblies won't be as
"static" as they are in Xamarin.Android.  (Which is why Issue dotnet#8
mentions using Expressions for the marshaling.)

However, before we can get there we first need a rational marshaling
abstrasction, something that is "pluggable" and readily extensible, so
taht we can e.g. have a custom attribute to control which marshaler to
use on types, parameters, and method return types.  JniMarshalInfo
wasn't that abstraction, and couldn't *be* that abstraction.

To replace JniMarshalInfo, introduce JniValueMarshaler and
JniValueMarshler<T>:

	public abstract partial class JniValueMarshaler {
		public  abstract    object                  CreateValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType = null);
		public  virtual     JniValueMarshalerState  CreateArgumentState (object value, ParameterAttributes synchronize = 0);
		public  abstract    JniValueMarshalerState  CreateObjectReferenceArgumentState (object value, ParameterAttributes synchronize = 0);
		public  abstract    void                    DestroyArgumentState (object value, ref JniValueMarshalerState state, ParameterAttributes synchronize = 0);
	}

	public abstract partial class JniValueMarshaler<T> : JniValueMarshaler {
		public  abstract    T                       CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType = null);
		public  virtual     JniValueMarshalerState  CreateGenericArgumentState (T value, ParameterAttributes synchronize = 0);
		public  abstract    JniValueMarshalerState  CreateGenericObjectReferenceArgumentState (T value, ParameterAttributes synchronize = 0);
		public  abstract    void                    DestroyGenericArgumentState (T value, ref JniValueMarshalerState state, ParameterAttributes synchronize = 0);

		public override object                  CreateValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType = null);
		public override JniValueMarshalerState  CreateArgumentState (object value, ParameterAttributes synchronize = 0);
		public override JniValueMarshalerState  CreateObjectReferenceArgumentState (object value, ParameterAttributes synchronize = 0);
		public override void                    DestroyArgumentState (object value, ref JniValueMarshalerState state, ParameterAttributes synchronize = 0);
	}

The intention is that a custom marshaler can be introduced by
inheriting from JniValueMarshaler<T> and overriding three methods.
This also provides a reasonable abstraction, e.g. to create a
JNI Local Reference:

	var state = marshaler.CreateGenericObjectReferenceArgumentState (value);
	// state.ReferenceValue contains a JNI Object Reference
	// use, then cleanup:
	marshaler.DestroyGenericArgumentState (value, ref state);

The methods are as follows:

  * CreateValue, CreateGenericValue:
    Java > Managed marshaler.
    Given a JniObjectReference, marshal the value to a type compatible
    with specified targetType, which should be either null or a class
    assignment compatible to T.

The other methods use a JniValueMarshalerState type:

	public partial struct JniValueMarshalerState {
		public JniArgumentValue         JniArgumentValue {get;}
		public JniObjectReference       ReferenceValue   {get;}
		public IJavaPeerable            PeerableValue    {get;}
	}

The remaining methods of import are:

  * CreateArgumentState(), CreateGenericArgumentState():
    Managed > Java marshaler.
    Used when the intention is to use
    JniValueMarshalerState.JniArgumentValue, which in turn is for use
    with JniEnvironment.*Methods.Call*Method().

  * CreateObjectReferenceArgumentState(),
    CreateGenericObjectReferenceArgumentState():
    Managed > Java marshaler.
    Used when the intention is to use
    JniValueMarshalerState.ReferenceValue.

  * DestroyArgumentState(), DestroyGenericArgumentState(): Generic
    resource cleanup mechanism for use with Create*ArgumentState().

The primary difference between Create*ArgumentState() and
Create*ObjectReferenceArgumentState() is with builtin value types,
e.g. `int`. Create*ArgumentState() will *only* provide a
JniArgumentValue, while JniValueMarshalerState.ReferenceValue will be
null. Create*ObjectReferenceArgumentState(), meanwhile, will "box" the
value type, providing a valid JniValueMarshalerState.ReferenceValue.
For all other types, these methods are identical.

JniValueMarshaler and JniValueMarshalerState together combine to
replace JniMarshalInfo and JniArgumentMarshalInfo, the latter of which
(1) wasn't public, and (2) was "duplicated" between the generic
marshalers and Java.Interop.Dynamic.  JniValueMarshaler thus allows
cleaning up this duplication.

Along with the new JniValueMarshaler abstraction, we provide
implementations for all builtin types, Nullable<T> for builtin types
(which always box), arrays of the builtin types, JavaObjectArray<T>,
IJavaPeerable, and `object`.

The `object` marshaler supersedes the previous
JniMarshal.GetValue<T>() semantics, using JavaProxyObject as needed.

One thing to note is the ParameterAttributes parameters.  This value
is intended to correspond with
System.Reflection.ParameterInfo.Attributes.  jnimarshalmethod-gen can
thus use the actual MethodInfo + ParameterInfo to encode parameter
marshaling direction.  For most types this won't be useful; this is
largely intended for *arrays*, e.g. we could have an `[In] int[]`
parameter which thus avoids the "copy out" behavior Xamarin.Android
currently performs, thus avoiding an extra JNI transition.  Or
vice-versa, have an `[Out] int[]` parameter, which avoids a JNI
transition to copy the managed `int[]` to the Java `int[]`, replacing
it with the copy-out transition.

Partially addressing Issue dotnet#2, this also introduces
JniRuntime.JniValueManager.GetValue() and GetValue<T>() methods.
GetObject() is not yet replaced, but will be in a future commit.
Actual implementation will be done alongside introduction of
CreateValue<T>() and cleaning up JniValueMarshaler.Create*Value()
methods to always create values when able, simplifying the logic if
JniValueMarshaler implementations.

Author: jonpryor

src/Java.Interop.Dynamic/Java.Interop.Dynamic/DynamicJavaClass.cs

@@ -144,41 +144,5 @@ static JavaModifiers ()
 			}
 		}
 	}
-
-	struct JniArgumentMarshalInfo {
-		JniArgumentValue                jvalue;
-		JniObjectReference              lref;
-		IJavaPeerable                   obj;
-		Action<IJavaPeerable, object>   cleanup;
-
-		internal JniArgumentMarshalInfo (object value, Type valueType)
-		{
-			this        = new JniArgumentMarshalInfo ();
-			var jvm     = JniEnvironment.Runtime;
-			var info    = jvm.ValueManager.GetJniMarshalInfoForType (valueType);
-			if (info.CreateJniArgumentValue != null)
-				jvalue = info.CreateJniArgumentValue (value);
-			else if (info.CreateMarshalCollection != null) {
-				obj     = info.CreateMarshalCollection (value);
-				jvalue  = new JniArgumentValue (obj);
-			} else if (info.CreateLocalRef != null) {
-				lref    = info.CreateLocalRef (value);
-				jvalue  = new JniArgumentValue (lref);
-			} else
-				throw new NotSupportedException ("Don't know how to get a JniArgumentValue for: " + valueType.FullName);
-			cleanup     = info.CleanupMarshalCollection;
-		}
-
-		public JniArgumentValue JniArgumentValue {
-			get {return jvalue;}
-		}
-
-		public  void    Cleanup (object value)
-		{
-			if (cleanup != null && obj != null)
-				cleanup (obj, value);
-			JniObjectReference.Dispose (ref lref);
-		}
-	}
 }
 

src/Java.Interop.Dynamic/Java.Interop.Dynamic/JavaClassInfo.cs

@@ -291,7 +291,8 @@ static bool IsStatic (JniObjectReference member)
 		internal unsafe bool TryInvokeMember (IJavaPeerable self, JavaMethodBase[] overloads, DynamicMetaObject[] args, out object value)
 		{
 			value       = null;
-			var margs   = (List<JniArgumentMarshalInfo>) null;
+			var vms     = (List<JniValueMarshaler>) null;
+			var states  = (JniValueMarshalerState[]) null;
 
 			var jtypes  = GetJniTypes (args);
 			try {
@@ -300,16 +301,21 @@ internal unsafe bool TryInvokeMember (IJavaPeerable self, JavaMethodBase[] overl
 				if (invoke == null)
 					return false;
 
-				margs       = args.Select (arg => new JniArgumentMarshalInfo (arg.Value, arg.LimitType)).ToList ();
-				var jargs   = stackalloc JniArgumentValue [margs.Count];
-				for (int i = 0; i < margs.Count; ++i)
-					jargs [i] = margs [i].JniArgumentValue;
+				var jvm     = JniEnvironment.Runtime;
+				vms         = args.Select (arg => jvm.ValueManager.GetValueMarshaler (arg.LimitType)).ToList ();
+				states      = new JniValueMarshalerState [vms.Count];
+				for (int i = 0; i < vms.Count; ++i) {
+					states [i]  = vms [i].CreateArgumentState (args [i].Value);
+				}
+				var jargs   = stackalloc JniArgumentValue [vms.Count];
+				for (int i = 0; i < states.Length; ++i)
+					jargs [i] = states [i].JniArgumentValue;
 				value       = invoke.Invoke (self, jargs);
 				return true;
 			}
 			finally {
-				for (int i = 0; margs != null && i < margs.Count; ++i) {
-					margs [i].Cleanup (args [i].Value);
+				for (int i = 0; vms != null && i < vms.Count; ++i) {
+					vms [i].DestroyArgumentState (args [i].Value, ref states [i]);
 				}
 				for (int i = 0; i < jtypes.Count; ++i) {
 					if (jtypes [i] != null)

src/Java.Interop.Dynamic/Java.Interop.Dynamic/JavaFieldInfo.cs

@@ -137,11 +137,16 @@ void SetStaticValue (object value)
 			case 'D':   members.StaticFields.SetValue (JniSignature, (double) value);   break;
 			case 'L':
 			case '[':
-				var lref = JniMarshal.CreateLocalRef (value);
+				if (value == null) {
+					members.StaticFields.SetValue (JniSignature, new JniObjectReference ());
+					return;
+				}
+				var vm  = JniEnvironment.Runtime.ValueManager.GetValueMarshaler (value.GetType ());
+				var s   = vm.CreateArgumentState (value);
 				try {
-					members.StaticFields.SetValue (JniSignature, lref);
+					members.StaticFields.SetValue (JniSignature, s.ReferenceValue);
 				} finally {
-					JniObjectReference.Dispose (ref lref);
+					vm.DestroyArgumentState (value, ref s, 0);
 				}
 				return;
 			default:
@@ -163,11 +168,16 @@ void SetInstanceValue (IJavaPeerable self, object value)
 			case 'D':   members.InstanceFields.SetValue (JniSignature, self,    (double) value);   break;
 			case 'L':
 			case '[':
-				var lref = JniMarshal.CreateLocalRef (value);
+				if (value == null) {
+					members.InstanceFields.SetValue (JniSignature, self, new JniObjectReference ());
+					return;
+				}
+				var vm  = JniEnvironment.Runtime.ValueManager.GetValueMarshaler (value.GetType ());
+				var s   = vm.CreateArgumentState (value);
 				try {
-					members.InstanceFields.SetValue (JniSignature, self, lref);
+					members.InstanceFields.SetValue (JniSignature, self, s.ReferenceValue);
 				} finally {
-					JniObjectReference.Dispose (ref lref);
+					vm.DestroyArgumentState (value, ref s, 0);
 				}
 				return;
 			default:

src/Java.Interop.Dynamic/Java.Interop.Dynamic/JniMetaObject.cs

@@ -52,12 +52,14 @@ public override DynamicMetaObject BindConvert (ConvertBinder binder)
 				return new DynamicMetaObject (typeE, BindingRestrictions.GetTypeRestriction (typeE, binder.Type), type);
 			}
 
-			var marshalInfo = vm.ValueManager.GetJniMarshalInfoForType (binder.Type);
-			if (marshalInfo.GetValueFromJni == null)
+			object value;
+			try {
+				var r   = ConversionTarget;
+				value   = vm.ValueManager.GetValue (ref r, JniObjectReferenceOptions.Copy, binder.Type);
+			} catch {
 				return binder.FallbackConvert (this);
+			}
 
-			var r       = ConversionTarget;
-			var value   = marshalInfo.GetValueFromJni (ref r, JniObjectReferenceOptions.Copy, binder.Type);
 			var valueE  = Expression.Convert (Expression.Constant (value), binder.Type);
 			return new DynamicMetaObject (valueE, BindingRestrictions.GetTypeRestriction (valueE, binder.Type), value);
 		}

src/Java.Interop/Java.Interop.csproj

@@ -119,14 +119,15 @@
     <Compile Include="Java.Interop\JniLocalReference.cs" />
     <Compile Include="Java.Interop\JniMarshal.cs" />
     <Compile Include="Java.Interop\JniMarshalMethod.cs" />
-    <Compile Include="Java.Interop\JniMarshalInfo.cs" />
     <Compile Include="Java.Interop\JniPeerMembers.JniMethods.cs">
       <DependentUpon>JniPeerMembers.JniMethods.tt</DependentUpon>
     </Compile>
     <Compile Include="Java.Interop\JniReferenceSafeHandle.cs" />
     <Compile Include="Java.Interop\JniRuntime.JniExportedMemberBuilder.cs" />
     <Compile Include="Java.Interop\JniRuntime.JniValueManager.cs" />
+    <Compile Include="Java.Interop\JniStringValueMarshaler.cs" />
     <Compile Include="Java.Interop\JniSystem.cs" />
+    <Compile Include="Java.Interop\JniValueMarshaler.cs" />
     <Compile Include="Java.Interop\JniWeakGlobalReference.cs" />
     <Compile Include="Java.Interop\ManagedPeer.cs" />
   </ItemGroup>

src/Java.Interop/Java.Interop/JavaArray.cs

@@ -3,6 +3,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Reflection;
 
 namespace Java.Interop
 {
@@ -102,7 +103,7 @@ internal IList<T> ToTargetType (Type targetType, bool dispose)
 		{
 			if (TargetTypeIsCurrentType (targetType))
 				return this;
-			if (targetType == typeof (T[])) {
+			if (targetType == typeof (T[]) || targetType.GetTypeInfo ().IsAssignableFrom (typeof (IList<T>).GetTypeInfo ())) {
 				try {
 					return ToArray ();
 				} finally {
@@ -125,22 +126,7 @@ internal static Exception CreateMarshalNotSupportedException (Type sourceType, T
 						sourceType.FullName, targetType.FullName));
 		}
 
-		internal static JniObjectReference CreateLocalRef<TArray> (object value, Func<IList<T>, TArray> creator)
-			where TArray : JavaArray<T>
-		{
-			if (value == null)
-				return new JniObjectReference ();
-			var array = value as TArray;
-			if (array != null)
-				return array.PeerReference.NewLocalRef ();
-			var items = value as IList<T>;
-			if (items == null)
-				throw CreateMarshalNotSupportedException (value.GetType (), typeof (TArray));
-			using (array = creator (items))
-				return array.PeerReference.NewLocalRef ();
-		}
-
-		internal static IList<T> GetValueFromJni<TArray> (ref JniObjectReference reference, JniObjectReferenceOptions transfer, Type targetType, ArrayCreator<TArray> creator)
+		internal static IList<T> CreateValue<TArray> (ref JniObjectReference reference, JniObjectReferenceOptions transfer, Type targetType, ArrayCreator<TArray> creator)
 			where TArray : JavaArray<T>
 		{
 			var value = JniEnvironment.Runtime.ValueManager.PeekObject (reference);
@@ -153,37 +139,57 @@ internal static IList<T> GetValueFromJni<TArray> (ref JniObjectReference referen
 				.ToTargetType (targetType, dispose: true);
 		}
 
-		internal static IJavaPeerable CreateMarshalCollection<TArray> (object value, Func<IList<T>, TArray> creator)
+		internal    static  JniValueMarshalerState  CreateArgumentState<TArray> (IList<T> value, ParameterAttributes synchronize, Func<IList<T>, bool, TArray> creator)
 			where TArray : JavaArray<T>
 		{
 			if (value == null)
-				return null;
+				return new JniValueMarshalerState ();
 			var v = value as TArray;
-			if (v != null)
-				return v;
+			if (v != null) {
+				return new JniValueMarshalerState (v);
+			}
 			var list = value as IList<T>;
 			if (list == null)
 				throw CreateMarshalNotSupportedException (value.GetType (), typeof (TArray));
-			return creator (list);
+			synchronize = GetCopyDirection (synchronize);
+			var c   = (synchronize & ParameterAttributes.In) == ParameterAttributes.In;
+			var a   = creator (list, c);
+			return new JniValueMarshalerState (a);
 		}
 
-		internal static void CleanupMarshalCollection<TArray> (IJavaPeerable marshalObject, object value)
+		internal static void DestroyArgumentState<TArray> (IList<T> value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
 			where TArray : JavaArray<T>
 		{
-			var source = (TArray) marshalObject;
+			var source = (TArray) state.PeerableValue;
 			if (source == null)
 				return;
 
-			var arrayDest = value as T[];
-			var listDest  = value as IList<T>;
-			if (arrayDest != null)
-				source.CopyTo (arrayDest, 0);
-			else if (listDest != null)
-				source.CopyToList (listDest, 0);
+			Debug.WriteLine ("# jonp: JavaArray<{0}>.DestroyArgumentState<{1}>: synchronize={2}", typeof(T).FullName, typeof(TArray).FullName, synchronize);
+			synchronize = GetCopyDirection (synchronize);
+			if ((synchronize & ParameterAttributes.Out) == ParameterAttributes.Out) {
+				var arrayDest = value as T[];
+				var listDest  = value as IList<T>;
+				if (arrayDest != null)
+					source.CopyTo (arrayDest, 0);
+				else if (listDest != null)
+					source.CopyToList (listDest, 0);
+			}
 
 			if (source.forMarshalCollection) {
 				source.Dispose ();
 			}
+
+			state   = new JniValueMarshalerState ();
+		}
+
+		internal static ParameterAttributes GetCopyDirection (ParameterAttributes value)
+		{
+			// If .In or .Out are specified, use as-is.
+			// Otherwise, we should copy both directions.
+			const   ParameterAttributes     inout   = ParameterAttributes.In | ParameterAttributes.Out;
+			if ((value & inout) != 0)
+				return (value & inout);
+			return inout;
 		}
 
 		internal virtual void CopyToList (IList<T> list, int index)

src/Java.Interop/Java.Interop/JavaObjectArray.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Reflection;
 
 namespace Java.Interop
 {
@@ -59,14 +60,15 @@ public override T this [int index] {
 		T GetElementAt (int index)
 		{
 			var lref = JniEnvironment.Arrays.GetObjectArrayElement (PeerReference, index);
-			return JniMarshal.GetValue<T> (ref lref, JniObjectReferenceOptions.CopyAndDispose);
+			return JniEnvironment.Runtime.ValueManager.GetValue<T> (ref lref, JniObjectReferenceOptions.CopyAndDispose);
 		}
 
 		void SetElementAt (int index, T value)
 		{
-			var h = JniMarshal.CreateLocalRef (value);
-			JniEnvironment.Arrays.SetObjectArrayElement (PeerReference, index, h);
-			JniObjectReference.Dispose (ref h, JniObjectReferenceOptions.CopyAndDispose);
+			var vm  = JniEnvironment.Runtime.ValueManager.GetValueMarshaler<T> ();
+			var s   = vm.CreateGenericObjectReferenceArgumentState (value);
+			JniEnvironment.Arrays.SetObjectArrayElement (PeerReference, index, s.ReferenceValue);
+			vm.DestroyGenericArgumentState (value, ref s, 0);
 		}
 
 		public override IEnumerator<T> GetEnumerator ()
@@ -80,11 +82,12 @@ public override IEnumerator<T> GetEnumerator ()
 		public override void Clear ()
 		{
 			int len = Length;
-			var v = JniMarshal.CreateLocalRef (default (T));
+			var vm  = JniEnvironment.Runtime.ValueManager.GetValueMarshaler<T> ();
+			var s   = vm.CreateArgumentState (default (T));
 			for (int i = 0; i < len; i++) {
-				JniEnvironment.Arrays.SetObjectArrayElement (PeerReference, i, v);
+				JniEnvironment.Arrays.SetObjectArrayElement (PeerReference, i, s.ReferenceValue);
 			}
-			JniObjectReference.Dispose (ref v, JniObjectReferenceOptions.CopyAndDispose);
+			vm.DestroyGenericArgumentState (default (T), ref s, 0);
 		}
 
 		public override int IndexOf (T item)
@@ -132,28 +135,30 @@ internal override bool TargetTypeIsCurrentType (Type targetType)
 				targetType == typeof (JavaObjectArray<T>);
 		}
 
-		internal static object GetValue (ref JniObjectReference handle, JniObjectReferenceOptions transfer, Type targetType)
-		{
-			return JavaArray<T>.GetValueFromJni (ref handle, transfer, targetType, (ref JniObjectReference h, JniObjectReferenceOptions t) => new JavaObjectArray<T> (ref h, t) {
-				forMarshalCollection    = true,
-			});
-		}
+		internal class ValueMarshaler : JniValueMarshaler<IList<T>> {
 
-		internal static JniObjectReference CreateLocalRef (object value)
-		{
-			return JavaArray<T>.CreateLocalRef (value, list => new JavaObjectArray<T>(list));
-		}
+			public override IList<T> CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType)
+			{
+				return JavaArray<T>.CreateValue (ref reference, options, targetType, (ref JniObjectReference h, JniObjectReferenceOptions t) => new JavaObjectArray<T> (ref h, t) {
+					forMarshalCollection    = true,
+				});
+			}
 
-		internal static IJavaPeerable CreateMarshalCollection (object value)
-		{
-			return JavaArray<T>.CreateMarshalCollection (value, list => new JavaObjectArray<T> (list) {
-				forMarshalCollection    = true,
-			});
-		}
+			public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (IList<T> value, ParameterAttributes synchronize)
+			{
+				return JavaArray<T>.CreateArgumentState (value, synchronize, (list, copy) => {
+					var a = copy
+						? new JavaObjectArray<T> (list)
+						: new JavaObjectArray<T> (list.Count);
+					a.forMarshalCollection = true;
+					return a;
+				});
+			}
 
-		internal static void CleanupMarshalCollection (IJavaPeerable marshalObject, object value)
-		{
-			JavaArray<T>.CleanupMarshalCollection<JavaObjectArray<T>> (marshalObject, value);
+			public override void DestroyGenericArgumentState (IList<T> value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
+			{
+				JavaArray<T>.DestroyArgumentState<JavaObjectArray<T>> (value, ref state, synchronize);
+			}
 		}
 	}
 }